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
2 years ago · 33186e3e23
parent 42cdec369d
commit 33186e3e23
7 changed files with 121 additions and 56 deletions
--- 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<TQuery extends DataQu
   * Returns a supplementary query to be used to fetch supplementary data based on the provided type and original query.
   * If provided query is not suitable for provided supplementary query type, undefined should be returned.
   */
-  getSupplementaryQuery(type: SupplementaryQueryType, query: TQuery): TQuery | undefined;
+  getSupplementaryQuery(options: SupplementaryQueryOptions, originalQuery: TQuery): TQuery | undefined;
 }

 export const hasSupplementaryQuerySupport = <TQuery extends DataQuery>(
--- 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) {
--- 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;
  }

--- 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: [
          {
--- 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<ElasticsearchQuery>): Observable<DataQueryResponse> | 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) {
--- 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);
      });
    });
--- 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<LokiQuery>): Observable<DataQueryResponse> | 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<LokiQuery>): Observable<DataQueryResponse> | 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) {