diff --git a/docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md b/docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md index 8c913782ebc..1eddc8ff05b 100644 --- a/docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md +++ b/docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md @@ -124,6 +124,7 @@ Experimental features might be changed or removed without prior notice. | `logsExploreTableVisualisation` | A table visualisation for logs in Explore | | `awsDatasourcesTempCredentials` | Support temporary security credentials in AWS plugins for Grafana Cloud customers | | `transformationsRedesign` | Enables the transformations redesign | +| `toggleLabelsInLogsUI` | Enable toggleable filters in log details view | | `mlExpressions` | Enable support for Machine Learning in server-side expressions | | `disableTraceQLStreaming` | Disables the option to stream the response of TraceQL queries of the Tempo data source | | `grafanaAPIServer` | Enable Kubernetes API Server for Grafana resources | diff --git a/packages/grafana-data/src/types/featureToggles.gen.ts b/packages/grafana-data/src/types/featureToggles.gen.ts index f2282efdb5e..e3166e1b322 100644 --- a/packages/grafana-data/src/types/featureToggles.gen.ts +++ b/packages/grafana-data/src/types/featureToggles.gen.ts @@ -110,6 +110,7 @@ export interface FeatureToggles { logsExploreTableVisualisation?: boolean; awsDatasourcesTempCredentials?: boolean; transformationsRedesign?: boolean; + toggleLabelsInLogsUI?: boolean; mlExpressions?: boolean; disableTraceQLStreaming?: boolean; grafanaAPIServer?: boolean; diff --git a/packages/grafana-data/src/types/logs.ts b/packages/grafana-data/src/types/logs.ts index e71bb6be176..557c445162d 100644 --- a/packages/grafana-data/src/types/logs.ts +++ b/packages/grafana-data/src/types/logs.ts @@ -2,7 +2,7 @@ import { Observable } from 'rxjs'; import { DataQuery } from '@grafana/schema'; -import { Labels } from './data'; +import { KeyValue, Labels } from './data'; import { DataFrame } from './dataFrame'; import { DataQueryRequest, DataQueryResponse } from './datasource'; import { AbsoluteTimeRange } from './time'; @@ -263,3 +263,42 @@ export const hasLogsContextUiSupport = (datasource: unknown): datasource is Data return withLogsSupport.getLogRowContextUi !== undefined; }; + +export interface QueryFilterOptions extends KeyValue {} +export interface ToggleFilterAction { + type: 'FILTER_FOR' | 'FILTER_OUT'; + options: QueryFilterOptions; +} +/** + * Data sources that support toggleable filters through `toggleQueryFilter`, and displaying the active + * state of filters through `queryHasFilter`, in the Log Details component in Explore. + * @internal + * @alpha + */ +export interface DataSourceWithToggleableQueryFiltersSupport { + /** + * Toggle filters on and off from query. + * If the filter is already present, it should be removed. + * If the opposite filter is present, it should be replaced. + */ + toggleQueryFilter(query: TQuery, filter: ToggleFilterAction): TQuery; + + /** + * Given a query, determine if it has a filter that matches the options. + */ + queryHasFilter(query: TQuery, filter: QueryFilterOptions): boolean; +} + +/** + * @internal + */ +export const hasToggleableQueryFiltersSupport = ( + datasource: unknown +): datasource is DataSourceWithToggleableQueryFiltersSupport => { + return ( + datasource !== null && + typeof datasource === 'object' && + 'toggleQueryFilter' in datasource && + 'queryHasFilter' in datasource + ); +}; diff --git a/pkg/services/featuremgmt/registry.go b/pkg/services/featuremgmt/registry.go index dd025a90319..7131f8699f4 100644 --- a/pkg/services/featuremgmt/registry.go +++ b/pkg/services/featuremgmt/registry.go @@ -628,6 +628,13 @@ var ( FrontendOnly: true, Owner: grafanaObservabilityMetricsSquad, }, + { + Name: "toggleLabelsInLogsUI", + Description: "Enable toggleable filters in log details view", + Stage: FeatureStageExperimental, + FrontendOnly: true, + Owner: grafanaObservabilityLogsSquad, + }, { Name: "mlExpressions", Description: "Enable support for Machine Learning in server-side expressions", diff --git a/pkg/services/featuremgmt/toggles_gen.csv b/pkg/services/featuremgmt/toggles_gen.csv index 56431e3f717..72e5973ea6d 100644 --- a/pkg/services/featuremgmt/toggles_gen.csv +++ b/pkg/services/featuremgmt/toggles_gen.csv @@ -91,6 +91,7 @@ prometheusIncrementalQueryInstrumentation,experimental,@grafana/observability-me logsExploreTableVisualisation,experimental,@grafana/observability-logs,false,false,false,true awsDatasourcesTempCredentials,experimental,@grafana/aws-datasources,false,false,false,false transformationsRedesign,experimental,@grafana/observability-metrics,false,false,false,true +toggleLabelsInLogsUI,experimental,@grafana/observability-logs,false,false,false,true mlExpressions,experimental,@grafana/alerting-squad,false,false,false,false disableTraceQLStreaming,experimental,@grafana/observability-traces-and-profiling,false,false,false,true grafanaAPIServer,experimental,@grafana/grafana-app-platform-squad,false,false,false,false diff --git a/pkg/services/featuremgmt/toggles_gen.go b/pkg/services/featuremgmt/toggles_gen.go index b6eda68dfae..cad80dbaee9 100644 --- a/pkg/services/featuremgmt/toggles_gen.go +++ b/pkg/services/featuremgmt/toggles_gen.go @@ -375,6 +375,10 @@ const ( // Enables the transformations redesign FlagTransformationsRedesign = "transformationsRedesign" + // FlagToggleLabelsInLogsUI + // Enable toggleable filters in log details view + FlagToggleLabelsInLogsUI = "toggleLabelsInLogsUI" + // FlagMlExpressions // Enable support for Machine Learning in server-side expressions FlagMlExpressions = "mlExpressions" diff --git a/public/app/features/explore/Explore.tsx b/public/app/features/explore/Explore.tsx index fe3d3ca8170..47d9c7430c9 100644 --- a/public/app/features/explore/Explore.tsx +++ b/public/app/features/explore/Explore.tsx @@ -15,6 +15,7 @@ import { EventBus, SplitOpenOptions, SupplementaryQueryType, + hasToggleableQueryFiltersSupport, } from '@grafana/data'; import { selectors } from '@grafana/e2e-selectors'; import { config, getDataSourceSrv, reportInteraction } from '@grafana/runtime'; @@ -183,10 +184,41 @@ export class Explore extends React.PureComponent { } }; + /** + * Used by Logs details. + * Returns true if all queries have the filter, otherwise false. + * TODO: In the future, we would like to return active filters based the query that produced the log line. + * @alpha + */ + isFilterLabelActive = async (key: string, value: string) => { + if (!config.featureToggles.toggleLabelsInLogsUI) { + return false; + } + if (this.props.queries.length === 0) { + return false; + } + for (const query of this.props.queries) { + const ds = await getDataSourceSrv().get(query.datasource); + if (!hasToggleableQueryFiltersSupport(ds)) { + return false; + } + if (!ds.queryHasFilter(query, { key, value })) { + return false; + } + } + return true; + }; + + /** + * Used by Logs details. + */ onClickFilterLabel = (key: string, value: string) => { this.onModifyQueries({ type: 'ADD_FILTER', options: { key, value } }); }; + /** + * Used by Logs details. + */ onClickFilterOutLabel = (key: string, value: string) => { this.onModifyQueries({ type: 'ADD_FILTER_OUT', options: { key, value } }); }; @@ -201,6 +233,9 @@ export class Explore extends React.PureComponent { makeAbsoluteTime(); }; + /** + * Used by Logs details. + */ onModifyQueries = (action: QueryFixAction) => { const modifier = async (query: DataQuery, modification: QueryFixAction) => { const { datasource } = query; @@ -208,6 +243,12 @@ export class Explore extends React.PureComponent { return query; } const ds = await getDataSourceSrv().get(datasource); + if (hasToggleableQueryFiltersSupport(ds) && config.featureToggles.toggleLabelsInLogsUI) { + return ds.toggleQueryFilter(query, { + type: modification.type === 'ADD_FILTER' ? 'FILTER_FOR' : 'FILTER_OUT', + options: modification.options ?? {}, + }); + } if (ds.modifyQuery) { return ds.modifyQuery(query, modification); } else { @@ -369,6 +410,7 @@ export class Explore extends React.PureComponent { eventBus={this.logsEventBus} splitOpenFn={this.onSplitOpen('logs')} scrollElement={this.scrollElement} + isFilterLabelActive={this.isFilterLabelActive} /> ); } diff --git a/public/app/features/explore/Logs/Logs.test.tsx b/public/app/features/explore/Logs/Logs.test.tsx index 1f8519f0b22..1770198d7f8 100644 --- a/public/app/features/explore/Logs/Logs.test.tsx +++ b/public/app/features/explore/Logs/Logs.test.tsx @@ -159,6 +159,7 @@ describe('Logs', () => { return []; }} eventBus={new EventBusSrv()} + isFilterLabelActive={jest.fn()} logsFrames={[testDataFrame]} {...partialProps} /> @@ -222,6 +223,7 @@ describe('Logs', () => { return []; }} eventBus={new EventBusSrv()} + isFilterLabelActive={jest.fn()} /> ); const button = screen.getByRole('button', { @@ -264,6 +266,7 @@ describe('Logs', () => { return []; }} eventBus={new EventBusSrv()} + isFilterLabelActive={jest.fn()} /> ); @@ -310,6 +313,7 @@ describe('Logs', () => { return []; }} eventBus={new EventBusSrv()} + isFilterLabelActive={jest.fn()} /> ); diff --git a/public/app/features/explore/Logs/Logs.tsx b/public/app/features/explore/Logs/Logs.tsx index c7913eec258..aa461951f34 100644 --- a/public/app/features/explore/Logs/Logs.tsx +++ b/public/app/features/explore/Logs/Logs.tsx @@ -95,6 +95,7 @@ interface Props extends Themeable2 { eventBus: EventBus; panelState?: ExplorePanelsState; scrollElement?: HTMLDivElement; + isFilterLabelActive: (key: string, value: string) => Promise; logsFrames?: DataFrame[]; range: TimeRange; } @@ -703,6 +704,7 @@ class UnthemedLogs extends PureComponent { onPermalinkClick={this.onPermalinkClick} permalinkedRowId={this.props.panelState?.logs?.id} scrollIntoView={this.scrollIntoView} + isFilterLabelActive={this.props.isFilterLabelActive} /> )} diff --git a/public/app/features/explore/Logs/LogsContainer.tsx b/public/app/features/explore/Logs/LogsContainer.tsx index 0a5b7391455..e964ffed1cc 100644 --- a/public/app/features/explore/Logs/LogsContainer.tsx +++ b/public/app/features/explore/Logs/LogsContainer.tsx @@ -52,6 +52,7 @@ interface LogsContainerProps extends PropsFromRedux { eventBus: EventBus; splitOpenFn: SplitOpen; scrollElement?: HTMLDivElement; + isFilterLabelActive: (key: string, value: string) => Promise; } class LogsContainer extends PureComponent { @@ -216,6 +217,7 @@ class LogsContainer extends PureComponent { panelState={this.props.panelState} logsFrames={this.props.logsFrames} scrollElement={scrollElement} + isFilterLabelActive={this.props.isFilterLabelActive} range={range} /> diff --git a/public/app/features/logs/components/LogDetails.tsx b/public/app/features/logs/components/LogDetails.tsx index 2f8a2cbba35..f4dad25f2fd 100644 --- a/public/app/features/logs/components/LogDetails.tsx +++ b/public/app/features/logs/components/LogDetails.tsx @@ -26,6 +26,7 @@ export interface Props extends Themeable2 { displayedFields?: string[]; onClickShowField?: (key: string) => void; onClickHideField?: (key: string) => void; + isFilterLabelActive?: (key: string, value: string) => Promise; } class UnThemedLogDetails extends PureComponent { @@ -103,6 +104,7 @@ class UnThemedLogDetails extends PureComponent { wrapLogMessage={wrapLogMessage} displayedFields={displayedFields} disableActions={false} + isFilterLabelActive={this.props.isFilterLabelActive} /> ); })} @@ -123,6 +125,7 @@ class UnThemedLogDetails extends PureComponent { row={row} app={app} disableActions={false} + isFilterLabelActive={this.props.isFilterLabelActive} /> ); })} diff --git a/public/app/features/logs/components/LogDetailsRow.test.tsx b/public/app/features/logs/components/LogDetailsRow.test.tsx index 17fbbc810fd..987322b5dbd 100644 --- a/public/app/features/logs/components/LogDetailsRow.test.tsx +++ b/public/app/features/logs/components/LogDetailsRow.test.tsx @@ -58,11 +58,18 @@ describe('LogDetailsRow', () => { it('should render filter label button', () => { setup(); expect(screen.getAllByRole('button', { name: 'Filter for value' })).toHaveLength(1); + expect(screen.queryByRole('button', { name: 'Remove filter' })).not.toBeInTheDocument(); }); it('should render filter out label button', () => { setup(); expect(screen.getAllByRole('button', { name: 'Filter out value' })).toHaveLength(1); }); + it('should render remove filter button', async () => { + setup({ + isFilterLabelActive: jest.fn().mockResolvedValue(true), + }); + expect(await screen.findByRole('button', { name: 'Remove filter' })).toBeInTheDocument(); + }); }); describe('if props is not a label', () => { diff --git a/public/app/features/logs/components/LogDetailsRow.tsx b/public/app/features/logs/components/LogDetailsRow.tsx index c560d44a068..18436e9d5a2 100644 --- a/public/app/features/logs/components/LogDetailsRow.tsx +++ b/public/app/features/logs/components/LogDetailsRow.tsx @@ -1,17 +1,15 @@ import { css, cx } from '@emotion/css'; import { isEqual } from 'lodash'; import memoizeOne from 'memoize-one'; -import React, { PureComponent } from 'react'; +import React, { PureComponent, useState } from 'react'; -import { CoreApp, Field, GrafanaTheme2, LinkModel, LogLabelStatsModel, LogRowModel } from '@grafana/data'; +import { CoreApp, Field, GrafanaTheme2, IconName, LinkModel, LogLabelStatsModel, LogRowModel } from '@grafana/data'; import { reportInteraction } from '@grafana/runtime'; import { ClipboardButton, DataLinkButton, IconButton, Themeable2, withTheme2 } from '@grafana/ui'; import { LogLabelStats } from './LogLabelStats'; import { getLogRowStyles } from './getLogRowStyles'; -//Components - export interface Props extends Themeable2 { parsedValues: string[]; parsedKeys: string[]; @@ -27,6 +25,7 @@ export interface Props extends Themeable2 { onClickHideField?: (key: string) => void; row: LogRowModel; app?: CoreApp; + isFilterLabelActive?: (key: string, value: string) => Promise; } interface State { @@ -134,6 +133,14 @@ class UnThemedLogDetailsRow extends PureComponent { }); }; + isFilterLabelActive = async () => { + const { isFilterLabelActive, parsedKeys, parsedValues } = this.props; + if (isFilterLabelActive) { + return await isFilterLabelActive(parsedKeys[0], parsedValues[0]); + } + return false; + }; + filterLabel = () => { const { onClickFilterLabel, parsedKeys, parsedValues, row } = this.props; if (onClickFilterLabel) { @@ -267,7 +274,7 @@ class UnThemedLogDetailsRow extends PureComponent {
{hasFilteringFunctionality && ( - + )} {hasFilteringFunctionality && ( @@ -330,5 +337,28 @@ class UnThemedLogDetailsRow extends PureComponent { } } +interface AsyncIconButtonProps extends Pick, 'onClick'> { + name: IconName; + isActive(): Promise; +} + +const AsyncIconButton = ({ isActive, ...rest }: AsyncIconButtonProps) => { + const [active, setActive] = useState(false); + + /** + * We purposely want to run this on every render to allow the active state to be updated + * when log details remains open between updates. + */ + isActive().then(setActive); + + return ( + + ); +}; + export const LogDetailsRow = withTheme2(UnThemedLogDetailsRow); LogDetailsRow.displayName = 'LogDetailsRow'; diff --git a/public/app/features/logs/components/LogRow.tsx b/public/app/features/logs/components/LogRow.tsx index 7302cf714d8..90c28510c64 100644 --- a/public/app/features/logs/components/LogRow.tsx +++ b/public/app/features/logs/components/LogRow.tsx @@ -42,6 +42,7 @@ interface Props extends Themeable2 { styles: LogRowStyles; permalinkedRowId?: string; scrollIntoView?: (element: HTMLElement) => void; + isFilterLabelActive?: (key: string, value: string) => Promise; onPinLine?: (row: LogRowModel) => void; onUnpinLine?: (row: LogRowModel) => void; pinned?: boolean; @@ -274,6 +275,7 @@ class UnThemedLogRow extends PureComponent { displayedFields={displayedFields} app={app} styles={styles} + isFilterLabelActive={this.props.isFilterLabelActive} /> )} diff --git a/public/app/features/logs/components/LogRows.tsx b/public/app/features/logs/components/LogRows.tsx index c1794c95caa..5ba3f224f3b 100644 --- a/public/app/features/logs/components/LogRows.tsx +++ b/public/app/features/logs/components/LogRows.tsx @@ -50,6 +50,7 @@ export interface Props extends Themeable2 { onPermalinkClick?: (row: LogRowModel) => Promise; permalinkedRowId?: string; scrollIntoView?: (element: HTMLElement) => void; + isFilterLabelActive?: (key: string, value: string) => Promise; pinnedRowId?: string; } @@ -144,6 +145,7 @@ class UnThemedLogRows extends PureComponent { onPinLine={this.props.onPinLine} onUnpinLine={this.props.onUnpinLine} pinned={this.props.pinnedRowId === row.uid} + isFilterLabelActive={this.props.isFilterLabelActive} {...rest} /> ))} @@ -164,6 +166,7 @@ class UnThemedLogRows extends PureComponent { onPinLine={this.props.onPinLine} onUnpinLine={this.props.onUnpinLine} pinned={this.props.pinnedRowId === row.uid} + isFilterLabelActive={this.props.isFilterLabelActive} {...rest} /> ))} diff --git a/public/app/plugins/datasource/elasticsearch/datasource.test.ts b/public/app/plugins/datasource/elasticsearch/datasource.test.ts index 1e9268aac21..88dbb47639a 100644 --- a/public/app/plugins/datasource/elasticsearch/datasource.test.ts +++ b/public/app/plugins/datasource/elasticsearch/datasource.test.ts @@ -1185,7 +1185,6 @@ describe('modifyQuery', () => { let ds: ElasticDatasource; beforeEach(() => { ds = getTestContext().ds; - config.featureToggles.elasticToggleableFilters = true; }); describe('with empty query', () => { let query: ElasticsearchQuery; @@ -1199,24 +1198,12 @@ describe('modifyQuery', () => { ); }); - it('should toggle the filter', () => { - query.query = 'foo:"bar"'; - expect(ds.modifyQuery(query, { type: 'ADD_FILTER', options: { key: 'foo', value: 'bar' } }).query).toBe(''); - }); - it('should add the negative filter', () => { expect(ds.modifyQuery(query, { type: 'ADD_FILTER_OUT', options: { key: 'foo', value: 'bar' } }).query).toBe( '-foo:"bar"' ); }); - it('should remove a positive filter to add a negative filter', () => { - query.query = 'foo:"bar"'; - expect(ds.modifyQuery(query, { type: 'ADD_FILTER_OUT', options: { key: 'foo', value: 'bar' } }).query).toBe( - '-foo:"bar"' - ); - }); - it('should do nothing on unknown type', () => { expect(ds.modifyQuery(query, { type: 'unknown', options: { key: 'foo', value: 'bar' } }).query).toBe(query.query); }); @@ -1244,29 +1231,82 @@ describe('modifyQuery', () => { expect(ds.modifyQuery(query, { type: 'unknown', options: { key: 'foo', value: 'bar' } }).query).toBe(query.query); }); }); +}); - describe('legacy behavior', () => { +describe('toggleQueryFilter', () => { + let ds: ElasticDatasource; + beforeEach(() => { + ds = getTestContext().ds; + config.featureToggles.elasticToggleableFilters = true; + }); + describe('with empty query', () => { + let query: ElasticsearchQuery; beforeEach(() => { - config.featureToggles.elasticToggleableFilters = false; + query = { query: '', refId: 'A' }; }); - it('should not modify other filters in the query', () => { - expect( - ds.modifyQuery( - { query: 'test:"value"', refId: 'A' }, - { type: 'ADD_FILTER', options: { key: 'test', value: 'value' } } - ).query - ).toBe('test:"value"'); - expect( - ds.modifyQuery( - { query: 'test:"value"', refId: 'A' }, - { type: 'ADD_FILTER_OUT', options: { key: 'test', value: 'value' } } - ).query - ).toBe('test:"value" AND -test:"value"'); + + it('should add the filter', () => { + expect(ds.toggleQueryFilter(query, { type: 'FILTER_FOR', options: { key: 'foo', value: 'bar' } }).query).toBe( + 'foo:"bar"' + ); + }); + + it('should toggle the filter', () => { + query.query = 'foo:"bar"'; + expect(ds.toggleQueryFilter(query, { type: 'FILTER_FOR', options: { key: 'foo', value: 'bar' } }).query).toBe(''); + }); + + it('should add the negative filter', () => { + expect(ds.toggleQueryFilter(query, { type: 'FILTER_OUT', options: { key: 'foo', value: 'bar' } }).query).toBe( + '-foo:"bar"' + ); + }); + + it('should remove a positive filter to add a negative filter', () => { + query.query = 'foo:"bar"'; + expect(ds.toggleQueryFilter(query, { type: 'FILTER_OUT', options: { key: 'foo', value: 'bar' } }).query).toBe( + '-foo:"bar"' + ); + }); + }); + + describe('with non-empty query', () => { + let query: ElasticsearchQuery; + beforeEach(() => { + query = { query: 'test:"value"', refId: 'A' }; + }); + + it('should add the filter', () => { + expect(ds.toggleQueryFilter(query, { type: 'FILTER_FOR', options: { key: 'foo', value: 'bar' } }).query).toBe( + 'test:"value" AND foo:"bar"' + ); + }); + + it('should add the negative filter', () => { + expect(ds.toggleQueryFilter(query, { type: 'FILTER_OUT', options: { key: 'foo', value: 'bar' } }).query).toBe( + 'test:"value" AND -foo:"bar"' + ); }); }); }); -describe('addAdHocFilters', () => { +describe('queryHasFilter()', () => { + let ds: ElasticDatasource; + beforeEach(() => { + ds = getTestContext().ds; + }); + it('inspects queries for filter presence', () => { + const query = { refId: 'A', query: 'grafana:"awesome"' }; + expect( + ds.queryHasFilter(query, { + key: 'grafana', + value: 'awesome', + }) + ).toBe(true); + }); +}); + +describe('addAdhocFilters', () => { describe('with invalid filters', () => { it('should filter out ad hoc filter without key', () => { const { ds, templateSrv } = getTestContext(); diff --git a/public/app/plugins/datasource/elasticsearch/datasource.ts b/public/app/plugins/datasource/elasticsearch/datasource.ts index 3f75a3cc0cb..9ee27a2019b 100644 --- a/public/app/plugins/datasource/elasticsearch/datasource.ts +++ b/public/app/plugins/datasource/elasticsearch/datasource.ts @@ -32,6 +32,9 @@ import { toUtc, AnnotationEvent, FieldType, + DataSourceWithToggleableQueryFiltersSupport, + QueryFilterOptions, + ToggleFilterAction, } from '@grafana/data'; import { DataSourceWithBackend, getDataSourceSrv, config, BackendSrvRequest } from '@grafana/runtime'; import { getTimeSrv, TimeSrv } from 'app/features/dashboard/services/TimeSrv'; @@ -90,7 +93,8 @@ export class ElasticDatasource implements DataSourceWithLogsContextSupport, DataSourceWithQueryImportSupport, - DataSourceWithSupplementaryQueriesSupport + DataSourceWithSupplementaryQueriesSupport, + DataSourceWithToggleableQueryFiltersSupport { basicAuth?: string; withCredentials?: boolean; @@ -892,41 +896,48 @@ export class ElasticDatasource return false; } + toggleQueryFilter(query: ElasticsearchQuery, filter: ToggleFilterAction): ElasticsearchQuery { + let expression = query.query ?? ''; + switch (filter.type) { + case 'FILTER_FOR': { + // This gives the user the ability to toggle a filter on and off. + expression = queryHasFilter(expression, filter.options.key, filter.options.value) + ? removeFilterFromQuery(expression, filter.options.key, filter.options.value) + : addFilterToQuery(expression, filter.options.key, filter.options.value); + break; + } + case 'FILTER_OUT': { + // If the opposite filter is present, remove it before adding the new one. + if (queryHasFilter(expression, filter.options.key, filter.options.value)) { + expression = removeFilterFromQuery(expression, filter.options.key, filter.options.value); + } + expression = addFilterToQuery(expression, filter.options.key, filter.options.value, '-'); + break; + } + } + + return { ...query, query: expression }; + } + + queryHasFilter(query: ElasticsearchQuery, options: QueryFilterOptions): boolean { + let expression = query.query ?? ''; + return queryHasFilter(expression, options.key, options.value); + } + modifyQuery(query: ElasticsearchQuery, action: QueryFixAction): ElasticsearchQuery { if (!action.options) { return query; } let expression = query.query ?? ''; - if (config.featureToggles.elasticToggleableFilters) { - switch (action.type) { - case 'ADD_FILTER': { - // This gives the user the ability to toggle a filter on and off. - expression = queryHasFilter(expression, action.options.key, action.options.value) - ? removeFilterFromQuery(expression, action.options.key, action.options.value) - : addFilterToQuery(expression, action.options.key, action.options.value); - break; - } - case 'ADD_FILTER_OUT': { - // If the opposite filter is present, remove it before adding the new one. - if (queryHasFilter(expression, action.options.key, action.options.value)) { - expression = removeFilterFromQuery(expression, action.options.key, action.options.value); - } - expression = addFilterToQuery(expression, action.options.key, action.options.value, '-'); - break; - } + switch (action.type) { + case 'ADD_FILTER': { + expression = addFilterToQuery(expression, action.options.key, action.options.value); + break; } - } else { - // Legacy behavior - switch (action.type) { - case 'ADD_FILTER': { - expression = addFilterToQuery(expression, action.options.key, action.options.value); - break; - } - case 'ADD_FILTER_OUT': { - expression = addFilterToQuery(expression, action.options.key, action.options.value, '-'); - break; - } + case 'ADD_FILTER_OUT': { + expression = addFilterToQuery(expression, action.options.key, action.options.value, '-'); + break; } } diff --git a/public/app/plugins/datasource/loki/datasource.test.ts b/public/app/plugins/datasource/loki/datasource.test.ts index 1eb10547c95..1cb884e9e8c 100644 --- a/public/app/plugins/datasource/loki/datasource.test.ts +++ b/public/app/plugins/datasource/loki/datasource.test.ts @@ -13,6 +13,7 @@ import { dateTime, FieldType, SupplementaryQueryType, + ToggleFilterAction, } from '@grafana/data'; import { BackendSrv, @@ -637,55 +638,22 @@ describe('LokiDatasource', () => { expect(result.refId).toEqual('A'); expect(result.expr).toEqual('rate({bar="baz", job="grafana"}[5m])'); }); - - describe('and the filter is already present', () => { - it('then it should remove the filter', () => { - const query: LokiQuery = { refId: 'A', expr: '{bar="baz", job="grafana"}' }; + describe('and query has parser', () => { + it('then the correct label should be added for logs query', () => { + const query: LokiQuery = { refId: 'A', expr: '{bar="baz"} | logfmt' }; const action = { options: { key: 'job', value: 'grafana' }, type: 'ADD_FILTER' }; const result = ds.modifyQuery(query, action); expect(result.refId).toEqual('A'); - expect(result.expr).toEqual('{bar="baz"}'); + expect(result.expr).toEqual('{bar="baz"} | logfmt | job=`grafana`'); }); - - it('then it should remove the filter with escaped value', () => { - const query: LokiQuery = { refId: 'A', expr: '{place="luna", job="\\"grafana/data\\""}' }; - const action = { options: { key: 'job', value: '"grafana/data"' }, type: 'ADD_FILTER' }; - const result = ds.modifyQuery(query, action); - - expect(result.refId).toEqual('A'); - expect(result.expr).toEqual('{place="luna"}'); - }); - }); - }); - - describe('and query has parser', () => { - it('then the correct label should be added for logs query', () => { - const query: LokiQuery = { refId: 'A', expr: '{bar="baz"} | logfmt' }; - const action = { options: { key: 'job', value: 'grafana' }, type: 'ADD_FILTER' }; - const result = ds.modifyQuery(query, action); - - expect(result.refId).toEqual('A'); - expect(result.expr).toEqual('{bar="baz"} | logfmt | job=`grafana`'); - }); - - it('then the correct label should be added for metrics query', () => { - const query: LokiQuery = { refId: 'A', expr: 'rate({bar="baz"} | logfmt [5m])' }; - const action = { options: { key: 'job', value: 'grafana' }, type: 'ADD_FILTER' }; - const result = ds.modifyQuery(query, action); - - expect(result.refId).toEqual('A'); - expect(result.expr).toEqual('rate({bar="baz"} | logfmt | job=`grafana` [5m])'); - }); - - describe('and the filter is already present', () => { - it('then it should remove the filter', () => { - const query: LokiQuery = { refId: 'A', expr: '{bar="baz"} | logfmt | job="grafana"' }; + it('then the correct label should be added for metrics query', () => { + const query: LokiQuery = { refId: 'A', expr: 'rate({bar="baz"} | logfmt [5m])' }; const action = { options: { key: 'job', value: 'grafana' }, type: 'ADD_FILTER' }; const result = ds.modifyQuery(query, action); expect(result.refId).toEqual('A'); - expect(result.expr).toEqual('{bar="baz"} | logfmt'); + expect(result.expr).toEqual('rate({bar="baz"} | logfmt | job=`grafana` [5m])'); }); }); }); @@ -724,12 +692,160 @@ describe('LokiDatasource', () => { expect(result.refId).toEqual('A'); expect(result.expr).toEqual('rate({bar="baz", job!="grafana"}[5m])'); }); + describe('and query has parser', () => { + let ds: LokiDatasource; + beforeEach(() => { + ds = createLokiDatasource(templateSrvStub); + }); + + it('then the correct label should be added for logs query', () => { + const query: LokiQuery = { refId: 'A', expr: '{bar="baz"} | logfmt' }; + const action = { options: { key: 'job', value: 'grafana' }, type: 'ADD_FILTER_OUT' }; + const result = ds.modifyQuery(query, action); + + expect(result.refId).toEqual('A'); + expect(result.expr).toEqual('{bar="baz"} | logfmt | job!=`grafana`'); + }); + it('then the correct label should be added for metrics query', () => { + const query: LokiQuery = { refId: 'A', expr: 'rate({bar="baz"} | logfmt [5m])' }; + const action = { options: { key: 'job', value: 'grafana' }, type: 'ADD_FILTER_OUT' }; + const result = ds.modifyQuery(query, action); + + expect(result.refId).toEqual('A'); + expect(result.expr).toEqual('rate({bar="baz"} | logfmt | job!=`grafana` [5m])'); + }); + }); + }); + }); + }); + + describe('toggleQueryFilter', () => { + describe('when called with FILTER', () => { + let ds: LokiDatasource; + beforeEach(() => { + ds = createLokiDatasource(templateSrvStub); + }); + + describe('and query has no parser', () => { + it('then the correct label should be added for logs query', () => { + const query: LokiQuery = { refId: 'A', expr: '{bar="baz"}' }; + const action: ToggleFilterAction = { options: { key: 'job', value: 'grafana' }, type: 'FILTER_FOR' }; + const result = ds.toggleQueryFilter(query, action); + + expect(result.refId).toEqual('A'); + expect(result.expr).toEqual('{bar="baz", job="grafana"}'); + }); + + it('then the correctly escaped label should be added for logs query', () => { + const query: LokiQuery = { refId: 'A', expr: '{bar="baz"}' }; + const action: ToggleFilterAction = { options: { key: 'job', value: '\\test' }, type: 'FILTER_FOR' }; + const result = ds.toggleQueryFilter(query, action); + + expect(result.refId).toEqual('A'); + expect(result.expr).toEqual('{bar="baz", job="\\\\test"}'); + }); + + it('then the correct label should be added for metrics query', () => { + const query: LokiQuery = { refId: 'A', expr: 'rate({bar="baz"}[5m])' }; + const action: ToggleFilterAction = { options: { key: 'job', value: 'grafana' }, type: 'FILTER_FOR' }; + const result = ds.toggleQueryFilter(query, action); + + expect(result.refId).toEqual('A'); + expect(result.expr).toEqual('rate({bar="baz", job="grafana"}[5m])'); + }); + + describe('and the filter is already present', () => { + it('then it should remove the filter', () => { + const query: LokiQuery = { refId: 'A', expr: '{bar="baz", job="grafana"}' }; + const action: ToggleFilterAction = { options: { key: 'job', value: 'grafana' }, type: 'FILTER_FOR' }; + const result = ds.toggleQueryFilter(query, action); + + expect(result.refId).toEqual('A'); + expect(result.expr).toEqual('{bar="baz"}'); + }); + + it('then it should remove the filter with escaped value', () => { + const query: LokiQuery = { refId: 'A', expr: '{place="luna", job="\\"grafana/data\\""}' }; + const action: ToggleFilterAction = { options: { key: 'job', value: '"grafana/data"' }, type: 'FILTER_FOR' }; + const result = ds.toggleQueryFilter(query, action); + + expect(result.refId).toEqual('A'); + expect(result.expr).toEqual('{place="luna"}'); + }); + }); + }); + + describe('and query has parser', () => { + it('then the correct label should be added for logs query', () => { + const query: LokiQuery = { refId: 'A', expr: '{bar="baz"} | logfmt' }; + const action: ToggleFilterAction = { options: { key: 'job', value: 'grafana' }, type: 'FILTER_FOR' }; + const result = ds.toggleQueryFilter(query, action); + + expect(result.refId).toEqual('A'); + expect(result.expr).toEqual('{bar="baz"} | logfmt | job=`grafana`'); + }); + + it('then the correct label should be added for metrics query', () => { + const query: LokiQuery = { refId: 'A', expr: 'rate({bar="baz"} | logfmt [5m])' }; + const action: ToggleFilterAction = { options: { key: 'job', value: 'grafana' }, type: 'FILTER_FOR' }; + const result = ds.toggleQueryFilter(query, action); + + expect(result.refId).toEqual('A'); + expect(result.expr).toEqual('rate({bar="baz"} | logfmt | job=`grafana` [5m])'); + }); + + describe('and the filter is already present', () => { + it('then it should remove the filter', () => { + const query: LokiQuery = { refId: 'A', expr: '{bar="baz"} | logfmt | job="grafana"' }; + const action: ToggleFilterAction = { options: { key: 'job', value: 'grafana' }, type: 'FILTER_FOR' }; + const result = ds.toggleQueryFilter(query, action); + + expect(result.refId).toEqual('A'); + expect(result.expr).toEqual('{bar="baz"} | logfmt'); + }); + }); + }); + }); + + describe('when called with FILTER_OUT', () => { + describe('and query has no parser', () => { + let ds: LokiDatasource; + beforeEach(() => { + ds = createLokiDatasource(templateSrvStub); + }); + + it('then the correct label should be added for logs query', () => { + const query: LokiQuery = { refId: 'A', expr: '{bar="baz"}' }; + const action: ToggleFilterAction = { options: { key: 'job', value: 'grafana' }, type: 'FILTER_OUT' }; + const result = ds.toggleQueryFilter(query, action); + + expect(result.refId).toEqual('A'); + expect(result.expr).toEqual('{bar="baz", job!="grafana"}'); + }); + + it('then the correctly escaped label should be added for logs query', () => { + const query: LokiQuery = { refId: 'A', expr: '{bar="baz"}' }; + const action: ToggleFilterAction = { options: { key: 'job', value: '"test' }, type: 'FILTER_OUT' }; + const result = ds.toggleQueryFilter(query, action); + + expect(result.refId).toEqual('A'); + expect(result.expr).toEqual('{bar="baz", job!="\\"test"}'); + }); + + it('then the correct label should be added for metrics query', () => { + const query: LokiQuery = { refId: 'A', expr: 'rate({bar="baz"}[5m])' }; + const action: ToggleFilterAction = { options: { key: 'job', value: 'grafana' }, type: 'FILTER_OUT' }; + const result = ds.toggleQueryFilter(query, action); + + expect(result.refId).toEqual('A'); + expect(result.expr).toEqual('rate({bar="baz", job!="grafana"}[5m])'); + }); describe('and the opposite filter is present', () => { it('then it should remove the filter', () => { const query: LokiQuery = { refId: 'A', expr: '{bar="baz", job="grafana"}' }; - const action = { options: { key: 'job', value: 'grafana' }, type: 'ADD_FILTER_OUT' }; - const result = ds.modifyQuery(query, action); + const action: ToggleFilterAction = { options: { key: 'job', value: 'grafana' }, type: 'FILTER_OUT' }; + const result = ds.toggleQueryFilter(query, action); expect(result.refId).toEqual('A'); expect(result.expr).toEqual('{bar="baz", job!="grafana"}'); @@ -745,8 +861,8 @@ describe('LokiDatasource', () => { it('then the correct label should be added for logs query', () => { const query: LokiQuery = { refId: 'A', expr: '{bar="baz"} | logfmt' }; - const action = { options: { key: 'job', value: 'grafana' }, type: 'ADD_FILTER_OUT' }; - const result = ds.modifyQuery(query, action); + const action: ToggleFilterAction = { options: { key: 'job', value: 'grafana' }, type: 'FILTER_OUT' }; + const result = ds.toggleQueryFilter(query, action); expect(result.refId).toEqual('A'); expect(result.expr).toEqual('{bar="baz"} | logfmt | job!=`grafana`'); @@ -754,8 +870,8 @@ describe('LokiDatasource', () => { it('then the correct label should be added for metrics query', () => { const query: LokiQuery = { refId: 'A', expr: 'rate({bar="baz"} | logfmt [5m])' }; - const action = { options: { key: 'job', value: 'grafana' }, type: 'ADD_FILTER_OUT' }; - const result = ds.modifyQuery(query, action); + const action: ToggleFilterAction = { options: { key: 'job', value: 'grafana' }, type: 'FILTER_OUT' }; + const result = ds.toggleQueryFilter(query, action); expect(result.refId).toEqual('A'); expect(result.expr).toEqual('rate({bar="baz"} | logfmt | job!=`grafana` [5m])'); @@ -764,8 +880,8 @@ describe('LokiDatasource', () => { describe('and the filter is already present', () => { it('then it should remove the filter', () => { const query: LokiQuery = { refId: 'A', expr: '{bar="baz"} | logfmt | job="grafana"' }; - const action = { options: { key: 'job', value: 'grafana' }, type: 'ADD_FILTER_OUT' }; - const result = ds.modifyQuery(query, action); + const action: ToggleFilterAction = { options: { key: 'job', value: 'grafana' }, type: 'FILTER_OUT' }; + const result = ds.toggleQueryFilter(query, action); expect(result.refId).toEqual('A'); expect(result.expr).toEqual('{bar="baz"} | logfmt | job!=`grafana`'); @@ -1371,6 +1487,22 @@ describe('showContextToggle()', () => { }); }); +describe('queryHasFilter()', () => { + let ds: LokiDatasource; + beforeEach(() => { + ds = createLokiDatasource(templateSrvStub); + }); + it('inspects queries for filter presence', () => { + const query = { refId: 'A', expr: '{grafana="awesome"}' }; + expect( + ds.queryHasFilter(query, { + key: 'grafana', + value: 'awesome', + }) + ).toBe(true); + }); +}); + function assertAdHocFilters(query: string, expectedResults: string, ds: LokiDatasource) { const lokiQuery: LokiQuery = { refId: 'A', expr: query }; const result = ds.addAdHocFilters(lokiQuery.expr); diff --git a/public/app/plugins/datasource/loki/datasource.ts b/public/app/plugins/datasource/loki/datasource.ts index 27203e6bb60..c35990ad887 100644 --- a/public/app/plugins/datasource/loki/datasource.ts +++ b/public/app/plugins/datasource/loki/datasource.ts @@ -33,6 +33,9 @@ import { SupplementaryQueryOptions, TimeRange, LogRowContextOptions, + DataSourceWithToggleableQueryFiltersSupport, + ToggleFilterAction, + QueryFilterOptions, } from '@grafana/data'; import { BackendSrvRequest, config, DataSourceWithBackend, FetchError } from '@grafana/runtime'; import { DataQuery } from '@grafana/schema'; @@ -131,7 +134,8 @@ export class LokiDatasource DataSourceWithLogsContextSupport, DataSourceWithSupplementaryQueriesSupport, DataSourceWithQueryImportSupport, - DataSourceWithQueryExportSupport + DataSourceWithQueryExportSupport, + DataSourceWithToggleableQueryFiltersSupport { private streams = new LiveStreams(); languageProvider: LanguageProvider; @@ -611,33 +615,61 @@ export class LokiDatasource return escapedValues.join('|'); } - modifyQuery(query: LokiQuery, action: QueryFixAction): LokiQuery { + toggleQueryFilter(query: LokiQuery, filter: ToggleFilterAction): LokiQuery { let expression = query.expr ?? ''; - switch (action.type) { - case 'ADD_FILTER': { - if (action.options?.key && action.options?.value) { - const value = escapeLabelValueInSelector(action.options.value); + switch (filter.type) { + case 'FILTER_FOR': { + if (filter.options?.key && filter.options?.value) { + const value = escapeLabelValueInSelector(filter.options.value); // This gives the user the ability to toggle a filter on and off. - expression = queryHasFilter(expression, action.options.key, '=', value) - ? removeLabelFromQuery(expression, action.options.key, '=', value) - : addLabelToQuery(expression, action.options.key, '=', value); + expression = queryHasFilter(expression, filter.options.key, '=', value) + ? removeLabelFromQuery(expression, filter.options.key, '=', value) + : addLabelToQuery(expression, filter.options.key, '=', value); } break; } - case 'ADD_FILTER_OUT': { - if (action.options?.key && action.options?.value) { - const value = escapeLabelValueInSelector(action.options.value); + case 'FILTER_OUT': { + if (filter.options?.key && filter.options?.value) { + const value = escapeLabelValueInSelector(filter.options.value); /** * If there is a filter with the same key and value, remove it. * This prevents the user from seeing no changes in the query when they apply * this filter. */ - if (queryHasFilter(expression, action.options.key, '=', value)) { - expression = removeLabelFromQuery(expression, action.options.key, '=', value); + if (queryHasFilter(expression, filter.options.key, '=', value)) { + expression = removeLabelFromQuery(expression, filter.options.key, '=', value); } + expression = addLabelToQuery(expression, filter.options.key, '!=', value); + } + break; + } + default: + break; + } + return { ...query, expr: expression }; + } + + queryHasFilter(query: LokiQuery, filter: QueryFilterOptions): boolean { + let expression = query.expr ?? ''; + return queryHasFilter(expression, filter.key, '=', filter.value); + } + + modifyQuery(query: LokiQuery, action: QueryFixAction): LokiQuery { + let expression = query.expr ?? ''; + switch (action.type) { + case 'ADD_FILTER': { + if (action.options?.key && action.options?.value) { + const value = escapeLabelValueInSelector(action.options.value); + expression = addLabelToQuery(expression, action.options.key, '=', value); + } + break; + } + case 'ADD_FILTER_OUT': { + if (action.options?.key && action.options?.value) { + const value = escapeLabelValueInSelector(action.options.value); expression = addLabelToQuery(expression, action.options.key, '!=', value); } break; diff --git a/public/app/plugins/datasource/loki/modifyQuery.ts b/public/app/plugins/datasource/loki/modifyQuery.ts index e69a08b7ae8..3678a723ca8 100644 --- a/public/app/plugins/datasource/loki/modifyQuery.ts +++ b/public/app/plugins/datasource/loki/modifyQuery.ts @@ -16,6 +16,7 @@ import { Selector, UnwrapExpr, String, + PipelineStage, } from '@grafana/lezer-logql'; import { QueryBuilderLabelFilter } from '../prometheus/querybuilder/shared/types'; @@ -70,7 +71,7 @@ export function removeLabelFromQuery(query: string, key: string, operator: strin function removeLabelFilter(query: string, matcher: SyntaxNode): string { const pipelineStage = matcher.parent?.parent; - if (!pipelineStage) { + if (!pipelineStage || pipelineStage.type.id !== PipelineStage) { return query; } return (query.substring(0, pipelineStage.from) + query.substring(pipelineStage.to)).trim(); @@ -96,7 +97,7 @@ function removeSelector(query: string, matcher: SyntaxNode): string { return prefix + modeller.renderQuery(matchVisQuery.query) + suffix; } -function getMatchersWithFilter(query: string, key: string, operator: string, value: string): SyntaxNode[] { +function getMatchersWithFilter(query: string, label: string, operator: string, value: string): SyntaxNode[] { const tree = parser.parse(query); const matchers: SyntaxNode[] = []; tree.iterate({ @@ -114,7 +115,7 @@ function getMatchersWithFilter(query: string, key: string, operator: string, val return false; } const labelName = query.substring(labelNode.from, labelNode.to); - if (labelName !== key) { + if (labelName !== label) { return false; } const labelValue = query.substring(valueNode.from, valueNode.to);