From 33186e3e233583245353733568edb9d0ac89c7e6 Mon Sep 17 00:00:00 2001 From: Ivana Huckova <30407135+ivanahuckova@users.noreply.github.com> Date: Wed, 19 Apr 2023 17:04:47 +0200 Subject: [PATCH] Explore: Fix using data source line limit when opening logs sample in split view (#66601) * Logs sample: Use data source max lines setting when opening logs in split view * Add internal tags --- packages/grafana-data/src/types/logs.ts | 22 +++- .../app/features/explore/LogsSamplePanel.tsx | 2 +- .../utils/supplementaryQueries.test.ts | 3 +- .../elasticsearch/datasource.test.ts | 28 +++-- .../datasource/elasticsearch/datasource.ts | 9 +- .../datasource/loki/datasource.test.ts | 100 ++++++++++++------ .../app/plugins/datasource/loki/datasource.ts | 13 +-- 7 files changed, 121 insertions(+), 56 deletions(-) diff --git a/packages/grafana-data/src/types/logs.ts b/packages/grafana-data/src/types/logs.ts index 9200942492c..8b180435628 100644 --- a/packages/grafana-data/src/types/logs.ts +++ b/packages/grafana-data/src/types/logs.ts @@ -163,6 +163,26 @@ export enum SupplementaryQueryType { LogsSample = 'LogsSample', } +/** + * @internal + */ +export type SupplementaryQueryOptions = LogsVolumeOption | LogsSampleOptions; + +/** + * @internal + */ +export type LogsVolumeOption = { + type: SupplementaryQueryType.LogsVolume; +}; + +/** + * @internal + */ +export type LogsSampleOptions = { + type: SupplementaryQueryType.LogsSample; + limit?: number; +}; + /** * Types of logs volume responses. A data source may return full range histogram (based on selected range) * or limited (based on returned results). This information is attached to DataFrame.meta.custom object. @@ -206,7 +226,7 @@ export interface DataSourceWithSupplementaryQueriesSupport( diff --git a/public/app/features/explore/LogsSamplePanel.tsx b/public/app/features/explore/LogsSamplePanel.tsx index 8f5a89f4865..6a3492e120f 100644 --- a/public/app/features/explore/LogsSamplePanel.tsx +++ b/public/app/features/explore/LogsSamplePanel.tsx @@ -50,7 +50,7 @@ export function LogsSamplePanel(props: Props) { } const logSampleQueries = queries - .map((query) => datasourceInstance.getSupplementaryQuery(SupplementaryQueryType.LogsSample, query)) + .map((query) => datasourceInstance.getSupplementaryQuery({ type: SupplementaryQueryType.LogsSample }, query)) .filter((query): query is DataQuery => !!query); if (!logSampleQueries.length) { diff --git a/public/app/features/explore/utils/supplementaryQueries.test.ts b/public/app/features/explore/utils/supplementaryQueries.test.ts index ce85095d7e1..3f13e0d2f51 100644 --- a/public/app/features/explore/utils/supplementaryQueries.test.ts +++ b/public/app/features/explore/utils/supplementaryQueries.test.ts @@ -13,6 +13,7 @@ import { LogsVolumeType, MutableDataFrame, SupplementaryQueryType, + SupplementaryQueryOptions, toDataFrame, } from '@grafana/data'; import { getDataSourceSrv } from '@grafana/runtime'; @@ -53,7 +54,7 @@ class MockDataSourceWithSupplementaryQuerySupport return undefined; } - getSupplementaryQuery(type: SupplementaryQueryType, query: DataQuery): DataQuery | undefined { + getSupplementaryQuery(options: SupplementaryQueryOptions, query: DataQuery): DataQuery | undefined { return query; } diff --git a/public/app/plugins/datasource/elasticsearch/datasource.test.ts b/public/app/plugins/datasource/elasticsearch/datasource.test.ts index c5bd3b8a5f6..6793fa31871 100644 --- a/public/app/plugins/datasource/elasticsearch/datasource.test.ts +++ b/public/app/plugins/datasource/elasticsearch/datasource.test.ts @@ -927,22 +927,28 @@ describe('ElasticDatasource', () => { it('does not return logs volume query for metric query', () => { expect( - ds.getSupplementaryQuery(SupplementaryQueryType.LogsVolume, { - refId: 'A', - metrics: [{ type: 'count', id: '1' }], - bucketAggs: [{ type: 'filters', settings: { filters: [{ query: 'foo', label: '' }] }, id: '1' }], - query: 'foo="bar"', - }) + ds.getSupplementaryQuery( + { type: SupplementaryQueryType.LogsVolume }, + { + refId: 'A', + metrics: [{ type: 'count', id: '1' }], + bucketAggs: [{ type: 'filters', settings: { filters: [{ query: 'foo', label: '' }] }, id: '1' }], + query: 'foo="bar"', + } + ) ).toEqual(undefined); }); it('returns logs volume query for log query', () => { expect( - ds.getSupplementaryQuery(SupplementaryQueryType.LogsVolume, { - refId: 'A', - metrics: [{ type: 'logs', id: '1' }], - query: 'foo="bar"', - }) + ds.getSupplementaryQuery( + { type: SupplementaryQueryType.LogsVolume }, + { + refId: 'A', + metrics: [{ type: 'logs', id: '1' }], + query: 'foo="bar"', + } + ) ).toEqual({ bucketAggs: [ { diff --git a/public/app/plugins/datasource/elasticsearch/datasource.ts b/public/app/plugins/datasource/elasticsearch/datasource.ts index 059d99f2e86..e1d887b9f6f 100644 --- a/public/app/plugins/datasource/elasticsearch/datasource.ts +++ b/public/app/plugins/datasource/elasticsearch/datasource.ts @@ -25,6 +25,7 @@ import { QueryFixAction, CoreApp, SupplementaryQueryType, + SupplementaryQueryOptions, DataQueryError, rangeUtil, Field, @@ -597,14 +598,14 @@ export class ElasticDatasource return [SupplementaryQueryType.LogsVolume]; } - getSupplementaryQuery(type: SupplementaryQueryType, query: ElasticsearchQuery): ElasticsearchQuery | undefined { - if (!this.getSupportedSupplementaryQueryTypes().includes(type)) { + getSupplementaryQuery(options: SupplementaryQueryOptions, query: ElasticsearchQuery): ElasticsearchQuery | undefined { + if (!this.getSupportedSupplementaryQueryTypes().includes(options.type)) { return undefined; } let isQuerySuitable = false; - switch (type) { + switch (options.type) { case SupplementaryQueryType.LogsVolume: // it has to be a logs-producing range-query isQuerySuitable = !!(query.metrics?.length === 1 && query.metrics[0].type === 'logs'); @@ -655,7 +656,7 @@ export class ElasticDatasource getLogsVolumeDataProvider(request: DataQueryRequest): Observable | undefined { const logsVolumeRequest = cloneDeep(request); const targets = logsVolumeRequest.targets - .map((target) => this.getSupplementaryQuery(SupplementaryQueryType.LogsVolume, target)) + .map((target) => this.getSupplementaryQuery({ type: SupplementaryQueryType.LogsVolume }, target)) .filter((query): query is ElasticsearchQuery => !!query); if (!targets.length) { diff --git a/public/app/plugins/datasource/loki/datasource.test.ts b/public/app/plugins/datasource/loki/datasource.test.ts index e9c275581aa..312ec7729d6 100644 --- a/public/app/plugins/datasource/loki/datasource.test.ts +++ b/public/app/plugins/datasource/loki/datasource.test.ts @@ -923,11 +923,14 @@ describe('LokiDatasource', () => { describe('logs volume', () => { it('returns logs volume query for range log query', () => { expect( - ds.getSupplementaryQuery(SupplementaryQueryType.LogsVolume, { - expr: '{label=value}', - queryType: LokiQueryType.Range, - refId: 'A', - }) + ds.getSupplementaryQuery( + { type: SupplementaryQueryType.LogsVolume }, + { + expr: '{label=value}', + queryType: LokiQueryType.Range, + refId: 'A', + } + ) ).toEqual({ expr: 'sum by (level) (count_over_time({label=value}[$__interval]))', instant: false, @@ -939,21 +942,27 @@ describe('LokiDatasource', () => { it('does not return logs volume query for instant log query', () => { expect( - ds.getSupplementaryQuery(SupplementaryQueryType.LogsVolume, { - expr: '{label=value}', - queryType: LokiQueryType.Instant, - refId: 'A', - }) + ds.getSupplementaryQuery( + { type: SupplementaryQueryType.LogsVolume }, + { + expr: '{label=value}', + queryType: LokiQueryType.Instant, + refId: 'A', + } + ) ).toEqual(undefined); }); it('does not return logs volume query for metric query', () => { expect( - ds.getSupplementaryQuery(SupplementaryQueryType.LogsVolume, { - expr: 'rate({label=value}[5m]', - queryType: LokiQueryType.Range, - refId: 'A', - }) + ds.getSupplementaryQuery( + { type: SupplementaryQueryType.LogsVolume }, + { + expr: 'rate({label=value}[5m]', + queryType: LokiQueryType.Range, + refId: 'A', + } + ) ).toEqual(undefined); }); }); @@ -961,41 +970,68 @@ describe('LokiDatasource', () => { describe('logs sample', () => { it('returns logs sample query for range metric query', () => { expect( - ds.getSupplementaryQuery(SupplementaryQueryType.LogsSample, { - expr: 'rate({label=value}[5m]', - queryType: LokiQueryType.Range, - refId: 'A', - }) + ds.getSupplementaryQuery( + { type: SupplementaryQueryType.LogsSample }, + { + expr: 'rate({label=value}[5m]', + queryType: LokiQueryType.Range, + refId: 'A', + } + ) ).toEqual({ expr: '{label=value}', queryType: 'range', refId: 'log-sample-A', - maxLines: 100, + maxLines: 20, }); }); it('returns logs sample query for instant metric query', () => { expect( - ds.getSupplementaryQuery(SupplementaryQueryType.LogsSample, { - expr: 'rate({label=value}[5m]', - queryType: LokiQueryType.Instant, - refId: 'A', - }) + ds.getSupplementaryQuery( + { type: SupplementaryQueryType.LogsSample }, + { + expr: 'rate({label=value}[5m]', + queryType: LokiQueryType.Instant, + refId: 'A', + } + ) ).toEqual({ expr: '{label=value}', queryType: 'instant', refId: 'log-sample-A', - maxLines: 100, + maxLines: 20, + }); + }); + + it('correctly overrides maxLines if limit is set', () => { + expect( + ds.getSupplementaryQuery( + { type: SupplementaryQueryType.LogsSample, limit: 5 }, + { + expr: 'rate({label=value}[5m]', + queryType: LokiQueryType.Instant, + refId: 'A', + } + ) + ).toEqual({ + expr: '{label=value}', + queryType: 'instant', + refId: 'log-sample-A', + maxLines: 5, }); }); it('does not return logs sample query for log query query', () => { expect( - ds.getSupplementaryQuery(SupplementaryQueryType.LogsSample, { - expr: '{label=value}', - queryType: LokiQueryType.Range, - refId: 'A', - }) + ds.getSupplementaryQuery( + { type: SupplementaryQueryType.LogsSample }, + { + expr: '{label=value}', + queryType: LokiQueryType.Range, + refId: 'A', + } + ) ).toEqual(undefined); }); }); diff --git a/public/app/plugins/datasource/loki/datasource.ts b/public/app/plugins/datasource/loki/datasource.ts index cd4dba02ca8..095abaa655e 100644 --- a/public/app/plugins/datasource/loki/datasource.ts +++ b/public/app/plugins/datasource/loki/datasource.ts @@ -30,6 +30,7 @@ import { QueryHint, rangeUtil, ScopedVars, + SupplementaryQueryOptions, TimeRange, LogRowContextOptions, } from '@grafana/data'; @@ -171,8 +172,8 @@ export class LokiDatasource return [SupplementaryQueryType.LogsVolume, SupplementaryQueryType.LogsSample]; } - getSupplementaryQuery(type: SupplementaryQueryType, query: LokiQuery): LokiQuery | undefined { - if (!this.getSupportedSupplementaryQueryTypes().includes(type)) { + getSupplementaryQuery(options: SupplementaryQueryOptions, query: LokiQuery): LokiQuery | undefined { + if (!this.getSupportedSupplementaryQueryTypes().includes(options.type)) { return undefined; } @@ -180,7 +181,7 @@ export class LokiDatasource const expr = removeCommentsFromQuery(normalizedQuery.expr); let isQuerySuitable = false; - switch (type) { + switch (options.type) { case SupplementaryQueryType.LogsVolume: // it has to be a logs-producing range-query isQuerySuitable = !!(query.expr && isLogsQuery(query.expr) && query.queryType === LokiQueryType.Range); @@ -206,7 +207,7 @@ export class LokiDatasource ...normalizedQuery, refId: `${REF_ID_STARTER_LOG_SAMPLE}${normalizedQuery.refId}`, expr: getLogQueryFromMetricsQuery(expr), - maxLines: 100, + maxLines: Number.isNaN(Number(options.limit)) ? this.maxLines : Number(options.limit), }; default: @@ -217,7 +218,7 @@ export class LokiDatasource getLogsVolumeDataProvider(request: DataQueryRequest): Observable | undefined { const logsVolumeRequest = cloneDeep(request); const targets = logsVolumeRequest.targets - .map((query) => this.getSupplementaryQuery(SupplementaryQueryType.LogsVolume, query)) + .map((query) => this.getSupplementaryQuery({ type: SupplementaryQueryType.LogsVolume }, query)) .filter((query): query is LokiQuery => !!query); if (!targets.length) { @@ -238,7 +239,7 @@ export class LokiDatasource getLogsSampleDataProvider(request: DataQueryRequest): Observable | undefined { const logsSampleRequest = cloneDeep(request); const targets = logsSampleRequest.targets - .map((query) => this.getSupplementaryQuery(SupplementaryQueryType.LogsSample, query)) + .map((query) => this.getSupplementaryQuery({ type: SupplementaryQueryType.LogsSample, limit: 100 }, query)) .filter((query): query is LokiQuery => !!query); if (!targets.length) {