dashboards: add pmproxy URL and hostspec variables to the PCP Vector …

…Host Overview dashboard
performancecopilot · Jun 17, 2021 · 422371c · 422371c
1 parent 40ed4f1
commit 422371c
Show file tree

Hide file tree

Showing 7 changed files with 196 additions and 87 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,6 +2,8 @@
 
 ## 3.0.4 (unreleased)
 
+- **dashboards**: PCP Vector Host Overview: add pmproxy URL and hostspec variables
+- **vector, bpftrace**: use `pcp://127.0.0.1` as default hostspec (no functional change)
 - **chore**: update dependencies
 - **test**: replace convey with testify for the Go tests
 

diff --git a/src/datasources/bpftrace/datasource.test.ts b/src/datasources/bpftrace/datasource.test.ts
@@ -31,15 +31,15 @@ describe('PCP BPFtrace', () => {
     });
 
     it('should register a script and return the data', async () => {
-        const queries = [
+        const targets = [
             {
                 refId: 'A',
                 expr: '...cpuwalk...',
                 format: TargetFormat.Heatmap,
             },
         ];
 
-        let response = await datasource.query(grafana.dataQueryRequest(queries));
+        let response = await datasource.query(grafana.dataQueryRequest({ targets }));
         expect(response).toEqual({ data: [] });
 
         mockNextResponses([
@@ -57,7 +57,7 @@ describe('PCP BPFtrace', () => {
         mockNextResponses([fetchResponse2]);
         await datasource.poller.poll();
 
-        response = await datasource.query(grafana.dataQueryRequest(queries));
+        response = await datasource.query(grafana.dataQueryRequest({ targets }));
         expect({ fields: response.data[0].fields }).toMatchInlineSnapshot(
             {
                 fields: [
@@ -245,14 +245,14 @@ describe('PCP BPFtrace', () => {
             Array [
               Object {
                 "params": Object {
-                  "hostspec": "127.0.0.1",
+                  "hostspec": "pcp://127.0.0.1",
                   "polltimeout": 11,
                 },
                 "url": "http://localhost:1234/pmapi/context",
               },
               Object {
                 "params": Object {
-                  "hostspec": "127.0.0.1",
+                  "hostspec": "pcp://127.0.0.1",
                   "polltimeout": 30,
                 },
                 "url": "http://localhost:1234/pmapi/context",

diff --git a/src/datasources/lib/pmapi/data_processor.test.ts b/src/datasources/lib/pmapi/data_processor.test.ts
@@ -5,7 +5,7 @@ import { processQueries } from './data_processor';
 describe('data processor', () => {
     it('should create a dataframe and handle missing frames and backward counters', () => {
         const target = poller.target({ query: { expr: 'disk.dev.read' } });
-        const dataQueryRequest = grafana.dataQueryRequest([target.query]); // request data between 10-20s
+        const dataQueryRequest = grafana.dataQueryRequest({ targets: [target.query] }); // request data between 10-20s
 
         const values = [
             {
@@ -266,7 +266,7 @@ describe('data processor', () => {
         };
 
         const endpoint = poller.endpoint({ metrics: [metricA, metricB], targets: [targetA, targetB] });
-        const dataQueryRequest = grafana.dataQueryRequest([targetA.query, targetB.query]);
+        const dataQueryRequest = grafana.dataQueryRequest({ targets: [targetA.query, targetB.query] });
 
         const result = processQueries(
             dataQueryRequest,
@@ -359,7 +359,7 @@ describe('data processor', () => {
         };
 
         const endpoint = poller.endpoint({ metrics: [metric], targets: [target] });
-        const dataQueryRequest = grafana.dataQueryRequest([target.query]);
+        const dataQueryRequest = grafana.dataQueryRequest({ targets: [target.query] });
         const result = processQueries(dataQueryRequest, [{ endpoint, query: target.query, metrics: [metric] }], 1);
         expect(result[0].fields).toMatchInlineSnapshot(`
             Array [

diff --git a/src/datasources/lib/pmapi/datasource_base.ts b/src/datasources/lib/pmapi/datasource_base.ts
@@ -19,7 +19,9 @@ import { MinimalPmapiQuery, PmapiDefaultOptions, PmapiOptions, PmapiQuery } from
 const log = getLogger('datasource_base');
 
 export abstract class DataSourceBase<Q extends MinimalPmapiQuery, O extends PmapiOptions> extends DataSourceApi<Q, O> {
+    /** URL as specified in the datasource settings page (can be undefined) */
     url?: string;
+    /** hostspec as specified in the datasource settings page, or default hostspec */
     hostspec: string;
     retentionTimeMs: number;
     pmApiService: PmApiService;
@@ -33,8 +35,19 @@ export abstract class DataSourceBase<Q extends MinimalPmapiQuery, O extends Pmap
     ) {
         super(instanceSettings);
         this.url = instanceSettings.url;
-        this.hostspec = instanceSettings.jsonData.hostspec ?? defaults.hostspec;
-        this.retentionTimeMs = interval_to_ms(instanceSettings.jsonData.retentionTime ?? defaults.retentionTime);
+
+        if (isBlank(instanceSettings.jsonData.hostspec)) {
+            this.hostspec = defaults.hostspec;
+        } else {
+            this.hostspec = instanceSettings.jsonData.hostspec!;
+        }
+
+        if (isBlank(instanceSettings.jsonData.retentionTime)) {
+            this.retentionTimeMs = interval_to_ms(defaults.retentionTime);
+        } else {
+            this.retentionTimeMs = interval_to_ms(instanceSettings.jsonData.retentionTime!);
+        }
+
         this.pmApiService = new PmApiService(getBackendSrv(), {
             dsInstanceSettings: instanceSettings,
             timeoutMs: apiTimeoutMs,
@@ -54,30 +67,48 @@ export abstract class DataSourceBase<Q extends MinimalPmapiQuery, O extends Pmap
     }
 
     getUrlAndHostspec(query?: Q, scopedVars: ScopedVars = {}): { url: string; hostspec: string } {
-        const url = getTemplateSrv().replace(query?.url ?? this.url ?? '', scopedVars);
-        const orInTheQueryErrorText = query ? ' or in the query editor' : '';
+        let url: string | undefined;
+        let hostspec: string | undefined;
+
+        // evaluate query settings first
+        if (query) {
+            if (!isBlank(query.url)) {
+                url = getTemplateSrv().replace(query.url);
+
+                if (this.url?.startsWith('/api/datasources/proxy') && !isBlank(url)) {
+                    // Grafana will send additional x-grafana headers to every request in browser mode, which make the CORS request fail
+                    throw new GenericError(
+                        'Please set the access mode to Browser in the datasource settings when using a custom pmproxy URL for this panel.'
+                    );
+                }
+            }
+            if (!isBlank(query.hostspec)) {
+                hostspec = getTemplateSrv().replace(query.hostspec);
+            }
+        }
 
-        if (this.url?.startsWith('/api/datasources/proxy') && !isBlank(query?.url)) {
-            // Grafana will send additional x-grafana headers, which make the CORS request fail
-            throw new GenericError(
-                'Please set the access mode to Browser in the datasource settings when using a custom URL for this panel.'
-            );
+        // if query is not defined (e.g. it's a dashboard variable query (metricFindQuery))
+        // or the url/hostspec of it evaluates to a blank string, use the datasource settings
+        if (isBlank(url)) {
+            url = this.url;
+        }
+        if (isBlank(hostspec)) {
+            hostspec = this.hostspec;
         }
 
+        const orInTheQueryErrorText = query ? ' or in the query editor' : '';
         if (isBlank(url)) {
             throw new GenericError(
                 `Please specify a connection URL in the datasource settings${orInTheQueryErrorText}.`
             );
         }
-
-        const hostspec = getTemplateSrv().replace(query?.hostspec ?? this.hostspec, scopedVars);
         if (isBlank(hostspec)) {
             throw new GenericError(
                 `Please specify a host specification in the datasource settings${orInTheQueryErrorText}.`
             );
         }
 
-        return { url, hostspec };
+        return { url: url!, hostspec: hostspec! };
     }
 
     async metricFindQuery(query: string): Promise<MetricFindValue[]> {

diff --git a/src/datasources/lib/specs/fixtures/grafana.ts b/src/datasources/lib/specs/fixtures/grafana.ts
@@ -1,5 +1,7 @@
 import { DataQuery, DataQueryRequest, dateTimeParse, TimeRange } from '@grafana/data';
 import { FetchResponse } from '@grafana/runtime';
+import { defaultsDeep } from 'lodash';
+import { DeepPartial } from 'utility-types';
 
 export function timeRange(): TimeRange {
     return {
@@ -12,8 +14,8 @@ export function timeRange(): TimeRange {
     };
 }
 
-export function dataQueryRequest<Q extends DataQuery>(queries: Q[] = []): DataQueryRequest<Q> {
-    return {
+export function dataQueryRequest<Q extends DataQuery>(props?: DeepPartial<DataQueryRequest<Q>>): DataQueryRequest<Q> {
+    return defaultsDeep({}, props, {
         app: 'dashboard',
         dashboardId: 1,
         panelId: 2,
@@ -24,8 +26,8 @@ export function dataQueryRequest<Q extends DataQuery>(queries: Q[] = []): DataQu
         timezone: '',
         range: timeRange(),
         scopedVars: {},
-        targets: queries,
-    };
+        targets: [],
+    });
 }
 
 export function response<T>(data: T): FetchResponse<T> {