From 3d0895c008a782d904e25876532fe657e1937d63 Mon Sep 17 00:00:00 2001 From: Jocelyn Collado-Kuri Date: Wed, 31 Dec 2025 06:46:56 -0800 Subject: [PATCH] Fix the way we merge frontend and backend queries (#2158) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary With the new logic to merge backend and frontend query responses, some of the frontend query responses were being lost due to early termination. Merging frontend and backend responses was not properly waiting for all promises to resolve before returning this "merged" result ## Detailed explanation In `mergeQueries` although `Promise.all` was triggered, we returned immediately without waiting for the promises to actually resolve, now instead of passing down promises: - use `switchMap` so that the upstream backend response observable can be used as an inner Observable value to be concatenated with the rest of the responses - Why not use `map`? To prevent out of sync results, we need to wait for the promises to resolve **before** we pass it down to the `mergeQueries` function, `map` does not allow that. - use `from` and trigger `Promise.all` **before** calling `mergeQueries` so that all promise results can be resolved and converted to observables by the time they get passed into the function - modify `mergeQueries` to accept `DataResponse` for arguments, clone the existing data and return the merged result. Screenshot 2025-12-29 at 1 06 05 PM ## Why To fix how frontend and backend queries are merged so that no response gets lost in the process ## How to test 1. Create a new dashboard or go to Explore 2. For query type select `Problems` or anything that is not `Metrics` 3. Execute the query, and you should be able to see a response. --- .changeset/true-dingos-retire.md | 5 + src/datasource/datasource.ts | 49 ++++---- src/datasource/specs/datasource.test.ts | 149 ++++++++++++++++++++++++ 3 files changed, 181 insertions(+), 22 deletions(-) create mode 100644 .changeset/true-dingos-retire.md create mode 100644 src/datasource/specs/datasource.test.ts diff --git a/.changeset/true-dingos-retire.md b/.changeset/true-dingos-retire.md new file mode 100644 index 0000000..fb6716c --- /dev/null +++ b/.changeset/true-dingos-retire.md @@ -0,0 +1,5 @@ +--- +'grafana-zabbix': minor +--- + +Fix how frontend and backend querying responses are merged diff --git a/src/datasource/datasource.ts b/src/datasource/datasource.ts index b760aca..d937dff 100644 --- a/src/datasource/datasource.ts +++ b/src/datasource/datasource.ts @@ -35,7 +35,7 @@ import { } from '@grafana/data'; import { AnnotationQueryEditor } from './components/AnnotationQueryEditor'; import { trackRequest } from './tracking'; -import { lastValueFrom, map, Observable } from 'rxjs'; +import { from, lastValueFrom, map, Observable, switchMap } from 'rxjs'; export class ZabbixDatasource extends DataSourceWithBackend { name: string; @@ -142,10 +142,7 @@ export class ZabbixDatasource extends DataSourceWithBackend - this.mergeQueries(queryResponse, dbConnectionResponsePromise, frontendResponsePromise, annotationResposePromise); + const annotationResponsePromise = this.annotationRequest({ ...request, targets: interpolatedTargets }); const applyFEFuncs = (queryResponse: DataQueryResponse) => this.applyFrontendFunctions(queryResponse, { ...request, @@ -156,7 +153,13 @@ export class ZabbixDatasource extends DataSourceWithBackend + from(Promise.all([dbConnectionResponsePromise, frontendResponsePromise, annotationResponsePromise])).pipe( + map(([dbConnectionRes, frontendRes, annotationRes]) => + this.mergeQueries(queryResponse, dbConnectionRes, frontendRes, annotationRes) + ) + ) + ) ); } @@ -906,24 +909,26 @@ export class ZabbixDatasource extends DataSourceWithBackend, - frontendResponsePromise: Promise, - annotationResposePromise: Promise + dbConnectionResponse?: DataQueryResponse, + frontendResponse?: DataQueryResponse, + annotationResponse?: DataQueryResponse ): DataQueryResponse { - Promise.all([dbConnectionResponsePromise, frontendResponsePromise, annotationResposePromise]).then((resp) => { - const [dbConnectionRes, frontendRes, annotationRes] = resp; - if (dbConnectionRes.data) { - queryResponse.data = queryResponse.data.concat(dbConnectionRes.data); - } - if (frontendRes.data) { - queryResponse.data = queryResponse.data.concat(frontendRes.data); - } + const mergedResponse: DataQueryResponse = { + ...queryResponse, + data: queryResponse.data ? [...queryResponse.data] : [], + }; - if (annotationRes.data) { - queryResponse.data = queryResponse.data.concat(annotationRes.data); - } - }); - return queryResponse; + if (dbConnectionResponse?.data) { + mergedResponse.data = mergedResponse.data.concat(dbConnectionResponse.data); + } + if (frontendResponse?.data) { + mergedResponse.data = mergedResponse.data.concat(frontendResponse.data); + } + if (annotationResponse?.data) { + mergedResponse.data = mergedResponse.data.concat(annotationResponse.data); + } + + return mergedResponse; } convertToWide(response: DataQueryResponse) { diff --git a/src/datasource/specs/datasource.test.ts b/src/datasource/specs/datasource.test.ts new file mode 100644 index 0000000..2865dcb --- /dev/null +++ b/src/datasource/specs/datasource.test.ts @@ -0,0 +1,149 @@ +import { lastValueFrom, of } from 'rxjs'; +import { ZabbixDatasource } from '../datasource'; +import * as c from '../constants'; +import { DataSourceWithBackend } from '@grafana/runtime'; + +const buildRequest = () => + ({ + targets: [{ refId: 'A', queryType: c.MODE_METRICS }], + range: { from: 'now-1h', to: 'now' }, + scopedVars: {}, + }) as any; + +const createDeferred = () => { + let resolve!: (value: T | PromiseLike) => void; + const promise = new Promise((res) => { + resolve = res; + }); + return { promise, resolve }; +}; + +jest.mock('../tracking', () => ({ + trackRequest: jest.fn(), +})); + +jest.mock('../responseHandler', () => ({ + __esModule: true, + default: { + convertZabbixUnits: (resp: any) => resp, + convertToWide: (data: any) => data, + isConvertibleToWide: () => false, + }, +})); + +jest.mock('../zabbix/zabbix', () => ({ + Zabbix: jest.fn().mockImplementation(() => ({})), +})); + +jest.mock('grafana/app/core/config', () => ({ + buildInfo: { env: 'development' }, +})); + +jest.mock('grafana/app/core/core', () => ({ + contextSrv: {}, +})); + +jest.mock('grafana/app/core/utils/datemath', () => ({ + parse: () => Date.now(), +})); + +jest.mock('@grafana/runtime', () => { + const { of } = require('rxjs'); + + class MockDataSourceWithBackend { + instanceSettings: any; + constructor(settings: any) { + this.instanceSettings = settings; + } + + query() { + return of({ data: [] }); + } + } + + return { + DataSourceWithBackend: MockDataSourceWithBackend, + getTemplateSrv: jest.fn(() => ({ + replace: (value: any) => value, + variableExists: () => false, + })), + getDataSourceSrv: jest.fn(() => ({ + getInstanceSettings: () => undefined, + })), + getBackendSrv: jest.fn(), + HealthCheckError: class {}, + TemplateSrv: class {}, + }; +}); + +describe('ZabbixDatasource', () => { + const instanceSettings: any = { id: 1, name: 'test-ds', jsonData: {} }; + const ds = new ZabbixDatasource(instanceSettings); + it('waits for all non-backend responses before emitting merged data', async () => { + jest.spyOn(ZabbixDatasource.prototype, 'interpolateVariablesInQueries').mockReturnValue(buildRequest().targets); + jest.spyOn(ds, 'applyFrontendFunctions').mockImplementation((response) => response); + + jest.spyOn(DataSourceWithBackend.prototype, 'query').mockReturnValue(of({ data: [{ refId: 'A' }] as any[] })); + + const dbDeferred = createDeferred(); + const frontendDeferred = createDeferred(); + const annotationDeferred = createDeferred(); + + jest.spyOn(ds, 'dbConnectionQuery').mockReturnValue(dbDeferred.promise); + jest.spyOn(ds, 'frontendQuery').mockReturnValue(frontendDeferred.promise); + jest.spyOn(ds, 'annotationRequest').mockReturnValue(annotationDeferred.promise); + + const request = buildRequest(); + let settled = false; + const resultPromise = lastValueFrom(ds.query(request)).then((res) => { + settled = true; + return res; + }); + + await Promise.resolve(); + expect(settled).toBe(false); + + dbDeferred.resolve({ data: [{ refId: 'B' }] }); + frontendDeferred.resolve({ data: [{ refId: 'C' }] }); + annotationDeferred.resolve({ data: [{ refId: 'D' }] }); + + const result = await resultPromise; + expect(result.data).toEqual([{ refId: 'A' }, { refId: 'B' }, { refId: 'C' }, { refId: 'D' }]); + }); + + it('mergeQueries combines data without mutating the original response', () => { + const baseResponse = { data: [{ refId: 'A' }] } as any; + const merged = ds.mergeQueries( + baseResponse, + { data: [{ refId: 'B' }] } as any, + { data: [{ refId: 'C' }] } as any, + { data: [{ refId: 'D' }] } as any + ); + + expect(merged.data).toEqual([{ refId: 'A' }, { refId: 'B' }, { refId: 'C' }, { refId: 'D' }]); + expect(baseResponse.data).toEqual([{ refId: 'A' }]); + }); + + it('convertToWide delegates when data is convertible', () => { + const ds = new ZabbixDatasource(instanceSettings); + const response = { data: ['narrow'] } as any; + const result = ds.convertToWide(response); + expect(result.data).toEqual(['narrow']); + }); + + it('detects backend vs DB connection targets based on flag', () => { + const ds = new ZabbixDatasource(instanceSettings); + const metricsTarget = { queryType: c.MODE_METRICS } as any; + const itemIdTarget = { queryType: c.MODE_ITEMID } as any; + const problemsTarget = { queryType: c.MODE_PROBLEMS } as any; + + expect(ds.isBackendTarget(metricsTarget)).toBe(true); + expect(ds.isBackendTarget(itemIdTarget)).toBe(true); + expect(ds.isBackendTarget(problemsTarget)).toBe(false); + expect(ds.isDBConnectionTarget(metricsTarget)).toBe(false); + + ds.enableDirectDBConnection = true; + expect(ds.isBackendTarget(metricsTarget)).toBe(false); + expect(ds.isDBConnectionTarget(metricsTarget)).toBe(true); + }); +});