AdhocFilters: Pass filters via request object (#382)

packages/scenes-app/src/demos/adhocFiltersDemo.tsx

@@ -34,6 +34,7 @@ export function getAdhocFiltersDemo(defaults: SceneAppPageState) {
                 baseFilters: [{ key: '__name__', operator: '=', value: 'ALERTS', condition: '' }],
                 datasource: { uid: 'gdev-prometheus' },
               }),
+              ...getEmbeddedSceneDefaults().controls,
             ],
             body: new SceneFlexLayout({
               direction: 'row',

packages/scenes/src/querying/SceneQueryRunner.test.ts

@@ -25,9 +25,11 @@ import { SceneTimeRangeCompare } from '../components/SceneTimeRangeCompare';
 import { SceneDataLayers } from './SceneDataLayers';
 import { TestAnnotationsDataLayer } from './layers/TestDataLayer';
 import { TestSceneWithRequestEnricher } from '../utils/test/TestSceneWithRequestEnricher';
+import { AdHocFilterSet } from '../variables/adhoc/AdHocFiltersSet';
 
 const getDataSourceMock = jest.fn().mockReturnValue({
-  getRef: () => ({ uid: 'test' }),
+  uid: 'test-uid',
+  getRef: () => ({ uid: 'test-uid' }),
   query: () =>
     of({
       data: [
@@ -68,8 +70,14 @@ jest.mock('@grafana/runtime', () => ({
     return runRequestMock(ds, request);
   },
   getDataSourceSrv: () => {
-    return { get: getDataSourceMock };
+    return {
+      get: getDataSourceMock,
+      getInstanceSettings: () => ({ uid: 'test-uid' }),
+    };
   },
+  getTemplateSrv: () => ({
+    getAdhocFilters: jest.fn(),
+  }),
   config: {
     theme: {
       palette: {
@@ -132,7 +140,7 @@ describe('SceneQueryRunner', () => {
           "targets": [
             {
               "datasource": {
-                "uid": "test",
+                "uid": "test-uid",
               },
               "refId": "A",
             },
@@ -194,6 +202,44 @@ describe('SceneQueryRunner', () => {
       expect(runRequestCall[1].scopedVars.__sceneObject).toEqual({ value: queryRunner, text: '__sceneObject' });
       expect(getDataSourceCall[1].__sceneObject).toEqual({ value: queryRunner, text: '__sceneObject' });
     });
+
+    it('should pass adhoc filters via request object', async () => {
+      const queryRunner = new SceneQueryRunner({
+        datasource: { uid: 'test-uid' },
+        queries: [{ refId: 'A' }],
+      });
+
+      const filterSet = new AdHocFilterSet({
+        datasource: { uid: 'test-uid' },
+        filters: [{ key: 'A', operator: '=', value: 'B', condition: '' }],
+      });
+
+      new EmbeddedScene({
+        $data: queryRunner,
+        controls: [filterSet],
+        body: new SceneCanvasText({ text: 'hello' }),
+      });
+
+      expect(queryRunner.state.data).toBeUndefined();
+
+      queryRunner.activate();
+
+      await new Promise((r) => setTimeout(r, 1));
+
+      const runRequestCall = runRequestMock.mock.calls[0];
+
+      expect(runRequestCall[1].filters).toEqual(filterSet.state.filters);
+
+      // Verify updating filter re-triggers query
+      filterSet._updateFilter(filterSet.state.filters[0], 'value', 'newValue');
+
+      await new Promise((r) => setTimeout(r, 1));
+
+      expect(runRequestMock.mock.calls.length).toEqual(2);
+
+      const runRequestCall2 = runRequestMock.mock.calls[1];
+      expect(runRequestCall2[1].filters).toEqual(filterSet.state.filters);
+    });
   });
 
   describe('when container width changed during deactivation', () => {
@@ -1375,7 +1421,7 @@ describe('SceneQueryRunner', () => {
             // This function is faking annotation events with exclude filter
             return [
               {
-                // only this annotation should we returned
+                // this annotation should we returned
                 source: {
                   filter: {
                     exclude: true,

packages/scenes/src/querying/SceneQueryRunner.ts

@@ -11,10 +11,9 @@ import {
   PanelData,
   preProcessPanelData,
   rangeUtil,
-  ScopedVar,
   transformDataFrame,
 } from '@grafana/data';
-import { getRunRequest, toDataQueryError } from '@grafana/runtime';
+import { getDataSourceSrv, getRunRequest, toDataQueryError } from '@grafana/runtime';
 
 import { SceneObjectBase } from '../core/SceneObjectBase';
 import { sceneGraph } from '../core/sceneGraph';
@@ -37,6 +36,7 @@ import { getClosest } from '../core/sceneGraph/utils';
 import { timeShiftQueryResponseOperator } from './timeShiftQueryResponseOperator';
 import { filterAnnotationsOperator } from './layers/annotations/filterAnnotationsOperator';
 import { getEnrichedDataRequest } from './getEnrichedDataRequest';
+import { AdHocFilterSet } from '../variables/adhoc/AdHocFiltersSet';
 
 let counter = 100;
 
@@ -69,6 +69,11 @@ export class SceneQueryRunner extends SceneObjectBase<QueryRunnerState> implemen
   private _containerWidth?: number;
   private _variableValueRecorder = new VariableValueRecorder();
   private _results = new ReplaySubject<SceneDataProviderResult>();
+  private _scopedVars = { __sceneObject: { value: this, text: '__sceneObject' } };
+
+  // Closest filter set if found)
+  private _adhocFilterSet?: AdHocFilterSet;
+  private _adhocFilterSub?: Unsubscribable;
 
   public getResultsStream() {
     return this._results;
@@ -237,6 +242,10 @@ export class SceneQueryRunner extends SceneObjectBase<QueryRunnerState> implemen
       this._dataLayersSub = undefined;
     }
 
+    if (this._adhocFilterSub) {
+      this._adhocFilterSub.unsubscribe();
+    }
+
     this._variableValueRecorder.recordCurrentDependencyValuesForSceneObject(this);
   }
 
@@ -316,9 +325,6 @@ export class SceneQueryRunner extends SceneObjectBase<QueryRunnerState> implemen
     }
 
     const { queries } = this.state;
-    const sceneObjectScopedVar: Record<string, ScopedVar<SceneQueryRunner>> = {
-      __sceneObject: { text: '__sceneObject', value: this },
-    };
 
     // Simple path when no queries exist
     if (!queries?.length) {
@@ -328,7 +334,11 @@ export class SceneQueryRunner extends SceneObjectBase<QueryRunnerState> implemen
 
     try {
       const datasource = this.state.datasource ?? findFirstDatasource(queries);
-      const ds = await getDataSource(datasource, sceneObjectScopedVar);
+      const ds = await getDataSource(datasource, this._scopedVars);
+
+      if (!this._adhocFilterSet) {
+        this.findAndSubscribeToAdhocFilters(ds.uid);
+      }
 
       const [request, secondaryRequest] = this.prepareRequests(timeRange, ds);
 
@@ -364,9 +374,6 @@ export class SceneQueryRunner extends SceneObjectBase<QueryRunnerState> implemen
   ): [DataQueryRequest, DataQueryRequest | undefined] => {
     const comparer = this.getTimeCompare();
     const { minInterval, queries } = this.state;
-    const sceneObjectScopedVar: Record<string, ScopedVar<SceneQueryRunner>> = {
-      __sceneObject: { text: '__sceneObject', value: this },
-    };
 
     let secondaryRequest: DataQueryRequest | undefined;
 
@@ -380,7 +387,7 @@ export class SceneQueryRunner extends SceneObjectBase<QueryRunnerState> implemen
       intervalMs: 1000,
       targets: cloneDeep(queries),
       maxDataPoints: this.getMaxDataPoints(),
-      scopedVars: sceneObjectScopedVar,
+      scopedVars: this._scopedVars,
       startTime: Date.now(),
       liveStreaming: this.state.liveStreaming,
       rangeRaw: {
@@ -391,6 +398,11 @@ export class SceneQueryRunner extends SceneObjectBase<QueryRunnerState> implemen
       ...getEnrichedDataRequest(this),
     };
 
+    if (this._adhocFilterSet) {
+      // @ts-ignore (Temporary ignore until we update @grafana/data)
+      request.filters = this._adhocFilterSet.state.filters;
+    }
+
     request.targets = request.targets.map((query) => {
       if (!query.datasource) {
         query.datasource = ds.getRef();
@@ -486,6 +498,38 @@ export class SceneQueryRunner extends SceneObjectBase<QueryRunnerState> implemen
       return found;
     });
   }
+
+  /**
+   * Walk up scene graph and find the closest filterset with matching data source
+   */
+  private findAndSubscribeToAdhocFilters(ourDataSourceUid: string) {
+    const set = getClosest(this, (s) => {
+      let found = null;
+      if (s instanceof AdHocFilterSet && s.state.datasource?.uid === this.state.datasource?.uid) {
+        return s;
+      }
+      s.forEachChild((child) => {
+        if (child instanceof AdHocFilterSet && child.state.datasource?.uid === this.state.datasource?.uid) {
+          found = child;
+        }
+      });
+      return found;
+    });
+
+    if (!set || set.state.applyMode !== 'same-datasource') {
+      return;
+    }
+
+    const setDataSource = getDataSourceSrv().getInstanceSettings(set?.state.datasource, this._scopedVars);
+
+    if (setDataSource?.uid !== ourDataSourceUid) {
+      return;
+    }
+
+    // Subscribe to filter set state changes so that queries are re-issued when it changes
+    this._adhocFilterSet = set;
+    this._adhocFilterSub = this._adhocFilterSet?.subscribeToState(() => this.runQueries());
+  }
 }
 
 export function findFirstDatasource(targets: DataQuery[]): DataSourceRef | undefined {

packages/scenes/src/variables/adhoc/AdHocFiltersSet.test.tsx

@@ -62,15 +62,19 @@ describe('AdHocFilter', () => {
   it('changes filter', async () => {
     const { filtersSet, runRequest } = setup();
 
+    await new Promise((r) => setTimeout(r, 1));
+
+    // should run initial query
+    expect(runRequest.mock.calls.length).toBe(1);
+
     const wrapper = screen.getByTestId('AdHocFilter-key1');
     const selects = getAllByRole(wrapper, 'combobox');
 
     await waitFor(() => select(selects[2], 'val4', { container: document.body }));
 
+    // should run new query when filter changed
+    expect(runRequest.mock.calls.length).toBe(1);
     expect(filtersSet.state.filters[0].value).toBe('val4');
-
-    // should run query for scene query runner
-    expect(runRequest.mock.calls.length).toBe(2);
   });
 
   it('url sync works', async () => {

packages/scenes/src/variables/adhoc/AdHocFiltersSet.tsx

@@ -1,16 +1,9 @@
 import { SceneObjectBase } from '../../core/SceneObjectBase';
-import {
-  AdHocVariableFilter,
-  DataSourceInstanceSettings,
-  GrafanaTheme2,
-  MetricFindValue,
-  SelectableValue,
-} from '@grafana/data';
+import { AdHocVariableFilter, GrafanaTheme2, MetricFindValue, SelectableValue } from '@grafana/data';
 import { patchGetAdhocFilters } from './patchGetAdhocFilters';
 import { DataSourceRef } from '@grafana/schema';
 import { getDataSourceSrv } from '@grafana/runtime';
-import { SceneQueryRunner } from '../../querying/SceneQueryRunner';
-import { SceneComponentProps, SceneObject, SceneObjectState, SceneObjectUrlSyncHandler } from '../../core/types';
+import { SceneComponentProps, SceneObjectState, SceneObjectUrlSyncHandler } from '../../core/types';
 import { AdHocFiltersVariableUrlSyncHandler } from './AdHocFiltersVariableUrlSyncHandler';
 import { useStyles2 } from '@grafana/ui';
 import React from 'react';
@@ -98,7 +91,6 @@ export class AdHocFilterSet extends SceneObjectBase<AdHocFilterSetState> {
       // If we set value we are done with this "work in progress" filter and we can add it
       if (prop === 'value') {
         this.setState({ filters: [...filters, { ..._wip, [prop]: value }], _wip: undefined });
-        this._runSceneQueries();
       } else {
         this.setState({ _wip: { ...filter, [prop]: value } });
       }
@@ -112,7 +104,7 @@ export class AdHocFilterSet extends SceneObjectBase<AdHocFilterSetState> {
       return f;
     });
 
-    this.updateFilters(updatedFilters);
+    this.setState({ filters: updatedFilters });
   }
 
   public _removeFilter(filter: AdHocVariableFilter) {
@@ -121,12 +113,7 @@ export class AdHocFilterSet extends SceneObjectBase<AdHocFilterSetState> {
       return;
     }
 
-    this.updateFilters(this.state.filters.filter((f) => f !== filter));
-  }
-
-  public updateFilters(filters: AdHocVariableFilter[]) {
-    this.setState({ filters });
-    this._runSceneQueries();
+    this.setState({ filters: this.state.filters.filter((f) => f !== filter) });
   }
 
   /**
@@ -191,53 +178,6 @@ export class AdHocFilterSet extends SceneObjectBase<AdHocFilterSetState> {
       value,
     }));
   }
-
-  private _runSceneQueries() {
-    // In manual mode we do not trigger any queries
-    if (this.state.applyMode === 'manual') {
-      return;
-    }
-
-    const startingPoint = this.parent;
-    if (!startingPoint) {
-      console.error('AdHocFiltersVariable could not find a parent scene to broadcast changes to');
-      return;
-    }
-
-    const ourDS = this._dataSourceSrv.getInstanceSettings(this.state.datasource, this._scopedVars);
-    if (!ourDS) {
-      console.error('AdHocFiltersVariable ds not found', this.state.datasource);
-      return;
-    }
-
-    const triggerQueriesRecursive = (startingPoint: SceneObject) => {
-      if (startingPoint instanceof SceneQueryRunner && this._isSameDS(ourDS, startingPoint.state.datasource)) {
-        startingPoint.runQueries();
-      } else {
-        startingPoint.forEachChild(triggerQueriesRecursive);
-      }
-    };
-
-    triggerQueriesRecursive(startingPoint);
-  }
-
-  private _isSameDS(ourDS: DataSourceInstanceSettings, queryRunnerDS: DataSourceRef | null | undefined) {
-    // This function does some initial checks to try to avoid haing to call _dataSourceSrv.getInstanceSettings
-    // Which is only needed when queryRunner is using data source variable but the adhoc filter is not
-
-    if (this.state.datasource === queryRunnerDS) {
-      return true;
-    }
-
-    // This works when both are using a variable as well
-    if (this.state.datasource?.uid === queryRunnerDS?.uid) {
-      return true;
-    }
-
-    // Finally the fool proof check that works when either we or the query runner is using a variable ds
-    const resolved = this._dataSourceSrv.getInstanceSettings(queryRunnerDS, this._scopedVars);
-    return ourDS?.uid === resolved?.uid;
-  }
 }
 
 export function AdHocFiltersSetRenderer({ model }: SceneComponentProps<AdHocFilterSet>) {

packages/scenes/src/variables/adhoc/AdHocFiltersVariableUrlSyncHandler.ts

@@ -27,7 +27,7 @@ export class AdHocFiltersVariableUrlSyncHandler implements SceneObjectUrlSyncHan
     }
 
     const filters = deserializeUrlToFilters(urlValue);
-    this._variable.updateFilters(filters);
+    this._variable.setState({ filters });
   }
 }