From 14d01e2b6e939da335a62f926014fefd48fb0536 Mon Sep 17 00:00:00 2001 From: ismail simsek Date: Wed, 11 Oct 2023 15:18:56 +0200 Subject: [PATCH] Prometheus: Remove prometheusResourceBrowserCache feature toggle (#76172) * Remove prometheusResourceBrowserCache feature toggle * Remove prometheusResourceBrowserCache feature toggle from prometheus datasource * 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 - .../prometheus/configuration/PromSettings.tsx | 69 +++++---- .../datasource/prometheus/datasource.test.ts | 47 +----- .../datasource/prometheus/datasource.tsx | 5 - .../prometheus/language_provider.test.ts | 47 ------ .../prometheus/language_provider.ts | 145 +----------------- .../prometheus/metric_find_query.test.ts | 4 +- 11 files changed, 49 insertions(+), 283 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 8412aa950d1..1e1d429e4bf 100644 --- a/docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md +++ b/docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md @@ -35,7 +35,6 @@ Some features are enabled by default. You can disable these feature by setting t | `logsContextDatasourceUi` | Allow datasource to provide custom UI for context view | Yes | | `gcomOnlyExternalOrgRoleSync` | Prohibits a user from changing organization roles synced with Grafana Cloud auth provider | | | `prometheusMetricEncyclopedia` | Adds the metrics explorer component to the Prometheus query builder as an option in metric select | Yes | -| `prometheusResourceBrowserCache` | Displays browser caching options in Prometheus data source configuration | Yes | | `prometheusDataplane` | Changes responses to from Prometheus to be compliant with the dataplane specification. In particular it sets the numeric Field.Name from 'Value' to the value of the `__name__` label when present. | Yes | | `lokiMetricDataplane` | Changes metric responses from Loki to be compliant with the dataplane specification. | Yes | | `dataplaneFrontendFallback` | Support dataplane contract field name change for transformations and field name matchers where the name is different | Yes | diff --git a/packages/grafana-data/src/types/featureToggles.gen.ts b/packages/grafana-data/src/types/featureToggles.gen.ts index 4acfe8c7aed..35a753964bc 100644 --- a/packages/grafana-data/src/types/featureToggles.gen.ts +++ b/packages/grafana-data/src/types/featureToggles.gen.ts @@ -66,7 +66,6 @@ export interface FeatureToggles { gcomOnlyExternalOrgRoleSync?: boolean; prometheusMetricEncyclopedia?: boolean; timeSeriesTable?: boolean; - prometheusResourceBrowserCache?: boolean; influxdbBackendMigration?: boolean; clientTokenRotation?: boolean; prometheusDataplane?: boolean; diff --git a/pkg/services/featuremgmt/registry.go b/pkg/services/featuremgmt/registry.go index c42da0ee7bc..28354e32caf 100644 --- a/pkg/services/featuremgmt/registry.go +++ b/pkg/services/featuremgmt/registry.go @@ -331,14 +331,6 @@ var ( FrontendOnly: true, Owner: appO11ySquad, }, - { - Name: "prometheusResourceBrowserCache", - Description: "Displays browser caching options in Prometheus data source configuration", - Stage: FeatureStageGeneralAvailability, - FrontendOnly: true, - Expression: "true", // turned on by default - Owner: grafanaObservabilityMetricsSquad, - }, { Name: "influxdbBackendMigration", Description: "Query InfluxDB InfluxQL without the proxy", diff --git a/pkg/services/featuremgmt/toggles_gen.csv b/pkg/services/featuremgmt/toggles_gen.csv index 0409ef6b370..651b7a48fa9 100644 --- a/pkg/services/featuremgmt/toggles_gen.csv +++ b/pkg/services/featuremgmt/toggles_gen.csv @@ -47,7 +47,6 @@ individualCookiePreferences,experimental,@grafana/backend-platform,false,false,f gcomOnlyExternalOrgRoleSync,GA,@grafana/grafana-authnz-team,false,false,false,false prometheusMetricEncyclopedia,GA,@grafana/observability-metrics,false,false,false,true timeSeriesTable,experimental,@grafana/app-o11y,false,false,false,true -prometheusResourceBrowserCache,GA,@grafana/observability-metrics,false,false,false,true influxdbBackendMigration,preview,@grafana/observability-metrics,false,false,false,true clientTokenRotation,experimental,@grafana/grafana-authnz-team,false,false,false,false prometheusDataplane,GA,@grafana/observability-metrics,false,false,false,false diff --git a/pkg/services/featuremgmt/toggles_gen.go b/pkg/services/featuremgmt/toggles_gen.go index 95da20c5195..820b0db4bdf 100644 --- a/pkg/services/featuremgmt/toggles_gen.go +++ b/pkg/services/featuremgmt/toggles_gen.go @@ -199,10 +199,6 @@ const ( // Enable time series table transformer & sparkline cell type FlagTimeSeriesTable = "timeSeriesTable" - // FlagPrometheusResourceBrowserCache - // Displays browser caching options in Prometheus data source configuration - FlagPrometheusResourceBrowserCache = "prometheusResourceBrowserCache" - // FlagInfluxdbBackendMigration // Query InfluxDB InfluxQL without the proxy FlagInfluxdbBackendMigration = "influxdbBackendMigration" diff --git a/public/app/plugins/datasource/prometheus/configuration/PromSettings.tsx b/public/app/plugins/datasource/prometheus/configuration/PromSettings.tsx index 7a5eae28637..4d0ce1931c8 100644 --- a/public/app/plugins/datasource/prometheus/configuration/PromSettings.tsx +++ b/public/app/plugins/datasource/prometheus/configuration/PromSettings.tsx @@ -12,7 +12,6 @@ import { ConfigSubSection } from '@grafana/experimental'; import { getBackendSrv } from '@grafana/runtime/src'; import { InlineField, Input, Select, Switch, useTheme2 } from '@grafana/ui'; -import config from '../../../../core/config'; import { useUpdateDatasource } from '../../../../features/datasources/state'; import { PromApplication, PromBuildInfoResponse } from '../../../../types/unified-alerting-dto'; import { QueryEditorMode } from '../querybuilder/shared/types'; @@ -198,7 +197,12 @@ export const PromSettings = (props: Props) => { spellCheck={false} placeholder="15s" onChange={onChangeHandler('timeInterval', options, onOptionsChange)} - onBlur={(e) => updateValidDuration({ ...validDuration, timeInterval: e.currentTarget.value })} + onBlur={(e) => + updateValidDuration({ + ...validDuration, + timeInterval: e.currentTarget.value, + }) + } /> {validateInput(validDuration.timeInterval, DURATION_REGEX, durationError)} @@ -222,7 +226,12 @@ export const PromSettings = (props: Props) => { onChange={onChangeHandler('queryTimeout', options, onOptionsChange)} spellCheck={false} placeholder="60s" - onBlur={(e) => updateValidDuration({ ...validDuration, queryTimeout: e.currentTarget.value })} + onBlur={(e) => + updateValidDuration({ + ...validDuration, + queryTimeout: e.currentTarget.value, + }) + } /> {validateInput(validDuration.queryTimeout, DURATION_REGEX, durationError)} @@ -362,33 +371,32 @@ export const PromSettings = (props: Props) => { )} - {config.featureToggles.prometheusResourceBrowserCache && ( -
-
- - Sets the browser caching level for editor queries. Higher cache settings are recommended for high - cardinality data sources. - + +
+
+ + Sets the browser caching level for editor queries. Higher cache settings are recommended for high + cardinality data sources. + + } + interactive={true} + disabled={options.readOnly} + > + o.value === options.jsonData.cacheLevel) ?? PrometheusCacheLevel.Low - } - /> - -
+ /> +
- )} +
@@ -431,7 +439,10 @@ export const PromSettings = (props: Props) => { <> - updateValidDuration({ ...validDuration, incrementalQueryOverlapWindow: e.currentTarget.value }) + updateValidDuration({ + ...validDuration, + incrementalQueryOverlapWindow: e.currentTarget.value, + }) } className="width-20" value={options.jsonData.incrementalQueryOverlapWindow ?? defaultPrometheusQueryOverlapWindow} diff --git a/public/app/plugins/datasource/prometheus/datasource.test.ts b/public/app/plugins/datasource/prometheus/datasource.test.ts index e717b5089be..afdaf6a19ff 100644 --- a/public/app/plugins/datasource/prometheus/datasource.test.ts +++ b/public/app/plugins/datasource/prometheus/datasource.test.ts @@ -14,14 +14,12 @@ import { getFieldDisplayName, LoadingState, toDataFrame, + VariableHide, } from '@grafana/data'; -import { config } from '@grafana/runtime'; import { TimeSrv } from 'app/features/dashboard/services/TimeSrv'; import { TemplateSrv } from 'app/features/templating/template_srv'; import { QueryOptions } from 'app/types'; -import { VariableHide } from '../../../features/variables/types'; - import { alignRange, extractRuleMappingFromGroups, @@ -440,50 +438,7 @@ describe('PrometheusDatasource', () => { }); }); - // Remove when prometheusResourceBrowserCache is removed - describe('When prometheusResourceBrowserCache feature flag is off, there should be no change to the query intervals ', () => { - beforeEach(() => { - config.featureToggles.prometheusResourceBrowserCache = false; - }); - - it('test default 1 minute quantization', () => { - const dataSource = new PrometheusDatasource( - { - ...instanceSettings, - jsonData: { ...instanceSettings.jsonData, cacheLevel: PrometheusCacheLevel.Low }, - }, - templateSrvStub as unknown as TemplateSrv, - timeSrvStub as unknown as TimeSrv - ); - const quantizedRange = dataSource.getAdjustedInterval(); - const oldRange = dataSource.getTimeRangeParams(); - // For "1 minute" the window is unchanged - expect(parseInt(quantizedRange.end, 10) - parseInt(quantizedRange.start, 10)).toBe(60); - expect(parseInt(oldRange.end, 10) - parseInt(oldRange.start, 10)).toBe(60); - }); - - it('test 10 minute quantization', () => { - const dataSource = new PrometheusDatasource( - { - ...instanceSettings, - jsonData: { ...instanceSettings.jsonData, cacheLevel: PrometheusCacheLevel.Medium }, - }, - templateSrvStub as unknown as TemplateSrv, - timeSrvStub as unknown as TimeSrv - ); - const quantizedRange = dataSource.getAdjustedInterval(); - const oldRange = dataSource.getTimeRangeParams(); - - expect(parseInt(quantizedRange.end, 10) - parseInt(quantizedRange.start, 10)).toBe(60); - expect(parseInt(oldRange.end, 10) - parseInt(oldRange.start, 10)).toBe(60); - }); - }); - describe('Test query range snapping', () => { - beforeEach(() => { - config.featureToggles.prometheusResourceBrowserCache = true; - }); - it('test default 1 minute quantization', () => { const dataSource = new PrometheusDatasource( { diff --git a/public/app/plugins/datasource/prometheus/datasource.tsx b/public/app/plugins/datasource/prometheus/datasource.tsx index bb0e2c01894..e5ea02a88e9 100644 --- a/public/app/plugins/datasource/prometheus/datasource.tsx +++ b/public/app/plugins/datasource/prometheus/datasource.tsx @@ -46,8 +46,6 @@ import { getTimeSrv, TimeSrv } from 'app/features/dashboard/services/TimeSrv'; import { getTemplateSrv, TemplateSrv } from 'app/features/templating/template_srv'; import { PromApiFeatures, PromApplication } from 'app/types/unified-alerting-dto'; -import config from '../../../core/config'; - import { addLabelToQuery } from './add_label_to_query'; import { AnnotationQueryEditor } from './components/AnnotationQueryEditor'; import PrometheusLanguageProvider from './language_provider'; @@ -1227,9 +1225,6 @@ export class PrometheusDatasource * Returns the adjusted "snapped" interval parameters */ getAdjustedInterval(): { start: string; end: string } { - if (!config.featureToggles.prometheusResourceBrowserCache) { - return this.getTimeRangeParams(); - } const range = this.timeSrv.timeRange(); return getRangeSnapInterval(this.cacheLevel, range); } diff --git a/public/app/plugins/datasource/prometheus/language_provider.test.ts b/public/app/plugins/datasource/prometheus/language_provider.test.ts index 406d2db26ac..51d821bc2fb 100644 --- a/public/app/plugins/datasource/prometheus/language_provider.test.ts +++ b/public/app/plugins/datasource/prometheus/language_provider.test.ts @@ -2,7 +2,6 @@ import { Editor as SlateEditor } from 'slate'; import Plain from 'slate-plain-serializer'; import { AbstractLabelOperator, dateTime, HistoryItem, TimeRange } from '@grafana/data'; -import { config } from '@grafana/runtime'; import { SearchFunctionType } from '@grafana/ui'; import { Label } from './components/monaco-query-field/monaco-completion-provider/situation'; @@ -98,53 +97,7 @@ describe('Language completion provider', () => { }); }); - // @todo clean up prometheusResourceBrowserCache feature flag - describe('getSeriesLabelsDeprecatedLRU', () => { - beforeEach(() => { - config.featureToggles.prometheusResourceBrowserCache = false; - }); - it('should call labels endpoint', () => { - const languageProvider = new LanguageProvider({ - ...defaultDatasource, - hasLabelsMatchAPISupport: () => true, - } as PrometheusDatasource); - const getSeriesLabels = languageProvider.getSeriesLabels; - const requestSpy = jest.spyOn(languageProvider, 'request'); - - const labelName = 'job'; - const labelValue = 'grafana'; - getSeriesLabels(`{${labelName}="${labelValue}"}`, [{ name: labelName, value: labelValue, op: '=' }] as Label[]); - expect(requestSpy).toHaveBeenCalled(); - expect(requestSpy).toHaveBeenCalledWith(`/api/v1/labels`, [], { - end: toPrometheusTimeString, - 'match[]': '{job="grafana"}', - start: fromPrometheusTimeString, - }); - }); - - it('should call series endpoint', () => { - const languageProvider = new LanguageProvider({ - ...defaultDatasource, - getAdjustedInterval: () => getRangeSnapInterval(PrometheusCacheLevel.None, getMockQuantizedTimeRangeParams()), - } as PrometheusDatasource); - const getSeriesLabels = languageProvider.getSeriesLabels; - const requestSpy = jest.spyOn(languageProvider, 'request'); - - const labelName = 'job'; - const labelValue = 'grafana'; - getSeriesLabels(`{${labelName}="${labelValue}"}`, [{ name: labelName, value: labelValue, op: '=' }] as Label[]); - expect(requestSpy).toHaveBeenCalled(); - expect(requestSpy).toHaveBeenCalledWith('/api/v1/series', [], { - end: toPrometheusTimeString, - 'match[]': '{job="grafana"}', - start: fromPrometheusTimeString, - }); - }); - }); describe('getSeriesLabels', () => { - beforeEach(() => { - config.featureToggles.prometheusResourceBrowserCache = true; - }); it('should call labels endpoint', () => { const languageProvider = new LanguageProvider({ ...defaultDatasource, diff --git a/public/app/plugins/datasource/prometheus/language_provider.ts b/public/app/plugins/datasource/prometheus/language_provider.ts index a1397517365..265b6332e02 100644 --- a/public/app/plugins/datasource/prometheus/language_provider.ts +++ b/public/app/plugins/datasource/prometheus/language_provider.ts @@ -1,5 +1,4 @@ import { chain, difference, once } from 'lodash'; -import { LRUCache } from 'lru-cache'; import Prism from 'prismjs'; import { Value } from 'slate'; @@ -11,7 +10,7 @@ import { HistoryItem, LanguageProvider, } from '@grafana/data'; -import { BackendSrvRequest, config } from '@grafana/runtime'; +import { BackendSrvRequest } from '@grafana/runtime'; import { CompletionItem, CompletionItemGroup, SearchFunctionType, TypeaheadInput, TypeaheadOutput } from '@grafana/ui'; import { Label } from './components/monaco-query-field/monaco-completion-provider/situation'; @@ -23,7 +22,6 @@ import { parseSelector, processHistogramMetrics, processLabels, - roundSecToMin, toPromLikeQuery, } from './language_utils'; import PromqlSyntax, { FUNCTIONS, RATE_RANGES } from './promql'; @@ -116,13 +114,7 @@ export default class PromQlLanguageProvider extends LanguageProvider { datasource: PrometheusDatasource; labelKeys: string[] = []; declare labelFetchTs: number; - /** - * Cache for labels of series. This is bit simplistic in the sense that it just counts responses each as a 1 and does - * not account for different size of a response. If that is needed a `length` function can be added in the options. - * 10 as a max size is totally arbitrary right now. - */ - private labelsCache = new LRUCache>({ max: 10 }); - private labelValuesCache = new LRUCache({ max: 10 }); + constructor(datasource: PrometheusDatasource, initialValues?: Partial) { super(); @@ -135,11 +127,8 @@ export default class PromQlLanguageProvider extends LanguageProvider { } getDefaultCacheHeaders() { - // @todo clean up prometheusResourceBrowserCache feature flag - if (config.featureToggles.prometheusResourceBrowserCache) { - if (this.datasource.cacheLevel !== PrometheusCacheLevel.None) { - return buildCacheHeaders(this.datasource.getCacheDurationInMinutes() * 60); - } + if (this.datasource.cacheLevel !== PrometheusCacheLevel.None) { + return buildCacheHeaders(this.datasource.getCacheDurationInMinutes() * 60); } return; } @@ -177,10 +166,7 @@ export default class PromQlLanguageProvider extends LanguageProvider { }; async loadMetricsMetadata() { - // @todo clean up prometheusResourceBrowserCache feature flag - const headers = config.featureToggles.prometheusResourceBrowserCache - ? buildCacheHeaders(this.datasource.getDaysToCacheMetadata() * secondsInDay) - : {}; + const headers = buildCacheHeaders(this.datasource.getDaysToCacheMetadata() * secondsInDay); this.metricsMetadata = fixSummariesMetadata( await this.request( '/api/v1/metadata', @@ -574,11 +560,6 @@ export default class PromQlLanguageProvider extends LanguageProvider { ...(interpolatedMatch && { 'match[]': interpolatedMatch }), }; - // @todo clean up prometheusResourceBrowserCache feature flag - if (!config.featureToggles.prometheusResourceBrowserCache) { - return await this.fetchSeriesValuesLRUCache(interpolatedName, range, name, urlParams); - } - const value = await this.request( `/api/v1/label/${interpolatedName}/values`, [], @@ -588,39 +569,6 @@ export default class PromQlLanguageProvider extends LanguageProvider { return value ?? []; }; - /** - * @deprecated - * @todo clean up prometheusResourceBrowserCache feature flag - * @param interpolatedName - * @param range - * @param name - * @param urlParams - * @private - */ - private async fetchSeriesValuesLRUCache( - interpolatedName: string | null, - range: { start: string; end: string }, - name: string, - urlParams: { start: string; end: string } - ) { - const cacheParams = new URLSearchParams({ - 'match[]': interpolatedName ?? '', - start: roundSecToMin(parseInt(range.start, 10)).toString(), - end: roundSecToMin(parseInt(range.end, 10)).toString(), - name: name, - }); - - const cacheKey = `/api/v1/label/?${cacheParams.toString()}/values`; - let value: string[] | undefined = this.labelValuesCache.get(cacheKey); - if (!value) { - value = await this.request(`/api/v1/label/${interpolatedName}/values`, [], urlParams); - if (value) { - this.labelValuesCache.set(cacheKey, value); - } - } - return value ?? []; - } - /** * Gets series labels * Function to replace old getSeries calls in a way that will provide faster endpoints for new prometheus instances, @@ -662,53 +610,11 @@ export default class PromQlLanguageProvider extends LanguageProvider { }; const url = `/api/v1/series`; - if (!config.featureToggles.prometheusResourceBrowserCache) { - return await this.fetchSeriesLabelsLRUCache(interpolatedName, range, withName, url, urlParams); - } - const data = await this.request(url, [], urlParams, this.getDefaultCacheHeaders()); const { values } = processLabels(data, withName); return values; }; - /** - * @deprecated - * @param interpolatedName - * @param range - * @param withName - * @param url - * @param urlParams - * @private - */ - private async fetchSeriesLabelsLRUCache( - interpolatedName: string, - range: { start: string; end: string }, - withName: boolean | undefined, - url: string, - urlParams: { start: string; 'match[]': string; end: string } - ) { - // Cache key is a bit different here. We add the `withName` param and also round up to a minute the intervals. - // The rounding may seem strange but makes relative intervals like now-1h less prone to need separate request every - // millisecond while still actually getting all the keys for the correct interval. This still can create problems - // when user does not the newest values for a minute if already cached. - const cacheParams = new URLSearchParams({ - 'match[]': interpolatedName, - start: roundSecToMin(parseInt(range.start, 10)).toString(), - end: roundSecToMin(parseInt(range.end, 10)).toString(), - withName: withName ? 'true' : 'false', - }); - - const cacheKey = `/api/v1/series?${cacheParams.toString()}`; - let value = this.labelsCache.get(cacheKey); - if (!value) { - const data = await this.request(url, [], urlParams); - const { values } = processLabels(data, withName); - value = values; - this.labelsCache.set(cacheKey, value); - } - return value; - } - /** * Fetch labels for a series using /labels endpoint. This is cached by its args but also by the global timeRange currently selected as * they can change over requested time. @@ -723,53 +629,12 @@ export default class PromQlLanguageProvider extends LanguageProvider { 'match[]': interpolatedName, }; const url = `/api/v1/labels`; - if (!config.featureToggles.prometheusResourceBrowserCache) { - return await this.fetchSeriesLabelMatchLRUCache(interpolatedName, range, withName, url, urlParams); - } const data: string[] = await this.request(url, [], urlParams, this.getDefaultCacheHeaders()); // Convert string array to Record return data.reduce((ac, a) => ({ ...ac, [a]: '' }), {}); }; - /** - * @deprecated - * @param interpolatedName - * @param range - * @param withName - * @param url - * @param urlParams - * @private - */ - private async fetchSeriesLabelMatchLRUCache( - interpolatedName: string, - range: { start: string; end: string }, - withName: boolean | undefined, - url: string, - urlParams: { start: string; 'match[]': string; end: string } - ) { - // Cache key is a bit different here. We add the `withName` param and also round up to a minute the intervals. - // The rounding may seem strange but makes relative intervals like now-1h less prone to need separate request every - // millisecond while still actually getting all the keys for the correct interval. This still can create problems - // when user does not the newest values for a minute if already cached. - const cacheParams = new URLSearchParams({ - 'match[]': interpolatedName, - start: roundSecToMin(parseInt(range.start, 10)).toString(), - end: roundSecToMin(parseInt(range.end, 10)).toString(), - withName: withName ? 'true' : 'false', - }); - - const cacheKey = `${url}?${cacheParams.toString()}`; - let value = this.labelsCache.get(cacheKey); - if (!value) { - const data: string[] = await this.request(url, [], urlParams); - // Convert string array to Record - value = data.reduce((ac, a) => ({ ...ac, [a]: '' }), {}); - this.labelsCache.set(cacheKey, value); - } - return value; - } - /** * Fetch series for a selector. Use this for raw results. Use fetchSeriesLabels() to get labels. * @param match diff --git a/public/app/plugins/datasource/prometheus/metric_find_query.test.ts b/public/app/plugins/datasource/prometheus/metric_find_query.test.ts index 8089abe9149..2acf59a6228 100644 --- a/public/app/plugins/datasource/prometheus/metric_find_query.test.ts +++ b/public/app/plugins/datasource/prometheus/metric_find_query.test.ts @@ -91,7 +91,9 @@ describe('PrometheusMetricFindQuery', () => { url: `/api/datasources/uid/ABCDEF/resources/api/v1/labels?start=${raw.from.unix()}&end=${raw.to.unix()}`, hideFromInspector: true, showErrorAlert: false, - headers: {}, + headers: { + 'X-Grafana-Cache': 'private, max-age=60', + }, }); });