From 46f331e2848e9d85f61bf074aa637707e71ddffd Mon Sep 17 00:00:00 2001 From: Matias Chomicki Date: Fri, 27 Oct 2023 13:00:49 +0200 Subject: [PATCH] Logs: remove toggleLabelsInLogsUI (#77264) * toggleLabelsInLogsUI: remove flag * Remove unused imports * isFilterLabelActive: refId is not optional * Revert "isFilterLabelActive: refId is not optional" This reverts commit 008931b7e9d068c34482ced164a42bd59160e850. * Revert method signature change * Update tests * Update tests --- .../feature-toggles/index.md | 1 - .../src/types/featureToggles.gen.ts | 1 - pkg/services/featuremgmt/registry.go | 8 ----- pkg/services/featuremgmt/toggles_gen.csv | 1 - pkg/services/featuremgmt/toggles_gen.go | 4 --- public/app/features/explore/Explore.tsx | 7 ++-- .../logs/components/LogDetails.test.tsx | 13 ++----- .../logs/components/LogDetailsRow.test.tsx | 34 +++++-------------- .../logs/components/LogDetailsRow.tsx | 22 +++++------- 9 files changed, 22 insertions(+), 69 deletions(-) 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 7e091a115e2..8ccd3dbb81e 100644 --- a/docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md +++ b/docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md @@ -48,7 +48,6 @@ Some features are enabled by default. You can disable these feature by setting t | `cloudWatchLogsMonacoEditor` | Enables the Monaco editor for CloudWatch Logs queries | Yes | | `recordedQueriesMulti` | Enables writing multiple items from a single query within Recorded Queries | Yes | | `transformationsRedesign` | Enables the transformations redesign | Yes | -| `toggleLabelsInLogsUI` | Enable toggleable filters in log details view | Yes | | `azureMonitorDataplane` | Adds dataplane compliant frame metadata in the Azure Monitor datasource | Yes | | `prometheusConfigOverhaulAuth` | Update the Prometheus configuration page with the new auth component | Yes | | `dashgpt` | Enable AI powered features in dashboards | Yes | diff --git a/packages/grafana-data/src/types/featureToggles.gen.ts b/packages/grafana-data/src/types/featureToggles.gen.ts index 8d905e8bfc1..1290d6503bb 100644 --- a/packages/grafana-data/src/types/featureToggles.gen.ts +++ b/packages/grafana-data/src/types/featureToggles.gen.ts @@ -103,7 +103,6 @@ export interface FeatureToggles { logsExploreTableVisualisation?: boolean; awsDatasourcesTempCredentials?: boolean; transformationsRedesign?: boolean; - toggleLabelsInLogsUI?: boolean; mlExpressions?: boolean; traceQLStreaming?: boolean; metricsSummary?: boolean; diff --git a/pkg/services/featuremgmt/registry.go b/pkg/services/featuremgmt/registry.go index d12d46d9dfe..89341a7077d 100644 --- a/pkg/services/featuremgmt/registry.go +++ b/pkg/services/featuremgmt/registry.go @@ -589,14 +589,6 @@ var ( Expression: "true", // enabled by default Owner: grafanaObservabilityMetricsSquad, }, - { - Name: "toggleLabelsInLogsUI", - Description: "Enable toggleable filters in log details view", - Stage: FeatureStageGeneralAvailability, - FrontendOnly: true, - Expression: "true", // enabled by default - 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 d94647febe3..00a269375c3 100644 --- a/pkg/services/featuremgmt/toggles_gen.csv +++ b/pkg/services/featuremgmt/toggles_gen.csv @@ -84,7 +84,6 @@ prometheusIncrementalQueryInstrumentation,experimental,@grafana/observability-me logsExploreTableVisualisation,experimental,@grafana/observability-logs,false,false,false,true awsDatasourcesTempCredentials,experimental,@grafana/aws-datasources,false,false,false,false transformationsRedesign,GA,@grafana/observability-metrics,false,false,false,true -toggleLabelsInLogsUI,GA,@grafana/observability-logs,false,false,false,true mlExpressions,experimental,@grafana/alerting-squad,false,false,false,false traceQLStreaming,experimental,@grafana/observability-traces-and-profiling,false,false,false,true metricsSummary,experimental,@grafana/observability-traces-and-profiling,false,false,false,true diff --git a/pkg/services/featuremgmt/toggles_gen.go b/pkg/services/featuremgmt/toggles_gen.go index 7cdc48e682a..304b9a8dfe1 100644 --- a/pkg/services/featuremgmt/toggles_gen.go +++ b/pkg/services/featuremgmt/toggles_gen.go @@ -347,10 +347,6 @@ 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 1fca13ba665..c41cbde0c0c 100644 --- a/public/app/features/explore/Explore.tsx +++ b/public/app/features/explore/Explore.tsx @@ -17,7 +17,7 @@ import { SupplementaryQueryType, } from '@grafana/data'; import { selectors } from '@grafana/e2e-selectors'; -import { config, getDataSourceSrv, reportInteraction } from '@grafana/runtime'; +import { getDataSourceSrv, reportInteraction } from '@grafana/runtime'; import { DataQuery } from '@grafana/schema'; import { AdHocFilterItem, @@ -206,9 +206,6 @@ export class Explore extends React.PureComponent { * @alpha */ isFilterLabelActive = async (key: string, value: string, refId?: string) => { - if (!config.featureToggles.toggleLabelsInLogsUI) { - return false; - } const query = this.props.queries.find((q) => q.refId === refId); if (!query) { return false; @@ -254,7 +251,7 @@ export class Explore extends React.PureComponent { return query; } const ds = await getDataSourceSrv().get(datasource); - if (hasToggleableQueryFiltersSupport(ds) && config.featureToggles.toggleLabelsInLogsUI) { + if (hasToggleableQueryFiltersSupport(ds)) { return ds.toggleQueryFilter(query, { type: modification.type === 'ADD_FILTER' ? 'FILTER_FOR' : 'FILTER_OUT', options: modification.options ?? {}, diff --git a/public/app/features/logs/components/LogDetails.test.tsx b/public/app/features/logs/components/LogDetails.test.tsx index 4d963491499..00e6aef9683 100644 --- a/public/app/features/logs/components/LogDetails.test.tsx +++ b/public/app/features/logs/components/LogDetails.test.tsx @@ -3,7 +3,6 @@ import userEvent from '@testing-library/user-event'; import React from 'react'; import { Field, LogLevel, LogRowModel, MutableDataFrame, createTheme, FieldType } from '@grafana/data'; -import { config } from '@grafana/runtime'; import { LogDetails, Props } from './LogDetails'; import { createLogRow } from './__mocks__/logRow'; @@ -57,16 +56,10 @@ describe('LogDetails', () => { }, { labels: { key1: 'label1' } } ); - expect(screen.getByLabelText('Filter for value')).toBeInTheDocument(); - expect(screen.getByLabelText('Filter out value')).toBeInTheDocument(); + expect(screen.getByLabelText('Filter for value in query A')).toBeInTheDocument(); + expect(screen.getByLabelText('Filter out value in query A')).toBeInTheDocument(); }); - describe('With toggleLabelsInLogsUI=true', () => { - beforeAll(() => { - config.featureToggles.toggleLabelsInLogsUI = true; - }); - afterAll(() => { - config.featureToggles.toggleLabelsInLogsUI = false; - }); + describe('Toggleable filters', () => { it('should provide the log row to Explore filter functions', async () => { const onClickFilterLabelMock = jest.fn(); const onClickFilterOutLabelMock = jest.fn(); diff --git a/public/app/features/logs/components/LogDetailsRow.test.tsx b/public/app/features/logs/components/LogDetailsRow.test.tsx index 55e01e99569..380c916655f 100644 --- a/public/app/features/logs/components/LogDetailsRow.test.tsx +++ b/public/app/features/logs/components/LogDetailsRow.test.tsx @@ -1,10 +1,8 @@ import { fireEvent, render, screen } from '@testing-library/react'; import React, { ComponentProps } from 'react'; -import { LogRowModel } from '@grafana/data'; -import config from 'app/core/config'; - import { LogDetailsRow } from './LogDetailsRow'; +import { createLogRow } from './__mocks__/logRow'; type Props = ComponentProps; @@ -20,7 +18,7 @@ const setup = (propOverrides?: Partial) => { onClickShowField: () => {}, onClickHideField: () => {}, displayedFields: [], - row: {} as LogRowModel, + row: createLogRow(), disableActions: false, }; @@ -55,32 +53,18 @@ describe('LogDetailsRow', () => { expect(screen.getAllByRole('button', { name: 'Ad-hoc statistics' })).toHaveLength(1); }); - describe('if props is a label', () => { - it('should render filter label button', () => { + describe('toggleable filters', () => { + it('should render filter buttons', () => { setup(); - expect(screen.getAllByRole('button', { name: 'Filter for value' })).toHaveLength(1); - expect(screen.queryByRole('button', { name: 'Remove filter' })).not.toBeInTheDocument(); + expect(screen.getAllByRole('button', { name: 'Filter for value in query A' })).toHaveLength(1); + expect(screen.getAllByRole('button', { name: 'Filter out value in query A' })).toHaveLength(1); + expect(screen.queryByRole('button', { name: 'Remove filter in query A' })).not.toBeInTheDocument(); }); - it('should render filter out label button', () => { - setup(); - expect(screen.getAllByRole('button', { name: 'Filter out value' })).toHaveLength(1); - }); - it('should render filter buttons when toggleLabelsInLogsUI false', async () => { + it('should render remove filter button when the filter is active', async () => { setup({ isFilterLabelActive: jest.fn().mockResolvedValue(true), }); - expect(screen.getByRole('button', { name: 'Filter for value' })).toBeInTheDocument(); - expect(screen.getByRole('button', { name: 'Filter out value' })).toBeInTheDocument(); - }); - - it('should render remove filter button when toggleLabelsInLogsUI true', async () => { - const defaultValue = config.featureToggles.toggleLabelsInLogsUI; - config.featureToggles.toggleLabelsInLogsUI = true; - setup({ - isFilterLabelActive: jest.fn().mockResolvedValue(true), - }); - expect(await screen.findByRole('button', { name: 'Remove filter' })).toBeInTheDocument(); - config.featureToggles.toggleLabelsInLogsUI = defaultValue; + expect(await screen.findByRole('button', { name: 'Remove filter in query A' })).toBeInTheDocument(); }); }); diff --git a/public/app/features/logs/components/LogDetailsRow.tsx b/public/app/features/logs/components/LogDetailsRow.tsx index 210b1fc71d3..b96dd64d38c 100644 --- a/public/app/features/logs/components/LogDetailsRow.tsx +++ b/public/app/features/logs/components/LogDetailsRow.tsx @@ -4,7 +4,7 @@ import memoizeOne from 'memoize-one'; import React, { PureComponent, useState } from 'react'; import { CoreApp, Field, GrafanaTheme2, IconName, LinkModel, LogLabelStatsModel, LogRowModel } from '@grafana/data'; -import { config, reportInteraction } from '@grafana/runtime'; +import { reportInteraction } from '@grafana/runtime'; import { ClipboardButton, DataLinkButton, IconButton, Themeable2, withTheme2 } from '@grafana/ui'; import { LogLabelStats } from './LogLabelStats'; @@ -257,8 +257,7 @@ class UnThemedLogDetailsRow extends PureComponent { const singleKey = parsedKeys == null ? false : parsedKeys.length === 1; const singleVal = parsedValues == null ? false : parsedValues.length === 1; const hasFilteringFunctionality = !disableActions && onClickFilterLabel && onClickFilterOutLabel; - const refIdTooltip = - config.featureToggles.toggleLabelsInLogsUI && row.dataFrame?.refId ? ` in query ${row.dataFrame?.refId}` : ''; + const refIdTooltip = row.dataFrame?.refId ? ` in query ${row.dataFrame?.refId}` : ''; const isMultiParsedValueWithNoContent = !singleVal && parsedValues != null && !parsedValues.every((val) => val === ''); @@ -277,17 +276,12 @@ class UnThemedLogDetailsRow extends PureComponent {
{hasFilteringFunctionality && ( <> - {config.featureToggles.toggleLabelsInLogsUI ? ( - // If we are using the new label toggling, we want to use the async icon button - - ) : ( - - )} +