From 058d3946b7642f8450e07ffe6c7ba56d32d60bc0 Mon Sep 17 00:00:00 2001 From: Ivana Huckova <30407135+ivanahuckova@users.noreply.github.com> Date: Tue, 28 Jan 2025 15:11:10 +0100 Subject: [PATCH] Zipkin: Remove frontend query running code (#99557) * Zipkin: Remove frontend query running code * Fix lint --- .betterer.results | 3 +- .../feature-toggles/index.md | 1 - .../src/types/featureToggles.gen.ts | 1 - pkg/services/featuremgmt/registry.go | 7 -- pkg/services/featuremgmt/toggles_gen.csv | 1 - pkg/services/featuremgmt/toggles_gen.go | 4 - pkg/services/featuremgmt/toggles_gen.json | 1 + .../datasource/zipkin/datasource.test.ts | 77 ++++++++----------- .../plugins/datasource/zipkin/datasource.ts | 66 +++------------- 9 files changed, 48 insertions(+), 113 deletions(-) diff --git a/.betterer.results b/.betterer.results index c08d07d5459..01d1a68627b 100644 --- a/.betterer.results +++ b/.betterer.results @@ -7550,8 +7550,7 @@ exports[`better eslint`] = { [0, 0, 0, "Unexpected any. Specify a different type.", "2"] ], "public/app/plugins/datasource/zipkin/datasource.ts:5381": [ - [0, 0, 0, "Do not use any type assertions.", "0"], - [0, 0, 0, "Unexpected any. Specify a different type.", "1"] + [0, 0, 0, "Do not use any type assertions.", "0"] ], "public/app/plugins/datasource/zipkin/utils/transforms.ts:5381": [ [0, 0, 0, "Unexpected any. Specify a different type.", "0"] 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 3038e1624b3..fecc19478b7 100644 --- a/docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md +++ b/docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md @@ -81,7 +81,6 @@ Most [generally available](https://grafana.com/docs/release-life-cycle/#general- | `userStorageAPI` | Enables the user storage API | Yes | | `azureMonitorDisableLogLimit` | Disables the log limit restriction for Azure Monitor when true. The limit is enabled by default. | | | `preinstallAutoUpdate` | Enables automatic updates for pre-installed plugins | Yes | -| `zipkinBackendMigration` | Enables querying Zipkin data source without the proxy | Yes | | `reportingUseRawTimeRange` | Uses the original report or dashboard time range instead of making an absolute transformation | Yes | | `alertingUIOptimizeReducer` | Enables removing the reducer from the alerting UI when creating a new alert rule and using instant query | Yes | | `azureMonitorEnableUserAuth` | Enables user auth for Azure Monitor datasource only | Yes | diff --git a/packages/grafana-data/src/types/featureToggles.gen.ts b/packages/grafana-data/src/types/featureToggles.gen.ts index 04d84ee202e..0a6775edac1 100644 --- a/packages/grafana-data/src/types/featureToggles.gen.ts +++ b/packages/grafana-data/src/types/featureToggles.gen.ts @@ -229,7 +229,6 @@ export interface FeatureToggles { exploreMetricsRelatedLogs?: boolean; prometheusSpecialCharsInLabelValues?: boolean; enableExtensionsAdminPage?: boolean; - zipkinBackendMigration?: boolean; enableSCIM?: boolean; crashDetection?: boolean; jaegerBackendMigration?: boolean; diff --git a/pkg/services/featuremgmt/registry.go b/pkg/services/featuremgmt/registry.go index a717b3bb707..a4baa2652bf 100644 --- a/pkg/services/featuremgmt/registry.go +++ b/pkg/services/featuremgmt/registry.go @@ -1587,13 +1587,6 @@ var ( Owner: grafanaPluginsPlatformSquad, RequiresRestart: true, }, - { - Name: "zipkinBackendMigration", - Description: "Enables querying Zipkin data source without the proxy", - Stage: FeatureStageGeneralAvailability, - Owner: grafanaOSSBigTent, - Expression: "true", // enabled by default - }, { Name: "enableSCIM", Description: "Enables SCIM support for user and group management", diff --git a/pkg/services/featuremgmt/toggles_gen.csv b/pkg/services/featuremgmt/toggles_gen.csv index 5a23134e827..34f40c2585d 100644 --- a/pkg/services/featuremgmt/toggles_gen.csv +++ b/pkg/services/featuremgmt/toggles_gen.csv @@ -210,7 +210,6 @@ passwordlessMagicLinkAuthentication,experimental,@grafana/identity-access-team,f exploreMetricsRelatedLogs,experimental,@grafana/observability-metrics,false,false,true prometheusSpecialCharsInLabelValues,experimental,@grafana/oss-big-tent,false,false,true enableExtensionsAdminPage,experimental,@grafana/plugins-platform-backend,false,true,false -zipkinBackendMigration,GA,@grafana/oss-big-tent,false,false,false enableSCIM,experimental,@grafana/identity-access-team,false,false,false crashDetection,experimental,@grafana/observability-traces-and-profiling,false,false,true jaegerBackendMigration,experimental,@grafana/oss-big-tent,false,false,false diff --git a/pkg/services/featuremgmt/toggles_gen.go b/pkg/services/featuremgmt/toggles_gen.go index 6ca1cd23fef..06bc4b00564 100644 --- a/pkg/services/featuremgmt/toggles_gen.go +++ b/pkg/services/featuremgmt/toggles_gen.go @@ -851,10 +851,6 @@ const ( // Enables the extension admin page regardless of development mode FlagEnableExtensionsAdminPage = "enableExtensionsAdminPage" - // FlagZipkinBackendMigration - // Enables querying Zipkin data source without the proxy - FlagZipkinBackendMigration = "zipkinBackendMigration" - // FlagEnableSCIM // Enables SCIM support for user and group management FlagEnableSCIM = "enableSCIM" diff --git a/pkg/services/featuremgmt/toggles_gen.json b/pkg/services/featuremgmt/toggles_gen.json index 0ac61ba717c..7ef236e5194 100644 --- a/pkg/services/featuremgmt/toggles_gen.json +++ b/pkg/services/featuremgmt/toggles_gen.json @@ -4062,6 +4062,7 @@ "name": "zipkinBackendMigration", "resourceVersion": "1733846643829", "creationTimestamp": "2024-11-07T09:35:53Z", + "deletionTimestamp": "2025-01-27T11:47:54Z", "annotations": { "grafana.app/updatedTimestamp": "2024-12-10 16:04:03.82919 +0000 UTC" } diff --git a/public/app/plugins/datasource/zipkin/datasource.test.ts b/public/app/plugins/datasource/zipkin/datasource.test.ts index 4123de7960d..5b1ab000004 100644 --- a/public/app/plugins/datasource/zipkin/datasource.test.ts +++ b/public/app/plugins/datasource/zipkin/datasource.test.ts @@ -1,5 +1,4 @@ import { lastValueFrom, of } from 'rxjs'; -import { createFetchResponse } from 'test/helpers/createFetchResponse'; import { DataFrameView, @@ -8,52 +7,49 @@ import { DataSourcePluginMeta, FieldType, } from '@grafana/data'; -import { BackendSrv, TemplateSrv } from '@grafana/runtime'; +import { BackendSrv, getBackendSrv, setBackendSrv, TemplateSrv } from '@grafana/runtime'; import { addNodeGraphFramesToResponse, ZipkinDatasource } from './datasource'; import mockJson from './mocks/mockJsonResponse.json'; import { mockTraceDataFrame } from './mocks/mockTraceDataFrame'; -import { ZipkinQuery, ZipkinSpan } from './types'; -import { traceFrameFields, zipkinResponse } from './utils/testData'; +import { ZipkinQuery } from './types'; -export const backendSrv = { fetch: jest.fn() } as unknown as BackendSrv; - -jest.mock('@grafana/runtime', () => ({ - ...jest.requireActual('@grafana/runtime'), - getBackendSrv: () => backendSrv, -})); +const templateSrv: TemplateSrv = { + replace: jest.fn().mockImplementation((value) => value), + getVariables: jest.fn(), + containsTemplate: jest.fn(), + updateTimeRange: jest.fn(), +}; describe('ZipkinDatasource', () => { + let origBackendSrv: BackendSrv; + let ds: ZipkinDatasource; + beforeEach(() => { + origBackendSrv = getBackendSrv(); + setBackendSrv({ ...origBackendSrv, fetch: jest.fn().mockReturnValue(of({ data: {} })) }); + ds = new ZipkinDatasource(defaultSettings, templateSrv); + }); + + afterEach(() => { + setBackendSrv(origBackendSrv); + jest.clearAllMocks(); + }); describe('query', () => { - const templateSrv: TemplateSrv = { - replace: jest.fn(), - getVariables: jest.fn(), - containsTemplate: jest.fn(), - updateTimeRange: jest.fn(), - }; - it('runs query', async () => { - setupBackendSrv(zipkinResponse); - const ds = new ZipkinDatasource(defaultSettings, templateSrv); - await expect(ds.query({ targets: [{ query: '12345' }] } as DataQueryRequest)).toEmitValuesWith( - (val) => { - expect(val[0].data[0].fields).toMatchObject(traceFrameFields); - } - ); - }); + const origBackendSrv = getBackendSrv(); + setBackendSrv({ + ...origBackendSrv, + fetch: jest.fn().mockReturnValue(of({ data: [] })), + }); - it('runs query with traceId that includes special characters', async () => { - setupBackendSrv(zipkinResponse); - const ds = new ZipkinDatasource(defaultSettings, templateSrv); - await expect(ds.query({ targets: [{ query: 'a/b' }] } as DataQueryRequest)).toEmitValuesWith( - (val) => { - expect(val[0].data[0].fields).toMatchObject(traceFrameFields); - } - ); + const response = await lastValueFrom(ds.query({ targets: [{ query: 'test' }] } as DataQueryRequest)); + expect(response).toEqual({ + state: 'Done', + data: [], + }); }); it('should handle json file upload', async () => { - const ds = new ZipkinDatasource(defaultSettings); ds.uploadedJson = JSON.stringify(mockJson); const response = await lastValueFrom( ds.query({ @@ -67,7 +63,6 @@ describe('ZipkinDatasource', () => { }); it('should fail on invalid json file upload', async () => { - const ds = new ZipkinDatasource(defaultSettings); ds.uploadedJson = JSON.stringify({ key: 'value', arr: [] }); const response = await lastValueFrom( ds.query({ @@ -81,7 +76,10 @@ describe('ZipkinDatasource', () => { describe('metadataRequest', () => { it('runs query', async () => { - setupBackendSrv(['service 1', 'service 2'] as unknown as ZipkinSpan[]); + setBackendSrv({ + ...origBackendSrv, + fetch: jest.fn().mockReturnValue(of({ data: ['service 1', 'service 2'] })), + }); const ds = new ZipkinDatasource(defaultSettings); const response = await ds.metadataRequest('services'); expect(response).toEqual(['service 1', 'service 2']); @@ -118,13 +116,6 @@ describe('addNodeGraphFramesToResponse', () => { }); }); -function setupBackendSrv(response: ZipkinSpan[]) { - const defaultMock = () => of(createFetchResponse(response)); - - const fetchMock = jest.spyOn(backendSrv, 'fetch'); - fetchMock.mockImplementation(defaultMock); -} - const defaultSettings: DataSourceInstanceSettings = { id: 1, uid: '1', diff --git a/public/app/plugins/datasource/zipkin/datasource.ts b/public/app/plugins/datasource/zipkin/datasource.ts index 54ff15dafb1..7b49e7a0c1c 100644 --- a/public/app/plugins/datasource/zipkin/datasource.ts +++ b/public/app/plugins/datasource/zipkin/datasource.ts @@ -1,4 +1,4 @@ -import { lastValueFrom, Observable, of } from 'rxjs'; +import { Observable, of } from 'rxjs'; import { map } from 'rxjs/operators'; import { @@ -9,26 +9,15 @@ import { FieldType, createDataFrame, ScopedVars, - urlUtil, toDataFrame, } from '@grafana/data'; import { createNodeGraphFrames, NodeGraphOptions, SpanBarOptions } from '@grafana/o11y-ds-frontend'; -import { - BackendSrvRequest, - config, - DataSourceWithBackend, - FetchResponse, - getBackendSrv, - getTemplateSrv, - TemplateSrv, -} from '@grafana/runtime'; +import { DataSourceWithBackend, getTemplateSrv, TemplateSrv } from '@grafana/runtime'; import { ZipkinQuery, ZipkinSpan } from './types'; import { createGraphFrames } from './utils/graphTransform'; import { transformResponse } from './utils/transforms'; -const apiPrefix = '/api/v2'; - export interface ZipkinJsonData extends DataSourceJsonData { nodeGraph?: NodeGraphOptions; } @@ -38,7 +27,7 @@ export class ZipkinDatasource extends DataSourceWithBackend, + instanceSettings: DataSourceInstanceSettings, private readonly templateSrv: TemplateSrv = getTemplateSrv() ) { super(instanceSettings); @@ -61,40 +50,24 @@ export class ZipkinDatasource extends DataSourceWithBackend { - if (this.nodeGraph?.enabled) { - return addNodeGraphFramesToResponse(response); - } - return response; - }) - ); - } - const query = this.applyTemplateVariables(target, options.scopedVars); - return this.request(`${apiPrefix}/trace/${encodeURIComponent(query.query)}`).pipe( - map((res) => responseToDataQueryResponse(res, this.nodeGraph?.enabled)) + return super.query(options).pipe( + map((response) => { + if (this.nodeGraph?.enabled) { + return addNodeGraphFramesToResponse(response); + } + return response; + }) ); } return of(emptyDataQueryResponse); } async metadataRequest(url: string, params?: Record) { - if (config.featureToggles.zipkinBackendMigration) { - return await this.getResource(url, params); - } - const urlWithPrefix = `${apiPrefix}/${url}`; - const res = await lastValueFrom(this.request(urlWithPrefix, params, { hideFromInspector: true })); - return res.data; + return await this.getResource(url, params); } async testDatasource(): Promise<{ status: string; message: string }> { - if (config.featureToggles.zipkinBackendMigration) { - return await super.testDatasource(); - } - - await this.metadataRequest('services'); - return { status: 'success', message: 'Data source is working' }; + return await super.testDatasource(); } getQueryDisplayText(query: ZipkinQuery): string { @@ -123,21 +96,6 @@ export class ZipkinDatasource extends DataSourceWithBackend( - apiUrl: string, - data?: unknown, - options?: Partial - ): Observable> { - const params = data ? urlUtil.serializeParams(data) : ''; - const url = `${this.instanceSettings.url}${apiUrl}${params.length ? `?${params}` : ''}`; - const req = { - ...options, - url, - }; - - return getBackendSrv().fetch(req); - } } function responseToDataQueryResponse(response: { data: ZipkinSpan[] }, nodeGraph = false): DataQueryResponse {