diff --git a/hack/docker-compose/docker-compose.test.yml b/hack/docker-compose/docker-compose.test.yml index 78994da62..7541c1bea 100644 --- a/hack/docker-compose/docker-compose.test.yml +++ b/hack/docker-compose/docker-compose.test.yml @@ -43,11 +43,11 @@ services: - inner loki-proxy: - image: nginx:alpine + image: openresty/openresty:alpine ports: - 3100:80 volumes: - - ./proxy.conf:/etc/nginx/templates/default.conf.template + - ./proxy.conf:/etc/nginx/conf.d/default.conf restart: unless-stopped networks: - inner diff --git a/hack/docker-compose/proxy.conf b/hack/docker-compose/proxy.conf index 165598cc5..31f54304d 100644 --- a/hack/docker-compose/proxy.conf +++ b/hack/docker-compose/proxy.conf @@ -2,10 +2,15 @@ server { listen 80 default_server; server_name _; server_name_in_redirect off; - access_log /var/log/nginx/access.log; - error_log /var/log/nginx/error.log debug; + access_log /dev/stdout; + error_log /dev/stderr debug; + + # Artificial query latency (seconds). Set to 0 to disable. + # Change this single value to make queries slower/faster. + set $query_delay 0; location / { + access_by_lua_block { ngx.sleep(tonumber(ngx.var.query_delay)) } proxy_set_header Host $host; proxy_pass http://loki:3100/; proxy_http_version 1.1; @@ -14,14 +19,16 @@ server { } location /api/logs/v1/application/ { + access_by_lua_block { ngx.sleep(tonumber(ngx.var.query_delay)) } proxy_set_header Host $host; proxy_pass http://loki:3100/; proxy_http_version 1.1; proxy_set_header Upgrade $http_upgrade; proxy_set_header Connection "upgrade"; } - + location /api/logs/v1/audit/ { + access_by_lua_block { ngx.sleep(tonumber(ngx.var.query_delay)) } proxy_set_header Host $host; proxy_pass http://loki:3100/; proxy_http_version 1.1; @@ -30,6 +37,7 @@ server { } location /api/logs/v1/infrastructure/ { + access_by_lua_block { ngx.sleep(tonumber(ngx.var.query_delay)) } proxy_set_header Host $host; proxy_pass http://loki:3100/; proxy_http_version 1.1; diff --git a/web/cypress/e2e/integration/logs-dev-page.cy.ts b/web/cypress/e2e/integration/logs-dev-page.cy.ts index 3ce4d9ede..42aa08308 100644 --- a/web/cypress/e2e/integration/logs-dev-page.cy.ts +++ b/web/cypress/e2e/integration/logs-dev-page.cy.ts @@ -431,6 +431,7 @@ describe('Logs Dev Page', () => { 'sum by (level) (count_over_time({ kubernetes_namespace_name="my-namespace" })[10m])', { parseSpecialCharSequences: false, + delay: 1, }, ); }); @@ -449,6 +450,7 @@ describe('Logs Dev Page', () => { .type('{backspace}') .type('{ kubernetes_namespace_name="my-namespace" }', { parseSpecialCharSequences: false, + delay: 1, }); }); diff --git a/web/cypress/e2e/integration/logs-page.cy.ts b/web/cypress/e2e/integration/logs-page.cy.ts index 44005dea2..f85e0a4ee 100644 --- a/web/cypress/e2e/integration/logs-page.cy.ts +++ b/web/cypress/e2e/integration/logs-page.cy.ts @@ -163,6 +163,90 @@ describe('Logs Page', () => { }); }); + it('displays a Loki error payload returned with HTTP 200', () => { + cy.intercept(QUERY_RANGE_STREAMS_URL_MATCH, { + statusCode: 200, + body: { + status: 'error', + errorType: 'bad_data', + error: 'parse error at line 1, col 1: unexpected IDENTIFIER', + }, + }).as('queryRangeStreams'); + + cy.visit(LOGS_PAGE_URL); + + cy.wait('@queryRangeStreams'); + + cy.byTestID(TestIds.LogsTable) + .should('exist') + .within(() => { + cy.contains(/bad_data/i); + cy.contains('parse error at line 1, col 1: unexpected IDENTIFIER'); + }); + }); + + it('keeps load more logs available after a Loki error payload returned with HTTP 200', () => { + let requestCount = 0; + + cy.intercept(QUERY_RANGE_STREAMS_URL_MATCH, (req) => { + requestCount += 1; + req.reply( + requestCount === 1 + ? queryRangeStreamsValidResponse({ message: TEST_MESSAGE }) + : { + statusCode: 200, + body: { + status: 'error', + errorType: 'bad_data', + error: 'parse error at line 1, col 1: unexpected IDENTIFIER', + }, + }, + ); + }).as('queryRangeStreams'); + + cy.visit(LOGS_PAGE_URL); + cy.wait('@queryRangeStreams'); + + cy.byTestID(TestIds.LoadMoreLogs).click(); + cy.wait('@queryRangeStreams'); + + cy.byTestID(TestIds.LoadMoreLogs).should('exist'); + }); + + it('keeps the latest query results when an earlier request completes late', () => { + let requestCount = 0; + + cy.intercept(QUERY_RANGE_STREAMS_URL_MATCH, (req) => { + requestCount += 1; + const body = queryRangeStreamsValidResponse({ + message: + requestCount === 1 + ? 'initial result' + : requestCount === 2 + ? 'stale result' + : 'latest result', + }); + + req.reply(requestCount === 2 ? { body, delay: 3_000 } : body); + }).as('queryRangeStreams'); + + cy.visit(LOGS_PAGE_URL); + cy.wait('@queryRangeStreams'); + + cy.byTestID(TestIds.SyncButton).click(); + cy.byTestID(TestIds.TimeRangeDropdown).click(); + cy.contains('Last 6 hours').click(); + + cy.contains('latest result').should('exist'); + cy.byTestID(TestIds.LoadMoreLogs).should('exist'); + + cy.wait('@queryRangeStreams'); + cy.wait('@queryRangeStreams'); + cy.contains('latest result').should('exist'); + cy.contains('stale result').should('not.exist'); + cy.byTestID(TestIds.LoadMoreLogs).should('exist'); + }); + it('executes a query when "run query" is pressed', () => { cy.intercept( QUERY_RANGE_STREAMS_URL_MATCH, diff --git a/web/eslint.config.ts b/web/eslint.config.ts index d32630364..5c6b134dc 100644 --- a/web/eslint.config.ts +++ b/web/eslint.config.ts @@ -20,6 +20,12 @@ const compat = new FlatCompat({ export default defineConfig([ { + linterOptions: { + // eslint --fix will get in a loop where there is no error so it deletes the directive, + // which in turn causes the error to then be shown. + reportUnusedDisableDirectives: 'off', + }, + extends: fixupConfigRules( compat.extends( 'eslint:recommended', diff --git a/web/locales/en/plugin__logging-view-plugin.json b/web/locales/en/plugin__logging-view-plugin.json index 234bd83eb..16b7577b3 100644 --- a/web/locales/en/plugin__logging-view-plugin.json +++ b/web/locales/en/plugin__logging-view-plugin.json @@ -155,6 +155,7 @@ "Streaming Logs...": "Streaming Logs...", "More data available": "More data available", "Click to load": "Click to load", + "No more data available": "No more data available", "Aggregated Logs": "Aggregated Logs", "Logs": "Logs", "Please select a namespace": "Please select a namespace" diff --git a/web/src/__tests__/loki-client.spec.ts b/web/src/__tests__/loki-client.spec.ts index 5addc369b..d95df4c58 100644 --- a/web/src/__tests__/loki-client.spec.ts +++ b/web/src/__tests__/loki-client.spec.ts @@ -1,5 +1,5 @@ import { SchemaConfig } from '../logs.types'; -import { getFetchConfig } from '../loki-client'; +import { getFetchConfig, isQueryRangeResponse, throwResponseError } from '../loki-client'; jest.mock('@openshift-console/dynamic-plugin-sdk', () => ({ consoleFetchJSON: jest.fn(), @@ -64,4 +64,18 @@ describe('Loki Client', () => { expect(getFetchConfig(config)).toEqual(expectedFetchConfig); }); }); + + it('rejects Loki error responses', () => { + expect(() => + throwResponseError({ + status: 'error', + errorType: 'bad_data', + error: 'parse error at line 1, col 1', + }), + ).toThrow('bad_data: parse error at line 1, col 1'); + }); + + it('identifies malformed successful responses', () => { + expect(isQueryRangeResponse({ status: 'success', data: {} })).toBe(false); + }); }); diff --git a/web/src/components/logs-table.css b/web/src/components/logs-table.css index 7faeeefed..ccafdc3d3 100644 --- a/web/src/components/logs-table.css +++ b/web/src/components/logs-table.css @@ -85,6 +85,9 @@ .lv-plugin__table__row-more-data td { text-align: center; +} + +.lv-plugin__table__row-more-data--clickable td { cursor: pointer; } diff --git a/web/src/components/virtualized-logs-table.tsx b/web/src/components/virtualized-logs-table.tsx index 871b3addf..b9d92f8f5 100644 --- a/web/src/components/virtualized-logs-table.tsx +++ b/web/src/components/virtualized-logs-table.tsx @@ -27,6 +27,7 @@ import { import { useTranslation } from 'react-i18next'; import { LogTableData, Schema } from '../logs.types'; import { getSeverityColor, Severity } from '../severity'; +import { TestIds } from '../test-ids'; import { CenteredContainer } from './centered-container'; import { ErrorMessage } from './error-message'; @@ -416,18 +417,27 @@ export const VirtualizedLogsTable = ({ )} - {!isLoading && hasMoreLogsData && ( + {!dataIsEmpty && ( { - setScrollToIndex(data.length - 1); - onLoadMore?.(); + if (!isLoading && !isLoadingMore && hasMoreLogsData) { + setScrollToIndex(data.length - 1); + onLoadMore?.(); + } }} > - - {t('More data available')}, {isLoadingMore ? t('Loading...') : t('Click to load')} - + {hasMoreLogsData ? ( + + {t('More data available')}, {isLoadingMore ? t('Loading...') : t('Click to load')} + + ) : ( + + {t('No more data available')} + + )} )} diff --git a/web/src/hooks/useLogs.ts b/web/src/hooks/useLogs.ts index 0db62c714..a1d5b376d 100644 --- a/web/src/hooks/useLogs.ts +++ b/web/src/hooks/useLogs.ts @@ -14,6 +14,10 @@ import { executeHistogramQuery, executeQueryRange, executeVolumeRange, + throwResponseError, + toQueryRangeResponse, + toRecord, + validateQueryRangeResponse, } from '../loki-client'; import { intervalFromTimeRange, numericTimeRange, timeRangeFromDuration } from '../time-range'; import { msToNs } from '../value-utils'; @@ -36,6 +40,7 @@ type State = { isLoadingMoreLogsData: boolean; logsData?: QueryRangeResponse; logsError?: unknown; + moreLogsError?: unknown; isLoadingVolumeData?: boolean; volumeData?: VolumeRangeResponse; volumeError?: unknown; @@ -80,6 +85,10 @@ type Action = type: 'logsError'; payload: { error: unknown }; } + | { + type: 'moreLogsError'; + payload: { error: unknown }; + } | { type: 'histogramError'; payload: { error: unknown }; @@ -161,7 +170,9 @@ const reducer = (state: State, action: Action): State => { isLoadingLogsData: true, logsData: undefined, logsError: undefined, + moreLogsError: undefined, hasMoreLogsData: false, + isLoadingMoreLogsData: false, isStreaming: false, isLoadingVolumeData: false, }; @@ -170,6 +181,7 @@ const reducer = (state: State, action: Action): State => { ...state, logsData: undefined, logsError: undefined, + moreLogsError: undefined, hasMoreLogsData: false, isStreaming: true, }; @@ -210,11 +222,13 @@ const reducer = (state: State, action: Action): State => { ...state, isLoadingMoreLogsData: true, logsError: undefined, + moreLogsError: undefined, }; case 'logsResponse': return { ...state, isLoadingLogsData: false, + isLoadingMoreLogsData: false, showVolumeGraph: false, logsData: action.payload.logsData, hasMoreLogsData: hasMoreLogs(action.payload.logsData, action.payload.config.logsLimit), @@ -232,6 +246,15 @@ const reducer = (state: State, action: Action): State => { isLoadingLogsData: false, isLoadingMoreLogsData: false, logsError: action.payload.error, + moreLogsError: undefined, + }; + case 'moreLogsError': + return { + ...state, + isLoadingLogsData: false, + isLoadingMoreLogsData: false, + logsError: undefined, + moreLogsError: action.payload.error, }; default: @@ -283,6 +306,7 @@ export const useLogs = ( histogramError, volumeData, logsError, + moreLogsError, volumeError, showVolumeGraph, hasMoreLogsData, @@ -312,7 +336,7 @@ export const useLogs = ( schema: Schema; }) => { if (query.length === 0) { - dispatch({ type: 'logsError', payload: { error: new Error('Query is empty') } }); + dispatch({ type: 'moreLogsError', payload: { error: new Error('Query is empty') } }); return; } @@ -355,7 +379,11 @@ export const useLogs = ( logsAbort.current = abort; - const queryResponse = await request(); + const queryResponse = await request() + .then(toRecord) + .then(throwResponseError) + .then(toQueryRangeResponse) + .then(validateQueryRangeResponse); dispatch({ type: 'moreLogsResponse', @@ -363,7 +391,7 @@ export const useLogs = ( }); } catch (error) { if (!isAbortError(error)) { - dispatch({ type: 'logsError', payload: { error } }); + dispatch({ type: 'moreLogsError', payload: { error } }); } } }; @@ -423,7 +451,11 @@ export const useLogs = ( logsAbort.current = abort; - const queryResponse = await request(); + const queryResponse = await request() + .then(toRecord) + .then(throwResponseError) + .then(toQueryRangeResponse) + .then(validateQueryRangeResponse); dispatch({ type: 'logsResponse', payload: { logsData: queryResponse, config } }); } catch (error) { @@ -671,6 +703,7 @@ export const useLogs = ( getMoreLogs, hasMoreLogsData, logsError, + moreLogsError, getHistogram, histogramError, toggleStreaming, diff --git a/web/src/loki-client.ts b/web/src/loki-client.ts index 695b7e50b..cb1a38e6b 100644 --- a/web/src/loki-client.ts +++ b/web/src/loki-client.ts @@ -61,6 +61,54 @@ type LokiTailQueryParams = { const MAX_RANGE_REQUEST_NS = 21_600_000_000_000n; // 6 hours in nanoseconds +export const isRecord = (response: unknown): response is Record => + typeof response === 'object' && response !== null && !Array.isArray(response); + +export const toRecord = (response: unknown): Record => { + if (!isRecord(response)) { + throw new Error('Invalid Loki query response'); + } + + return response; +}; + +export const throwResponseError = (response: Record): Record => { + if (response.status !== 'error') { + return response; + } + + const errorType = typeof response.errorType === 'string' ? response.errorType : undefined; + const error = typeof response.error === 'string' ? response.error : undefined; + throw new Error([errorType, error].filter(Boolean).join(': ') || 'Loki query failed'); +}; + +export const isQueryRangeResponse = ( + response: Record, +): response is QueryRangeResponse => { + const data = response.data; + return isRecord(data) && Array.isArray(data.result); +}; + +export const toQueryRangeResponse = (response: Record): QueryRangeResponse => { + if (!isQueryRangeResponse(response)) { + throw new Error('Invalid Loki query response: missing data.result'); + } + + return response; +}; + +export const validateQueryRangeResponse = (response: QueryRangeResponse): QueryRangeResponse => { + if (response.status !== 'success') { + throw new Error(`Invalid Loki query response status: ${String(response.status)}`); + } + + if (response.data.resultType !== 'streams' && response.data.resultType !== 'matrix') { + throw new Error('Invalid Loki query response: invalid data.resultType'); + } + + return response; +}; + export const getFetchConfig = ({ config, tenant, diff --git a/web/src/test-ids.ts b/web/src/test-ids.ts index 623b30e92..4dbf89a20 100644 --- a/web/src/test-ids.ts +++ b/web/src/test-ids.ts @@ -24,4 +24,5 @@ export enum TestIds { NamespaceDropdown = 'NamespaceDropdown', NamespaceToggle = 'NamespaceToggle', SchemaToggle = 'SchemaToggle', + LoadMoreLogs = 'LoadMoreLogs', }