chore(service): allow to retrieve envoy gateway logs #2326
Conversation
|
Qovery can create a Preview Environment for this PR.
This comment has been generated from Qovery AI 🤖.
|
|
/qovery preview 28c47145-c8e7-4b9d-8d9e-c65c95b48425 |
|
A preview environment was automatically created via Qovery. Another comment will be posted when deployments are finished |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## staging #2326 +/- ##
===========================================
+ Coverage 47.73% 49.09% +1.35%
===========================================
Files 1254 922 -332
Lines 22717 18523 -4194
Branches 6635 5466 -1169
===========================================
- Hits 10844 9093 -1751
+ Misses 9808 7709 -2099
+ Partials 2065 1721 -344
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Your preview environment has been successfully deployed ! |
Ticket: QOV-1430
benjaminch
force-pushed
the
chore/QOV-1430/allow-to-retrieve-envoy-gateway-logs
branch
from
dcafb9a to
ab4a64d
Compare
There was a problem hiding this comment.
Pull request overview
This PR extends the service logs functionality to support retrieving Envoy Gateway logs, mirroring the existing capability for Nginx logs. Users can now filter and view logs from both Nginx and Envoy ingress controllers alongside their service logs.
Changes:
- Added 'envoy' filter support throughout the logs architecture (search, context, data access)
- Implemented Envoy-specific WebSocket subscriptions and query handling for both live and historical logs
- Added UI elements (ENVOY badge) to distinguish Envoy logs from regular service and Nginx logs
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| search-service-logs.tsx | Added 'envoy' to valid filter keys, buildValueOptions, buildQueryParams, and filter options |
| search-service-logs.spec.tsx | Updated test to use refetchHistoryLogs parameter instead of isLoading |
| search-service-logs-helpers.spec.ts | New test file for filter parsing logic (tests inline logic, not implementation) |
| service-logs-context.tsx | Added envoy BooleanParam to query parameters |
| row-service-logs.tsx | Added isEnvoy detection, ENVOY badge, and conditional rendering logic matching nginx |
| row-service-logs.spec.tsx | Added comprehensive test coverage for envoy logs behavior |
| use-service-live-logs.ts | Added envoy WebSocket subscription with separate query and log handler |
| use-service-history-logs.ts | Added envoy query support, log accumulation, and refetch logic |
| domains-service-logs-data-access.ts | Added LogType type ('service' | 'nginx' | 'envoy'), updated buildLokiQuery to support envoy |
| domains-service-logs-data-access.spec.ts | New comprehensive test file for buildLokiQuery with all log types |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| describe('SearchServiceLogs helpers', () => { | ||
| describe('filter parsing', () => { | ||
| it('should parse nginx filter', () => { | ||
| const input = 'nginx:true' | ||
| const filterRegex = /(\w+)[:]([^\s]*)/g | ||
| const matches = input.match(filterRegex) | ||
|
|
||
| expect(matches).toEqual(['nginx:true']) | ||
|
|
||
| const parts = matches?.[0].split(/[:]/) | ||
| expect(parts?.[0]).toBe('nginx') | ||
| expect(parts?.[1]).toBe('true') | ||
| }) | ||
|
|
||
| it('should parse envoy filter', () => { | ||
| const input = 'envoy:true' | ||
| const filterRegex = /(\w+)[:]([^\s]*)/g | ||
| const matches = input.match(filterRegex) | ||
|
|
||
| expect(matches).toEqual(['envoy:true']) | ||
|
|
||
| const parts = matches?.[0].split(/[:]/) | ||
| expect(parts?.[0]).toBe('envoy') | ||
| expect(parts?.[1]).toBe('true') | ||
| }) | ||
|
|
||
| it('should parse multiple filters including nginx', () => { | ||
| const input = 'level:error nginx:true' | ||
| const filterRegex = /(\w+)[:]([^\s]*)/g | ||
| const matches = input.match(filterRegex) | ||
|
|
||
| expect(matches).toEqual(['level:error', 'nginx:true']) | ||
| }) | ||
|
|
||
| it('should parse multiple filters including envoy', () => { | ||
| const input = 'level:error envoy:true' | ||
| const filterRegex = /(\w+)[:]([^\s]*)/g | ||
| const matches = input.match(filterRegex) | ||
|
|
||
| expect(matches).toEqual(['level:error', 'envoy:true']) | ||
| }) | ||
|
|
||
| it('should parse instance filter with envoy', () => { | ||
| const input = 'instance:envoy-pod-123 envoy:true' | ||
| const filterRegex = /(\w+)[:]([^\s]*)/g | ||
| const matches = input.match(filterRegex) | ||
|
|
||
| expect(matches).toEqual(['instance:envoy-pod-123', 'envoy:true']) | ||
| }) | ||
|
|
||
| it('should parse search text with nginx filter', () => { | ||
| const input = 'nginx:true error occurred' | ||
| const filterRegex = /(\w+)[:]([^\s]*)/g | ||
| const textWithoutFilters = input.replace(filterRegex, '').trim() | ||
|
|
||
| expect(textWithoutFilters).toBe('error occurred') | ||
| }) | ||
|
|
||
| it('should parse search text with envoy filter', () => { | ||
| const input = 'envoy:true upstream timeout' | ||
| const filterRegex = /(\w+)[:]([^\s]*)/g | ||
| const textWithoutFilters = input.replace(filterRegex, '').trim() | ||
|
|
||
| expect(textWithoutFilters).toBe('upstream timeout') | ||
| }) | ||
|
|
||
| it('should handle both nginx and envoy filters together', () => { | ||
| const input = 'nginx:true envoy:true level:error' | ||
| const filterRegex = /(\w+)[:]([^\s]*)/g | ||
| const matches = input.match(filterRegex) | ||
|
|
||
| expect(matches).toEqual(['nginx:true', 'envoy:true', 'level:error']) | ||
| }) | ||
| }) | ||
|
|
||
| describe('valid filter keys', () => { | ||
| const VALID_FILTER_KEYS = ['level', 'instance', 'message', 'nginx', 'envoy', 'search', 'deploymentId'] | ||
|
|
||
| it('should include nginx in valid filter keys', () => { | ||
| expect(VALID_FILTER_KEYS).toContain('nginx') | ||
| }) | ||
|
|
||
| it('should include envoy in valid filter keys', () => { | ||
| expect(VALID_FILTER_KEYS).toContain('envoy') | ||
| }) | ||
|
|
||
| it('should validate nginx filter key', () => { | ||
| expect(VALID_FILTER_KEYS.includes('nginx')).toBe(true) | ||
| }) | ||
|
|
||
| it('should validate envoy filter key', () => { | ||
| expect(VALID_FILTER_KEYS.includes('envoy')).toBe(true) | ||
| }) | ||
|
|
||
| it('should include all expected filter keys', () => { | ||
| expect(VALID_FILTER_KEYS).toEqual( | ||
| expect.arrayContaining(['level', 'instance', 'message', 'nginx', 'envoy', 'search', 'deploymentId']) | ||
| ) | ||
| }) | ||
| }) | ||
|
|
||
| describe('buildValueOptions logic', () => { | ||
| it('should create nginx filter option', () => { | ||
| const queryParams = { nginx: true } | ||
| const value = 'nginx:true' | ||
|
|
||
| expect(value).toBe('nginx:true') | ||
| }) | ||
|
|
||
| it('should create envoy filter option', () => { | ||
| const queryParams = { envoy: true } | ||
| const value = 'envoy:true' | ||
|
|
||
| expect(value).toBe('envoy:true') | ||
| }) | ||
|
|
||
| it('should handle both nginx and envoy params', () => { | ||
| const nginxValue = 'nginx:true' | ||
| const envoyValue = 'envoy:true' | ||
|
|
||
| expect(nginxValue).not.toBe(envoyValue) | ||
| expect(nginxValue).toContain('nginx') | ||
| expect(envoyValue).toContain('envoy') | ||
| }) | ||
| }) | ||
|
|
||
| describe('buildQueryParams logic', () => { | ||
| it('should set nginx param to true when nginx filter is present', () => { | ||
| const filterKey = 'nginx' | ||
| const shouldBeTrue = filterKey === 'nginx' | ||
|
|
||
| expect(shouldBeTrue).toBe(true) | ||
| }) | ||
|
|
||
| it('should set envoy param to true when envoy filter is present', () => { | ||
| const filterKey = 'envoy' | ||
| const shouldBeTrue = filterKey === 'envoy' | ||
|
|
||
| expect(shouldBeTrue).toBe(true) | ||
| }) | ||
|
|
||
| it('should handle both nginx and envoy filters independently', () => { | ||
| const filters = ['nginx', 'envoy'] | ||
|
|
||
| expect(filters.includes('nginx')).toBe(true) | ||
| expect(filters.includes('envoy')).toBe(true) | ||
| }) | ||
| }) | ||
| }) |
There was a problem hiding this comment.
The test file 'search-service-logs-helpers.spec.ts' contains tests that don't actually test the implementation in 'search-service-logs.tsx'. The tests redefine logic inline (like VALID_FILTER_KEYS at line 77, filterRegex patterns, etc.) rather than importing and testing the actual buildValueOptions and buildQueryParams functions from the implementation. This means the tests will always pass regardless of whether the actual implementation is correct. Consider either:
- Exporting the helper functions (buildValueOptions, buildQueryParams, VALID_FILTER_KEYS) from search-service-logs.tsx so they can be imported and tested, or
- Removing this test file and adding more comprehensive tests to search-service-logs.spec.tsx that test the component behavior with envoy filters.
| const [enabledNginx, setEnabledNginx] = useState(false) | ||
| const [enabledEnvoy, setEnabledEnvoy] = useState(false) |
There was a problem hiding this comment.
The state variables 'enabledNginx' (line 35) and 'enabledEnvoy' (line 36) along with their setters are defined and exported (lines 280-283) but are never actually used anywhere in the codebase. The WebSocket subscriptions are already properly controlled by the 'enabled' prop condition based on queryParams.nginx and queryParams.envoy. Consider removing these unused state variables unless they are intended for future use.
| const [enabledNginx, setEnabledNginx] = useState(false) | |
| const [enabledEnvoy, setEnabledEnvoy] = useState(false) |
| queryParams.message, | ||
| queryParams.search, | ||
| queryParams.version, | ||
| queryParams.nginx, |
There was a problem hiding this comment.
The dependency array includes 'queryParams.nginx' but this value is not used in the useMemo computation. The nginx parameter only affects the 'enabled' condition of the WebSocket subscription, not the query string itself. Including it here causes unnecessary recalculations of the memoized query. Consider removing 'queryParams.nginx' from this dependency array.
| queryParams.nginx, |
| queryParams.message, | ||
| queryParams.search, | ||
| queryParams.version, | ||
| queryParams.envoy, |
There was a problem hiding this comment.
The dependency array includes 'queryParams.envoy' but this value is not used in the useMemo computation. The envoy parameter only affects the 'enabled' condition of the WebSocket subscription, not the query string itself. Including it here causes unnecessary recalculations of the memoized query. Consider removing 'queryParams.envoy' from this dependency array.
| queryParams.envoy, |
| const queryParams = { nginx: true } | ||
| const value = 'nginx:true' | ||
|
|
||
| expect(value).toBe('nginx:true') | ||
| }) | ||
|
|
||
| it('should create envoy filter option', () => { | ||
| const queryParams = { envoy: true } |
There was a problem hiding this comment.
Unused variable queryParams.
| const queryParams = { nginx: true } | |
| const value = 'nginx:true' | |
| expect(value).toBe('nginx:true') | |
| }) | |
| it('should create envoy filter option', () => { | |
| const queryParams = { envoy: true } | |
| const value = 'nginx:true' | |
| expect(value).toBe('nginx:true') | |
| }) | |
| it('should create envoy filter option', () => { |
| const queryParams = { nginx: true } | ||
| const value = 'nginx:true' | ||
|
|
||
| expect(value).toBe('nginx:true') | ||
| }) | ||
|
|
||
| it('should create envoy filter option', () => { | ||
| const queryParams = { envoy: true } |
There was a problem hiding this comment.
Unused variable queryParams.
| const queryParams = { nginx: true } | |
| const value = 'nginx:true' | |
| expect(value).toBe('nginx:true') | |
| }) | |
| it('should create envoy filter option', () => { | |
| const queryParams = { envoy: true } | |
| const value = 'nginx:true' | |
| expect(value).toBe('nginx:true') | |
| }) | |
| it('should create envoy filter option', () => { |
Summary
Issue: Ticket: QOV-1430
This PR allows users to retrieve Envoy logs for their services as they can already do for Nginx.
Screenshots / Recordings
Testing
yarn testoryarn test -u(if you need to regenerate snapshots)yarn formatyarn lintPR Checklist
.cursor/rules)feat(service): add new Terraform service) - required for semantic-release