From f704fa423d2465fc7c951ac7794c377cda46de42 Mon Sep 17 00:00:00 2001 From: "Grot (@grafanabot)" <43478413+grafanabot@users.noreply.github.com> Date: Fri, 28 May 2021 10:05:33 -0400 Subject: [PATCH] Prometheus: exemplars show different symbols (#34763) (#34899) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Show different symbols for different queries * Only run different exemplars * Address review comment * Do the same for dashboard + tests (cherry picked from commit 44358958333860f9b2eac914029f894b278692f1) Co-authored-by: Zoltán Bedi --- .../uPlot/geometries/EventsCanvas.tsx | 13 +- .../datasource/prometheus/datasource.test.ts | 139 ++++++++++++++++-- .../datasource/prometheus/datasource.ts | 32 ++-- .../timeseries/plugins/AnnotationsPlugin.tsx | 10 +- .../timeseries/plugins/ExemplarMarker.tsx | 83 ++++++++--- .../timeseries/plugins/ExemplarsPlugin.tsx | 21 ++- 6 files changed, 239 insertions(+), 59 deletions(-) diff --git a/packages/grafana-ui/src/components/uPlot/geometries/EventsCanvas.tsx b/packages/grafana-ui/src/components/uPlot/geometries/EventsCanvas.tsx index 48ec08dd1cc..fe2b5e7cbb1 100644 --- a/packages/grafana-ui/src/components/uPlot/geometries/EventsCanvas.tsx +++ b/packages/grafana-ui/src/components/uPlot/geometries/EventsCanvas.tsx @@ -1,4 +1,4 @@ -import { DataFrame } from '@grafana/data'; +import { DataFrame, DataFrameFieldIndex } from '@grafana/data'; import React, { useLayoutEffect, useMemo, useState } from 'react'; import { usePlotContext } from '../context'; import { Marker } from './Marker'; @@ -9,8 +9,11 @@ interface EventsCanvasProps { id: string; config: UPlotConfigBuilder; events: DataFrame[]; - renderEventMarker: (dataFrame: DataFrame, index: number) => React.ReactNode; - mapEventToXYCoords: (dataFrame: DataFrame, index: number) => { x: number; y: number } | undefined; + renderEventMarker: (dataFrame: DataFrame, dataFrameFieldIndex: DataFrameFieldIndex) => React.ReactNode; + mapEventToXYCoords: ( + dataFrame: DataFrame, + dataFrameFieldIndex: DataFrameFieldIndex + ) => { x: number; y: number } | undefined; } export function EventsCanvas({ id, events, renderEventMarker, mapEventToXYCoords, config }: EventsCanvasProps) { @@ -35,13 +38,13 @@ export function EventsCanvas({ id, events, renderEventMarker, mapEventToXYCoords for (let i = 0; i < events.length; i++) { const frame = events[i]; for (let j = 0; j < frame.length; j++) { - const coords = mapEventToXYCoords(frame, j); + const coords = mapEventToXYCoords(frame, { fieldIndex: j, frameIndex: i }); if (!coords) { continue; } markers.push( - {renderEventMarker(frame, j)} + {renderEventMarker(frame, { fieldIndex: j, frameIndex: i })} ); } diff --git a/public/app/plugins/datasource/prometheus/datasource.test.ts b/public/app/plugins/datasource/prometheus/datasource.test.ts index 3a90c7dc976..45de4c866c1 100644 --- a/public/app/plugins/datasource/prometheus/datasource.test.ts +++ b/public/app/plugins/datasource/prometheus/datasource.test.ts @@ -1759,7 +1759,17 @@ describe('PrometheusDatasource for POST', () => { }); }); -const getPrepareTargetsContext = (target: PromQuery, app?: CoreApp, queryOptions?: Partial) => { +function getPrepareTargetsContext({ + targets, + app, + queryOptions, + languageProvider, +}: { + targets: PromQuery[]; + app?: CoreApp; + queryOptions?: Partial; + languageProvider?: any; +}) { const instanceSettings = ({ url: 'proxied', directUrl: 'direct', @@ -1771,7 +1781,7 @@ const getPrepareTargetsContext = (target: PromQuery, app?: CoreApp, queryOptions const end = 1; const panelId = '2'; const options = ({ - targets: [target], + targets, interval: '1s', panelId, app, @@ -1779,6 +1789,9 @@ const getPrepareTargetsContext = (target: PromQuery, app?: CoreApp, queryOptions } as any) as DataQueryRequest; const ds = new PrometheusDatasource(instanceSettings, templateSrvStub as any, timeSrvStub as any); + if (languageProvider) { + ds.languageProvider = languageProvider; + } const { queries, activeTargets } = ds.prepareTargets(options, start, end); return { @@ -1788,7 +1801,7 @@ const getPrepareTargetsContext = (target: PromQuery, app?: CoreApp, queryOptions end, panelId, }; -}; +} describe('prepareTargets', () => { describe('when run from a Panel', () => { @@ -1799,7 +1812,7 @@ describe('prepareTargets', () => { requestId: '2A', }; - const { queries, activeTargets, panelId, end, start } = getPrepareTargetsContext(target); + const { queries, activeTargets, panelId, end, start } = getPrepareTargetsContext({ targets: [target] }); expect(queries.length).toBe(1); expect(activeTargets.length).toBe(1); @@ -1819,6 +1832,51 @@ describe('prepareTargets', () => { }); expect(activeTargets[0]).toEqual(target); }); + + it('should give back 3 targets when multiple queries with exemplar enabled and same metric', () => { + const targetA: PromQuery = { + refId: 'A', + expr: 'histogram_quantile(0.95, sum(rate(tns_request_duration_seconds_bucket[5m])) by (le))', + exemplar: true, + }; + const targetB: PromQuery = { + refId: 'B', + expr: 'histogram_quantile(0.5, sum(rate(tns_request_duration_seconds_bucket[5m])) by (le))', + exemplar: true, + }; + + const { queries, activeTargets } = getPrepareTargetsContext({ + targets: [targetA, targetB], + languageProvider: { + histogramMetrics: ['tns_request_duration_seconds_bucket'], + }, + }); + expect(queries).toHaveLength(3); + expect(activeTargets).toHaveLength(3); + }); + + it('should give back 4 targets when multiple queries with exemplar enabled', () => { + const targetA: PromQuery = { + refId: 'A', + expr: 'histogram_quantile(0.95, sum(rate(tns_request_duration_seconds_bucket[5m])) by (le))', + exemplar: true, + }; + const targetB: PromQuery = { + refId: 'B', + expr: 'histogram_quantile(0.5, sum(rate(tns_request_duration_bucket[5m])) by (le))', + exemplar: true, + }; + + const { queries, activeTargets } = getPrepareTargetsContext({ + targets: [targetA, targetB], + languageProvider: { + histogramMetrics: ['tns_request_duration_seconds_bucket'], + }, + }); + expect(queries).toHaveLength(4); + expect(activeTargets).toHaveLength(4); + }); + it('should give back 2 targets when exemplar enabled', () => { const target: PromQuery = { refId: 'A', @@ -1826,7 +1884,7 @@ describe('prepareTargets', () => { exemplar: true, }; - const { queries, activeTargets } = getPrepareTargetsContext(target); + const { queries, activeTargets } = getPrepareTargetsContext({ targets: [target] }); expect(queries).toHaveLength(2); expect(activeTargets).toHaveLength(2); expect(activeTargets[0].exemplar).toBe(true); @@ -1840,7 +1898,7 @@ describe('prepareTargets', () => { instant: true, }; - const { queries, activeTargets } = getPrepareTargetsContext(target); + const { queries, activeTargets } = getPrepareTargetsContext({ targets: [target] }); expect(queries).toHaveLength(1); expect(activeTargets).toHaveLength(1); expect(activeTargets[0].instant).toBe(true); @@ -1849,6 +1907,60 @@ describe('prepareTargets', () => { describe('when run from Explore', () => { describe('when query type Both is selected', () => { + it('should give back 6 targets when multiple queries with exemplar enabled', () => { + const targetA: PromQuery = { + refId: 'A', + expr: 'histogram_quantile(0.95, sum(rate(tns_request_duration_seconds_bucket[5m])) by (le))', + instant: true, + range: true, + exemplar: true, + }; + const targetB: PromQuery = { + refId: 'B', + expr: 'histogram_quantile(0.5, sum(rate(tns_request_duration_bucket[5m])) by (le))', + exemplar: true, + instant: true, + range: true, + }; + + const { queries, activeTargets } = getPrepareTargetsContext({ + targets: [targetA, targetB], + app: CoreApp.Explore, + languageProvider: { + histogramMetrics: ['tns_request_duration_seconds_bucket'], + }, + }); + expect(queries).toHaveLength(6); + expect(activeTargets).toHaveLength(6); + }); + + it('should give back 5 targets when multiple queries with exemplar enabled and same metric', () => { + const targetA: PromQuery = { + refId: 'A', + expr: 'histogram_quantile(0.95, sum(rate(tns_request_duration_seconds_bucket[5m])) by (le))', + instant: true, + range: true, + exemplar: true, + }; + const targetB: PromQuery = { + refId: 'B', + expr: 'histogram_quantile(0.5, sum(rate(tns_request_duration_seconds_bucket[5m])) by (le))', + exemplar: true, + instant: true, + range: true, + }; + + const { queries, activeTargets } = getPrepareTargetsContext({ + targets: [targetA, targetB], + app: CoreApp.Explore, + languageProvider: { + histogramMetrics: ['tns_request_duration_seconds_bucket'], + }, + }); + expect(queries).toHaveLength(5); + expect(activeTargets).toHaveLength(5); + }); + it('then it should return both instant and time series related objects', () => { const target: PromQuery = { refId: 'A', @@ -1858,7 +1970,10 @@ describe('prepareTargets', () => { requestId: '2A', }; - const { queries, activeTargets, panelId, end, start } = getPrepareTargetsContext(target, CoreApp.Explore); + const { queries, activeTargets, panelId, end, start } = getPrepareTargetsContext({ + targets: [target], + app: CoreApp.Explore, + }); expect(queries.length).toBe(2); expect(activeTargets.length).toBe(2); @@ -1916,7 +2031,10 @@ describe('prepareTargets', () => { requestId: '2A', }; - const { queries, activeTargets, panelId, end, start } = getPrepareTargetsContext(target, CoreApp.Explore); + const { queries, activeTargets, panelId, end, start } = getPrepareTargetsContext({ + targets: [target], + app: CoreApp.Explore, + }); expect(queries.length).toBe(1); expect(activeTargets.length).toBe(1); @@ -1949,7 +2067,10 @@ describe('prepareTargets', () => { requestId: '2A', }; - const { queries, activeTargets, panelId, end, start } = getPrepareTargetsContext(target, CoreApp.Explore); + const { queries, activeTargets, panelId, end, start } = getPrepareTargetsContext({ + targets: [target], + app: CoreApp.Explore, + }); expect(queries.length).toBe(1); expect(activeTargets.length).toBe(1); diff --git a/public/app/plugins/datasource/prometheus/datasource.ts b/public/app/plugins/datasource/prometheus/datasource.ts index 91ccc0ff170..1f1c35760b1 100644 --- a/public/app/plugins/datasource/prometheus/datasource.ts +++ b/public/app/plugins/datasource/prometheus/datasource.ts @@ -206,6 +206,7 @@ export class PrometheusDatasource extends DataSourceApi } target.requestId = options.panelId + target.refId; + const metricName = this.languageProvider.histogramMetrics.find((m) => target.expr.includes(m)); // In Explore, we run both (instant and range) queries if both are true (selected) or both are undefined (legacy Explore queries) if (options.app === CoreApp.Explore && target.range === target.instant) { @@ -226,13 +227,19 @@ export class PrometheusDatasource extends DataSourceApi // Create exemplar query if (target.exemplar) { - const exemplarTarget = cloneDeep(target); - exemplarTarget.instant = false; - exemplarTarget.requestId += '_exemplar'; + // Only create exemplar target for different metric names + if ( + !metricName || + (metricName && !activeTargets.some((activeTarget) => activeTarget.expr.includes(metricName))) + ) { + const exemplarTarget = cloneDeep(target); + exemplarTarget.instant = false; + exemplarTarget.requestId += '_exemplar'; + queries.push(this.createQuery(exemplarTarget, options, start, end)); + activeTargets.push(exemplarTarget); + } instantTarget.exemplar = false; rangeTarget.exemplar = false; - queries.push(this.createQuery(exemplarTarget, options, start, end)); - activeTargets.push(exemplarTarget); } // Add both targets to activeTargets and queries arrays @@ -250,12 +257,17 @@ export class PrometheusDatasource extends DataSourceApi } else { // It doesn't make sense to query for exemplars in dashboard if only instant is selected if (target.exemplar && !target.instant) { - const exemplarTarget = cloneDeep(target); - exemplarTarget.requestId += '_exemplar'; + if ( + !metricName || + (metricName && !activeTargets.some((activeTarget) => activeTarget.expr.includes(metricName))) + ) { + const exemplarTarget = cloneDeep(target); + exemplarTarget.requestId += '_exemplar'; + queries.push(this.createQuery(exemplarTarget, options, start, end)); + activeTargets.push(exemplarTarget); + this.exemplarErrors.next(); + } target.exemplar = false; - queries.push(this.createQuery(exemplarTarget, options, start, end)); - activeTargets.push(exemplarTarget); - this.exemplarErrors.next(); } if (target.exemplar && target.instant) { this.exemplarErrors.next('Exemplars are not available for instant queries.'); diff --git a/public/app/plugins/panel/timeseries/plugins/AnnotationsPlugin.tsx b/public/app/plugins/panel/timeseries/plugins/AnnotationsPlugin.tsx index 0d61e1746ae..e42e1eb5013 100644 --- a/public/app/plugins/panel/timeseries/plugins/AnnotationsPlugin.tsx +++ b/public/app/plugins/panel/timeseries/plugins/AnnotationsPlugin.tsx @@ -1,4 +1,4 @@ -import { DataFrame, DataFrameView, getColorForTheme, TimeZone } from '@grafana/data'; +import { DataFrame, DataFrameFieldIndex, DataFrameView, getColorForTheme, TimeZone } from '@grafana/data'; import { EventsCanvas, UPlotConfigBuilder, usePlotContext, useTheme } from '@grafana/ui'; import React, { useCallback, useEffect, useLayoutEffect, useRef } from 'react'; import { AnnotationMarker } from './AnnotationMarker'; @@ -65,9 +65,9 @@ export const AnnotationsPlugin: React.FC = ({ annotation }, [config, theme]); const mapAnnotationToXYCoords = useCallback( - (frame: DataFrame, index: number) => { + (frame: DataFrame, dataFrameFieldIndex: DataFrameFieldIndex) => { const view = new DataFrameView(frame); - const annotation = view.get(index); + const annotation = view.get(dataFrameFieldIndex.fieldIndex); const plotInstance = plotCtx.plot; if (!annotation.time || !plotInstance) { return undefined; @@ -82,9 +82,9 @@ export const AnnotationsPlugin: React.FC = ({ annotation ); const renderMarker = useCallback( - (frame: DataFrame, index: number) => { + (frame: DataFrame, dataFrameFieldIndex: DataFrameFieldIndex) => { const view = new DataFrameView(frame); - const annotation = view.get(index); + const annotation = view.get(dataFrameFieldIndex.fieldIndex); return ; }, [timeZone] diff --git a/public/app/plugins/panel/timeseries/plugins/ExemplarMarker.tsx b/public/app/plugins/panel/timeseries/plugins/ExemplarMarker.tsx index 56543fe4c96..9b76eaff2e3 100644 --- a/public/app/plugins/panel/timeseries/plugins/ExemplarMarker.tsx +++ b/public/app/plugins/panel/timeseries/plugins/ExemplarMarker.tsx @@ -1,6 +1,7 @@ import { css, cx } from '@emotion/css'; import { DataFrame, + DataFrameFieldIndex, dateTimeFormat, Field, FieldType, @@ -10,18 +11,25 @@ import { TimeZone, } from '@grafana/data'; import { selectors } from '@grafana/e2e-selectors'; -import { FieldLinkList, Portal, useStyles } from '@grafana/ui'; +import { FieldLinkList, Portal, UPlotConfigBuilder, useStyles } from '@grafana/ui'; import React, { useCallback, useRef, useState } from 'react'; import { usePopper } from 'react-popper'; interface ExemplarMarkerProps { timeZone: TimeZone; dataFrame: DataFrame; - index: number; + dataFrameFieldIndex: DataFrameFieldIndex; + config: UPlotConfigBuilder; getFieldLinks: (field: Field, rowIndex: number) => Array>; } -export const ExemplarMarker: React.FC = ({ timeZone, dataFrame, index, getFieldLinks }) => { +export const ExemplarMarker: React.FC = ({ + timeZone, + dataFrame, + dataFrameFieldIndex, + config, + getFieldLinks, +}) => { const styles = useStyles(getExemplarMarkerStyles); const [isOpen, setIsOpen] = useState(false); const [markerElement, setMarkerElement] = React.useState(null); @@ -29,6 +37,24 @@ export const ExemplarMarker: React.FC = ({ timeZone, dataFr const { styles: popperStyles, attributes } = usePopper(markerElement, popperElement); const popoverRenderTimeout = useRef(); + const getSymbol = () => { + const symbols = [ + , + , + , + , + , + , + ]; + return symbols[dataFrameFieldIndex.frameIndex % symbols.length]; + }; + const onMouseEnter = useCallback(() => { if (popoverRenderTimeout.current) { clearTimeout(popoverRenderTimeout.current); @@ -68,8 +94,10 @@ export const ExemplarMarker: React.FC = ({ timeZone, dataFr {dataFrame.fields.map((field, i) => { - const value = field.values.get(index); - const links = field.config.links?.length ? getFieldLinks(field, index) : undefined; + const value = field.values.get(dataFrameFieldIndex.fieldIndex); + const links = field.config.links?.length + ? getFieldLinks(field, dataFrameFieldIndex.fieldIndex) + : undefined; return ( @@ -93,7 +121,7 @@ export const ExemplarMarker: React.FC = ({ timeZone, dataFr attributes.popper, dataFrame.fields, getFieldLinks, - index, + dataFrameFieldIndex, onMouseEnter, onMouseLeave, popperStyles.popper, @@ -101,6 +129,10 @@ export const ExemplarMarker: React.FC = ({ timeZone, dataFr timeZone, ]); + const seriesColor = config + .getSeries() + .find((s) => s.props.dataFrameFieldIndex?.frameIndex === dataFrameFieldIndex.frameIndex)?.props.lineColor; + return ( <>
= ({ timeZone, dataFr className={styles.markerWrapper} aria-label={selectors.components.DataSource.Prometheus.exemplarMarker} > - - + + {getSymbol()}
{isOpen && {renderMarker()}} @@ -123,21 +161,8 @@ const getExemplarMarkerStyles = (theme: GrafanaTheme) => { const bg = theme.isDark ? theme.palette.dark2 : theme.palette.white; const headerBg = theme.isDark ? theme.palette.dark9 : theme.palette.gray5; const shadowColor = theme.isDark ? theme.palette.black : theme.palette.white; - const marbleFill = theme.isDark ? theme.palette.gray3 : theme.palette.gray1; - const marbleFillHover = theme.isDark ? theme.palette.blue85 : theme.palette.blue77; const tableBgOdd = theme.isDark ? theme.palette.dark3 : theme.palette.gray6; - const marble = css` - display: block; - fill: ${marbleFill}; - transition: transform 0.15s ease-out; - `; - const activeMarble = css` - fill: ${marbleFillHover}; - transform: scale(1.3); - filter: drop-shadow(0 0 8px rgba(0, 0, 0, 0.5)); - `; - return { markerWrapper: css` padding: 0 4px 4px 4px; @@ -147,7 +172,9 @@ const getExemplarMarkerStyles = (theme: GrafanaTheme) => { &:hover { > svg { - ${activeMarble} + transform: scale(1.3); + opacity: 1; + filter: drop-shadow(0 0 8px rgba(0, 0, 0, 0.5)); } } `, @@ -218,7 +245,15 @@ const getExemplarMarkerStyles = (theme: GrafanaTheme) => { padding: ${theme.spacing.sm}; font-weight: ${theme.typography.weight.semibold}; `, - marble, - activeMarble, + marble: css` + display: block; + opacity: 0.5; + transition: transform 0.15s ease-out; + `, + activeMarble: css` + transform: scale(1.3); + opacity: 1; + filter: drop-shadow(0 0 8px rgba(0, 0, 0, 0.5)); + `, }; }; diff --git a/public/app/plugins/panel/timeseries/plugins/ExemplarsPlugin.tsx b/public/app/plugins/panel/timeseries/plugins/ExemplarsPlugin.tsx index ad1be4156ee..7c34ee868b5 100644 --- a/public/app/plugins/panel/timeseries/plugins/ExemplarsPlugin.tsx +++ b/public/app/plugins/panel/timeseries/plugins/ExemplarsPlugin.tsx @@ -1,5 +1,6 @@ import { DataFrame, + DataFrameFieldIndex, Field, LinkModel, TimeZone, @@ -21,7 +22,7 @@ export const ExemplarsPlugin: React.FC = ({ exemplars, tim const plotCtx = usePlotContext(); const mapExemplarToXYCoords = useCallback( - (dataFrame: DataFrame, index: number) => { + (dataFrame: DataFrame, dataFrameFieldIndex: DataFrameFieldIndex) => { const plotInstance = plotCtx.plot; const time = dataFrame.fields.find((f) => f.name === TIME_SERIES_TIME_FIELD_NAME); const value = dataFrame.fields.find((f) => f.name === TIME_SERIES_VALUE_FIELD_NAME); @@ -37,7 +38,7 @@ export const ExemplarsPlugin: React.FC = ({ exemplars, tim const yMin = plotInstance.scales[yScale].min; const yMax = plotInstance.scales[yScale].max; - let y = value.values.get(index); + let y = value.values.get(dataFrameFieldIndex.fieldIndex); // To not to show exemplars outside of the graph we set the y value to min if it is smaller and max if it is bigger than the size of the graph if (yMin != null && y < yMin) { y = yMin; @@ -48,7 +49,7 @@ export const ExemplarsPlugin: React.FC = ({ exemplars, tim } return { - x: plotInstance.valToPos(time.values.get(index), 'x'), + x: plotInstance.valToPos(time.values.get(dataFrameFieldIndex.fieldIndex), 'x'), y: plotInstance.valToPos(y, yScale), }; }, @@ -56,10 +57,18 @@ export const ExemplarsPlugin: React.FC = ({ exemplars, tim ); const renderMarker = useCallback( - (dataFrame: DataFrame, index: number) => { - return ; + (dataFrame: DataFrame, dataFrameFieldIndex: DataFrameFieldIndex) => { + return ( + + ); }, - [timeZone, getFieldLinks] + [config, timeZone, getFieldLinks] ); return (
{field.name}