From 45a8701cb57cdbe6dac419ee43b7382b51c65350 Mon Sep 17 00:00:00 2001 From: ArturWierzbicki Date: Mon, 25 Oct 2021 12:11:44 +0400 Subject: [PATCH 01/49] Chore: Updated the link to the plugin developer guide docs - it used to point to an old version (#40710) --- PLUGIN_DEV.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/PLUGIN_DEV.md b/PLUGIN_DEV.md index e33c8047e57..fbb2b79c0f2 100644 --- a/PLUGIN_DEV.md +++ b/PLUGIN_DEV.md @@ -6,7 +6,7 @@ upgrading Grafana please check here before creating an issue. ## Plugin development resources -- [Grafana plugin developer guide](http://docs.grafana.org/plugins/developing/development/) +- [Grafana plugin developer guide](https://grafana.com/docs/grafana/latest/developers/plugins/) - [Webpack Grafana plugin template project](https://github.com/CorpGlory/grafana-plugin-template-webpack) - [Simple JSON datasource plugin](https://github.com/grafana/simple-json-datasource) From d5de885633eb8b99b6a98e9492291043b88e21c8 Mon Sep 17 00:00:00 2001 From: Junya Hayashi Date: Mon, 25 Oct 2021 17:53:08 +0900 Subject: [PATCH 02/49] CloudMonitoring: Fix TypeError in annotation queries (#40740) --- public/app/plugins/datasource/cloud-monitoring/datasource.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/public/app/plugins/datasource/cloud-monitoring/datasource.ts b/public/app/plugins/datasource/cloud-monitoring/datasource.ts index bb2e866eacb..874bde7f0cc 100644 --- a/public/app/plugins/datasource/cloud-monitoring/datasource.ts +++ b/public/app/plugins/datasource/cloud-monitoring/datasource.ts @@ -62,7 +62,7 @@ export default class CloudMonitoringDatasource extends DataSourceWithBackend< metricType: this.templateSrv.replace(annotation.target.metricType, options.scopedVars || {}), title: this.templateSrv.replace(annotation.target.title, options.scopedVars || {}), text: this.templateSrv.replace(annotation.target.text, options.scopedVars || {}), - tags: this.templateSrv.replace(annotation.target.tags, options.scopedVars || {}), + tags: (annotation.target.tags || []).map((t: string) => this.templateSrv.replace(t, options.scopedVars || {})), projectName: this.templateSrv.replace( annotation.target.projectName ? annotation.target.projectName : this.getDefaultProject(), options.scopedVars || {} From e6d2324516e5c6b3fa2cbc612fb979e081bd75bb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zolt=C3=A1n=20Bedi?= Date: Mon, 25 Oct 2021 11:21:51 +0200 Subject: [PATCH 03/49] Time series/Bar chart panel: Add ability to sort series via legend (#40226) * Make legend sorting work in Time series panel * Import from schema Add properties to the cue schema as well * Order stacking * Add tests for orderIdsByCalcs * Add check for legend options * Fix cue schema * UI fixes * Order bars as well in barchart * Use different index when ordered * Legend sort series doc * Fix nits * Update docs/sources/panels/legend-options.md Co-authored-by: Dominik Prokop * Fix linting * Apply suggestions from code review Co-authored-by: Ursula Kallio <73951760+osg-grafana@users.noreply.github.com> * Update docs/sources/panels/legend-options.md Co-authored-by: Dominik Prokop Co-authored-by: Ursula Kallio <73951760+osg-grafana@users.noreply.github.com> --- docs/sources/panels/legend-options.md | 14 +++ .../grafana-schema/src/schema/graph.gen.ts | 2 + packages/grafana-schema/src/schema/legend.cue | 8 +- .../components/PanelChrome/PanelContext.ts | 5 + .../src/components/TimeSeries/TimeSeries.tsx | 5 +- .../src/components/TimeSeries/utils.ts | 11 +- .../src/components/VizLegend/VizLegend.tsx | 4 +- .../components/VizLegend/VizLegendTable.tsx | 40 ++++--- .../VizLegend/VizLegendTableItem.tsx | 2 +- .../src/components/uPlot/PlotLegend.tsx | 8 +- .../src/components/uPlot/utils.test.ts | 110 +++++++++++++++++- .../grafana-ui/src/components/uPlot/utils.ts | 37 +++++- .../dashboard/dashgrid/PanelChrome.tsx | 31 +++++ .../app/plugins/panel/barchart/BarChart.tsx | 8 +- .../plugins/panel/barchart/BarChartPanel.tsx | 6 +- public/app/plugins/panel/barchart/bars.ts | 20 +++- .../app/plugins/panel/barchart/utils.test.ts | 35 +++++- public/app/plugins/panel/barchart/utils.ts | 56 +++++++-- 18 files changed, 340 insertions(+), 62 deletions(-) diff --git a/docs/sources/panels/legend-options.md b/docs/sources/panels/legend-options.md index 9e8822f39ea..69a42224da6 100644 --- a/docs/sources/panels/legend-options.md +++ b/docs/sources/panels/legend-options.md @@ -10,8 +10,13 @@ Use the legend to adjust how a visualization displays series. This legend functi This topic currently applies to the following visualizations: +- [Bar chart panel]({{< relref "../visualizations/bar-chart.md">}}) +- [Histogram panel]({{< relref "../visualizations/histogram.md">}}) - [Pie chart panel]({{< relref "../visualizations/pie-chart-panel.md">}}) +- [State timeline panel]({{< relref "../visualizations/state-timeline.md">}}) +- [Status history panel]({{< relref "../visualizations/status-history.md">}}) - [Time series panel]({{< relref "../visualizations/time-series/_index.md" >}}) +- XY chart panel ## Toggle series @@ -34,3 +39,12 @@ This creates a system override that hides the other series. You can view this ov Click on the series icon (colored line beside the series label) in the legend to change selected series color. ![Change legend series color](/static/img/docs/legend/legend-series-color-7-5.png) + +## Sort series + +Change legend mode to **Table** and choose [calculations]({{< relref "./calculations-list.md" >}}) to be displayed in the legend. Click the calculation name header in the legend table to sort the values in the table in ascending or descending order. +The sort order affects the positions of the bars in the Bar chart panel as well as the order of stacked series in the Time series and Bar chart panels. + +> **Note:** This feature is only supported in these panels: Bar chart, Histogram, Time series, XY Chart. + +![Sort legend series](/static/img/docs/legend/legend-series-sort-8-3.png) diff --git a/packages/grafana-schema/src/schema/graph.gen.ts b/packages/grafana-schema/src/schema/graph.gen.ts index 5d9004906c0..0c2df46280c 100644 --- a/packages/grafana-schema/src/schema/graph.gen.ts +++ b/packages/grafana-schema/src/schema/graph.gen.ts @@ -258,6 +258,8 @@ export interface VizLegendOptions { displayMode: LegendDisplayMode; isVisible?: boolean; placement: LegendPlacement; + sortBy?: string; + sortDesc?: boolean; } export enum BarGaugeDisplayMode { diff --git a/packages/grafana-schema/src/schema/legend.cue b/packages/grafana-schema/src/schema/legend.cue index 23986bd83ff..336ab2324b2 100644 --- a/packages/grafana-schema/src/schema/legend.cue +++ b/packages/grafana-schema/src/schema/legend.cue @@ -5,9 +5,11 @@ LegendPlacement: "bottom" | "right" @cuetsy(kind="type") LegendDisplayMode: "list" | "table" | "hidden" @cuetsy(kind="enum") VizLegendOptions: { - displayMode: LegendDisplayMode - placement: LegendPlacement + displayMode: LegendDisplayMode + placement: LegendPlacement asTable?: bool isVisible?: bool - calcs: [...string] + sortBy?: string + sortDesc?: bool + calcs: [...string] } @cuetsy(kind="interface") diff --git a/packages/grafana-ui/src/components/PanelChrome/PanelContext.ts b/packages/grafana-ui/src/components/PanelChrome/PanelContext.ts index 583552046ea..b1581a2414f 100644 --- a/packages/grafana-ui/src/components/PanelChrome/PanelContext.ts +++ b/packages/grafana-ui/src/components/PanelChrome/PanelContext.ts @@ -59,6 +59,11 @@ export interface PanelContext { /** Update instance state, this is only supported in dashboard panel context currently */ onInstanceStateChange?: (state: any) => void; + + /** + * Called when a panel is changing the sort order of the legends. + */ + onToggleLegendSort?: (sortBy: string) => void; } export const PanelContextRoot = React.createContext({ diff --git a/packages/grafana-ui/src/components/TimeSeries/TimeSeries.tsx b/packages/grafana-ui/src/components/TimeSeries/TimeSeries.tsx index 67a2671df9b..f9da81aebdd 100644 --- a/packages/grafana-ui/src/components/TimeSeries/TimeSeries.tsx +++ b/packages/grafana-ui/src/components/TimeSeries/TimeSeries.tsx @@ -8,7 +8,7 @@ import { preparePlotConfigBuilder } from './utils'; import { withTheme2 } from '../../themes/ThemeContext'; import { PanelContext, PanelContextRoot } from '../PanelChrome/PanelContext'; -const propsToDiff: string[] = []; +const propsToDiff: string[] = ['legend']; type TimeSeriesProps = Omit; @@ -18,7 +18,7 @@ export class UnthemedTimeSeries extends React.Component { prepConfig = (alignedFrame: DataFrame, allFrames: DataFrame[], getTimeRange: () => TimeRange) => { const { eventBus, sync } = this.context; - const { theme, timeZone } = this.props; + const { theme, timeZone, legend } = this.props; return preparePlotConfigBuilder({ frame: alignedFrame, @@ -28,6 +28,7 @@ export class UnthemedTimeSeries extends React.Component { eventBus, sync, allFrames, + legend, }); }; diff --git a/packages/grafana-ui/src/components/TimeSeries/utils.ts b/packages/grafana-ui/src/components/TimeSeries/utils.ts index af1e092e96d..00b9a2356f7 100644 --- a/packages/grafana-ui/src/components/TimeSeries/utils.ts +++ b/packages/grafana-ui/src/components/TimeSeries/utils.ts @@ -23,8 +23,9 @@ import { VisibilityMode, ScaleDirection, ScaleOrientation, + VizLegendOptions, } from '@grafana/schema'; -import { collectStackingGroups, preparePlotData } from '../uPlot/utils'; +import { collectStackingGroups, orderIdsByCalcs, preparePlotData } from '../uPlot/utils'; import uPlot from 'uplot'; const defaultFormatter = (v: any) => (v == null ? '-' : v.toFixed(1)); @@ -35,7 +36,7 @@ const defaultConfig: GraphFieldConfig = { axisPlacement: AxisPlacement.Auto, }; -export const preparePlotConfigBuilder: UPlotConfigPrepFn<{ sync: DashboardCursorSync }> = ({ +export const preparePlotConfigBuilder: UPlotConfigPrepFn<{ sync: DashboardCursorSync; legend?: VizLegendOptions }> = ({ frame, theme, timeZone, @@ -43,10 +44,11 @@ export const preparePlotConfigBuilder: UPlotConfigPrepFn<{ sync: DashboardCursor eventBus, sync, allFrames, + legend, }) => { const builder = new UPlotConfigBuilder(timeZone); - builder.setPrepData(preparePlotData); + builder.setPrepData((prepData) => preparePlotData(prepData, undefined, legend)); // X is the first field in the aligned frame const xField = frame.fields[0]; @@ -265,7 +267,8 @@ export const preparePlotConfigBuilder: UPlotConfigPrepFn<{ sync: DashboardCursor if (stackingGroups.size !== 0) { builder.setStacking(true); - for (const [_, seriesIdxs] of stackingGroups.entries()) { + for (const [_, seriesIds] of stackingGroups.entries()) { + const seriesIdxs = orderIdsByCalcs({ ids: seriesIds, legend, frame }); for (let j = seriesIdxs.length - 1; j > 0; j--) { builder.addBand({ series: [seriesIdxs[j], seriesIdxs[j - 1]], diff --git a/packages/grafana-ui/src/components/VizLegend/VizLegend.tsx b/packages/grafana-ui/src/components/VizLegend/VizLegend.tsx index e034f85f37d..c44a72924d2 100644 --- a/packages/grafana-ui/src/components/VizLegend/VizLegend.tsx +++ b/packages/grafana-ui/src/components/VizLegend/VizLegend.tsx @@ -23,7 +23,7 @@ export function VizLegend({ itemRenderer, readonly, }: LegendProps) { - const { eventBus, onToggleSeriesVisibility } = usePanelContext(); + const { eventBus, onToggleSeriesVisibility, onToggleLegendSort } = usePanelContext(); const onMouseEnter = useCallback( (item: VizLegendItem, event: React.MouseEvent) => { @@ -82,7 +82,7 @@ export function VizLegend({ sortBy={sortKey} sortDesc={sortDesc} onLabelClick={onLegendLabelClick} - onToggleSort={onToggleSort} + onToggleSort={onToggleSort || onToggleLegendSort} onLabelMouseEnter={onMouseEnter} onLabelMouseOut={onMouseOut} itemRenderer={itemRenderer} diff --git a/packages/grafana-ui/src/components/VizLegend/VizLegendTable.tsx b/packages/grafana-ui/src/components/VizLegend/VizLegendTable.tsx index dbe7fba1103..0bd289bd8f9 100644 --- a/packages/grafana-ui/src/components/VizLegend/VizLegendTable.tsx +++ b/packages/grafana-ui/src/components/VizLegend/VizLegendTable.tsx @@ -3,7 +3,7 @@ import { css, cx } from '@emotion/css'; import { VizLegendTableProps } from './types'; import { Icon } from '../Icon/Icon'; import { useStyles2 } from '../../themes/ThemeContext'; -import { sortBy } from 'lodash'; +import { orderBy } from 'lodash'; import { LegendTableItem } from './VizLegendTableItem'; import { DisplayValue, GrafanaTheme2 } from '@grafana/data'; @@ -34,13 +34,17 @@ export const VizLegendTable = ({ } const sortedItems = sortKey - ? sortBy(items, (item) => { - if (item.getDisplayValues) { - const stat = item.getDisplayValues().filter((stat) => stat.title === sortKey)[0]; - return stat && stat.numeric; - } - return undefined; - }) + ? orderBy( + items, + (item) => { + if (item.getDisplayValues) { + const stat = item.getDisplayValues().filter((stat) => stat.title === sortKey)[0]; + return stat && stat.numeric; + } + return undefined; + }, + sortDesc ? 'desc' : 'asc' + ) : items; if (!itemRenderer) { @@ -68,7 +72,9 @@ export const VizLegendTable = ({ { if (onToggleSort) { onToggleSort(columnTitle); @@ -76,9 +82,7 @@ export const VizLegendTable = ({ }} > {columnTitle} - {sortKey === columnTitle && ( - - )} + {sortKey === columnTitle && } ); })} @@ -94,21 +98,23 @@ const getStyles = (theme: GrafanaTheme2) => ({ width: 100%; th:first-child { width: 100%; + border-bottom: 1px solid ${theme.colors.border.weak}; } `, header: css` color: ${theme.colors.primary.text}; font-weight: ${theme.typography.fontWeightMedium}; border-bottom: 1px solid ${theme.colors.border.weak}; - padding: ${theme.spacing(0.25, 1)}; + padding: ${theme.spacing(0.25, 2, 0.25, 1)}; font-size: ${theme.typography.bodySmall.fontSize}; - text-align: right; + text-align: left; white-space: nowrap; `, + // This needs to be padding-right - icon size(xs==12) to avoid jumping + withIcon: css` + padding-right: 4px; + `, headerSortable: css` cursor: pointer; `, - sortIcon: css` - margin-left: ${theme.spacing(1)}; - `, }); diff --git a/packages/grafana-ui/src/components/VizLegend/VizLegendTableItem.tsx b/packages/grafana-ui/src/components/VizLegend/VizLegendTableItem.tsx index 720396ceee5..f35f7bbb712 100644 --- a/packages/grafana-ui/src/components/VizLegend/VizLegendTableItem.tsx +++ b/packages/grafana-ui/src/components/VizLegend/VizLegendTableItem.tsx @@ -120,7 +120,7 @@ const getStyles = (theme: GrafanaTheme2) => { align-items: center; `, value: css` - text-align: right; + text-align: left; `, yAxisLabel: css` color: ${theme.colors.text.secondary}; diff --git a/packages/grafana-ui/src/components/uPlot/PlotLegend.tsx b/packages/grafana-ui/src/components/uPlot/PlotLegend.tsx index 847a268870f..ae7117765b6 100644 --- a/packages/grafana-ui/src/components/uPlot/PlotLegend.tsx +++ b/packages/grafana-ui/src/components/uPlot/PlotLegend.tsx @@ -85,7 +85,13 @@ export const PlotLegend: React.FC = ({ return ( - + ); }; diff --git a/packages/grafana-ui/src/components/uPlot/utils.test.ts b/packages/grafana-ui/src/components/uPlot/utils.test.ts index 68f9ffecd8f..3897377bba5 100644 --- a/packages/grafana-ui/src/components/uPlot/utils.test.ts +++ b/packages/grafana-ui/src/components/uPlot/utils.test.ts @@ -1,4 +1,4 @@ -import { preparePlotData, timeFormatToTemplate } from './utils'; +import { orderIdsByCalcs, preparePlotData, timeFormatToTemplate } from './utils'; import { FieldType, MutableDataFrame } from '@grafana/data'; import { StackingMode } from '@grafana/schema'; @@ -295,5 +295,113 @@ describe('preparePlotData', () => { ] `); }); + + describe('with legend sorted', () => { + it('should affect when single group', () => { + const df = new MutableDataFrame({ + fields: [ + { name: 'time', type: FieldType.time, values: [9997, 9998, 9999] }, + { + name: 'a', + values: [-10, 20, 10], + state: { calcs: { max: 20 } }, + config: { custom: { stacking: { mode: StackingMode.Normal, group: 'stackA' } } }, + }, + { + name: 'b', + values: [10, 10, 10], + state: { calcs: { max: 10 } }, + config: { custom: { stacking: { mode: StackingMode.Normal, group: 'stackA' } } }, + }, + { + name: 'c', + values: [20, 20, 20], + state: { calcs: { max: 20 } }, + config: { custom: { stacking: { mode: StackingMode.Normal, group: 'stackA' } } }, + }, + ], + }); + + expect(preparePlotData([df], undefined, { sortBy: 'Max', sortDesc: false } as any)).toMatchInlineSnapshot(` + Array [ + Array [ + 9997, + 9998, + 9999, + ], + Array [ + 0, + 30, + 20, + ], + Array [ + 10, + 10, + 10, + ], + Array [ + 20, + 50, + 40, + ], + ] + `); + expect(preparePlotData([df], undefined, { sortBy: 'Max', sortDesc: true } as any)).toMatchInlineSnapshot(` + Array [ + Array [ + 9997, + 9998, + 9999, + ], + Array [ + -10, + 20, + 10, + ], + Array [ + 20, + 50, + 40, + ], + Array [ + 10, + 40, + 30, + ], + ] + `); + }); + }); + }); +}); + +describe('orderIdsByCalcs', () => { + const ids = [1, 2, 3, 4]; + const frame = new MutableDataFrame({ + fields: [ + { name: 'time', type: FieldType.time, values: [9997, 9998, 9999] }, + { name: 'a', values: [-10, 20, 10], state: { calcs: { min: -10 } } }, + { name: 'b', values: [20, 20, 20], state: { calcs: { min: 20 } } }, + { name: 'c', values: [10, 10, 10], state: { calcs: { min: 10 } } }, + { name: 'd', values: [30, 30, 30] }, + ], + }); + + it.each([ + { legend: undefined }, + { legend: { sortBy: 'Min' } }, + { legend: { sortDesc: false } }, + { legend: {} }, + { sortBy: 'Mik', sortDesc: true }, + ])('should return without ordering if legend option is %o', (legend: any) => { + const result = orderIdsByCalcs({ ids, frame, legend }); + expect(result).toEqual([1, 2, 3, 4]); + }); + + it('should order the ids based on the frame stat', () => { + const resultDesc = orderIdsByCalcs({ ids, frame, legend: { sortBy: 'Min', sortDesc: true } as any }); + expect(resultDesc).toEqual([4, 2, 3, 1]); + const resultAsc = orderIdsByCalcs({ ids, frame, legend: { sortBy: 'Min', sortDesc: false } as any }); + expect(resultAsc).toEqual([1, 3, 2, 4]); }); }); diff --git a/packages/grafana-ui/src/components/uPlot/utils.ts b/packages/grafana-ui/src/components/uPlot/utils.ts index aaa496f0654..58f8ae37517 100755 --- a/packages/grafana-ui/src/components/uPlot/utils.ts +++ b/packages/grafana-ui/src/components/uPlot/utils.ts @@ -1,8 +1,9 @@ import { DataFrame, ensureTimeField, Field, FieldType } from '@grafana/data'; -import { StackingMode } from '@grafana/schema'; -import { createLogger } from '../../utils/logger'; -import { attachDebugger } from '../../utils'; +import { StackingMode, VizLegendOptions } from '@grafana/schema'; +import { orderBy } from 'lodash'; import { AlignedData, Options, PaddingSide } from 'uplot'; +import { attachDebugger } from '../../utils'; +import { createLogger } from '../../utils/logger'; const ALLOWED_FORMAT_STRINGS_REGEX = /\b(YYYY|YY|MMMM|MMM|MM|M|DD|D|WWWW|WWW|HH|H|h|AA|aa|a|mm|m|ss|s|fff)\b/g; @@ -39,7 +40,11 @@ interface StackMeta { } /** @internal */ -export function preparePlotData(frames: DataFrame[], onStackMeta?: (meta: StackMeta) => void): AlignedData { +export function preparePlotData( + frames: DataFrame[], + onStackMeta?: (meta: StackMeta) => void, + legend?: VizLegendOptions +): AlignedData { const frame = frames[0]; const result: any[] = []; const stackingGroups: Map = new Map(); @@ -67,7 +72,9 @@ export function preparePlotData(frames: DataFrame[], onStackMeta?: (meta: StackM alignedTotals[0] = null; // array or stacking groups - for (const [_, seriesIdxs] of stackingGroups.entries()) { + for (const [_, seriesIds] of stackingGroups.entries()) { + const seriesIdxs = orderIdsByCalcs({ ids: seriesIds, legend, frame }); + const groupTotals = byPct ? Array(dataLength).fill(0) : null; if (byPct) { @@ -184,3 +191,23 @@ export const pluginLogger = createLogger('uPlot'); export const pluginLog = pluginLogger.logger; // pluginLogger.enable(); attachDebugger('graphng', undefined, pluginLogger); + +type OrderIdsByCalcsOptions = { + legend?: VizLegendOptions; + ids: number[]; + frame: DataFrame; +}; +export function orderIdsByCalcs({ legend, ids, frame }: OrderIdsByCalcsOptions) { + if (!legend?.sortBy || legend.sortDesc == null) { + return ids; + } + const orderedIds = orderBy( + ids, + (id) => { + return frame.fields[id].state?.calcs?.[legend.sortBy!.toLowerCase()]; + }, + legend.sortDesc ? 'desc' : 'asc' + ); + + return orderedIds; +} diff --git a/public/app/features/dashboard/dashgrid/PanelChrome.tsx b/public/app/features/dashboard/dashgrid/PanelChrome.tsx index 7f9995eef85..cc91d44babd 100644 --- a/public/app/features/dashboard/dashgrid/PanelChrome.tsx +++ b/public/app/features/dashboard/dashgrid/PanelChrome.tsx @@ -20,6 +20,7 @@ import { toUtc, } from '@grafana/data'; import { ErrorBoundary, PanelContext, PanelContextProvider, SeriesVisibilityChangeMode } from '@grafana/ui'; +import { VizLegendOptions } from '@grafana/schema'; import { selectors } from '@grafana/e2e-selectors'; import { PanelHeader } from './PanelHeader/PanelHeader'; @@ -89,6 +90,7 @@ export class PanelChrome extends PureComponent { onAnnotationDelete: this.onAnnotationDelete, canAddAnnotations: () => Boolean(props.dashboard.meta.canEdit || props.dashboard.meta.canMakeEditable), onInstanceStateChange: this.onInstanceStateChange, + onToggleLegendSort: this.onToggleLegendSort, }, data: this.getInitialPanelDataState(), }; @@ -127,6 +129,35 @@ export class PanelChrome extends PureComponent { ); }; + onToggleLegendSort = (sortKey: string) => { + const legendOptions: VizLegendOptions = this.props.panel.options.legend; + + // We don't want to do anything when legend options are not available + if (!legendOptions) { + return; + } + + let sortDesc = legendOptions.sortDesc; + let sortBy = legendOptions.sortBy; + if (sortKey !== sortBy) { + sortDesc = undefined; + } + + // if already sort ascending, disable sorting + if (sortDesc === false) { + sortBy = undefined; + sortDesc = undefined; + } else { + sortDesc = !sortDesc; + sortBy = sortKey; + } + + this.onOptionsChange({ + ...this.props.panel.options, + legend: { ...legendOptions, sortBy, sortDesc }, + }); + }; + getInitialPanelDataState(): PanelData { return { state: LoadingState.NotStarted, diff --git a/public/app/plugins/panel/barchart/BarChart.tsx b/public/app/plugins/panel/barchart/BarChart.tsx index bc7fcafaefa..ee416b7b7e6 100644 --- a/public/app/plugins/panel/barchart/BarChart.tsx +++ b/public/app/plugins/panel/barchart/BarChart.tsx @@ -4,7 +4,7 @@ import { DataFrame, FieldType, TimeRange } from '@grafana/data'; import { GraphNG, GraphNGProps, PlotLegend, UPlotConfigBuilder, usePanelContext, useTheme2 } from '@grafana/ui'; import { LegendDisplayMode } from '@grafana/schema'; import { BarChartOptions } from './types'; -import { preparePlotConfigBuilder, preparePlotFrame } from './utils'; +import { isLegendOrdered, preparePlotConfigBuilder, preparePlotFrame } from './utils'; import { PropDiffFn } from '../../../../../packages/grafana-ui/src/components/GraphNG/GraphNG'; /** @@ -20,6 +20,7 @@ const propsToDiff: Array = [ 'groupWidth', 'stacking', 'showValue', + 'legend', (prev: BarChartProps, next: BarChartProps) => next.text?.valueSize === prev.text?.valueSize, ]; @@ -39,6 +40,11 @@ export const BarChart: React.FC = (props) => { }; const rawValue = (seriesIdx: number, valueIdx: number) => { + // When sorted by legend state.seriesIndex is not changed and is not equal to the sorted index of the field + if (isLegendOrdered(props.legend)) { + return frame0Ref.current!.fields[seriesIdx].values.get(valueIdx); + } + let field = frame0Ref.current!.fields.find( (f) => f.type === FieldType.number && f.state?.seriesIndex === seriesIdx - 1 ); diff --git a/public/app/plugins/panel/barchart/BarChartPanel.tsx b/public/app/plugins/panel/barchart/BarChartPanel.tsx index f766626c92d..ec3a8a5b24a 100755 --- a/public/app/plugins/panel/barchart/BarChartPanel.tsx +++ b/public/app/plugins/panel/barchart/BarChartPanel.tsx @@ -14,11 +14,7 @@ interface Props extends PanelProps {} export const BarChartPanel: React.FunctionComponent = ({ data, options, width, height, timeZone }) => { const theme = useTheme2(); - const { frames, warn } = useMemo(() => prepareGraphableFrames(data?.series, theme, options.stacking), [ - data, - theme, - options.stacking, - ]); + const { frames, warn } = useMemo(() => prepareGraphableFrames(data?.series, theme, options), [data, theme, options]); const orientation = useMemo(() => { if (!options.orientation || options.orientation === VizOrientation.Auto) { return width < height ? VizOrientation.Horizontal : VizOrientation.Vertical; diff --git a/public/app/plugins/panel/barchart/bars.ts b/public/app/plugins/panel/barchart/bars.ts index 51c7c907f68..9b4fdfdc319 100644 --- a/public/app/plugins/panel/barchart/bars.ts +++ b/public/app/plugins/panel/barchart/bars.ts @@ -3,7 +3,14 @@ import { pointWithin, Quadtree, Rect } from './quadtree'; import { distribute, SPACE_BETWEEN } from './distribute'; import { DataFrame, GrafanaTheme2 } from '@grafana/data'; import { calculateFontSize, PlotTooltipInterpolator } from '@grafana/ui'; -import { StackingMode, VisibilityMode, ScaleDirection, ScaleOrientation, VizTextDisplayOptions } from '@grafana/schema'; +import { + StackingMode, + VisibilityMode, + ScaleDirection, + ScaleOrientation, + VizTextDisplayOptions, + VizLegendOptions, +} from '@grafana/schema'; import { preparePlotData } from '../../../../../packages/grafana-ui/src/components/uPlot/utils'; const groupDistr = SPACE_BETWEEN; @@ -40,6 +47,7 @@ export interface BarsOptions { text?: VizTextDisplayOptions; onHover?: (seriesIdx: number, valueIdx: number) => void; onLeave?: (seriesIdx: number, valueIdx: number) => void; + legend?: VizLegendOptions; } /** @@ -311,9 +319,13 @@ export function getConfig(opts: BarsOptions, theme: GrafanaTheme2) { function prepData(frames: DataFrame[]) { alignedTotals = null; - return preparePlotData(frames, ({ totals }) => { - alignedTotals = totals; - }); + return preparePlotData( + frames, + ({ totals }) => { + alignedTotals = totals; + }, + opts.legend + ); } return { diff --git a/public/app/plugins/panel/barchart/utils.test.ts b/public/app/plugins/panel/barchart/utils.test.ts index 0b1333f4f22..61fcd36a497 100644 --- a/public/app/plugins/panel/barchart/utils.test.ts +++ b/public/app/plugins/panel/barchart/utils.test.ts @@ -144,7 +144,7 @@ describe('BarChart utils', () => { describe('prepareGraphableFrames', () => { it('will warn when there is no data in the response', () => { - const result = prepareGraphableFrames([], createTheme(), StackingMode.None); + const result = prepareGraphableFrames([], createTheme(), { stacking: StackingMode.None } as any); expect(result.warn).toEqual('No data in response'); }); @@ -155,7 +155,7 @@ describe('BarChart utils', () => { { name: 'value', values: [1, 2, 3, 4, 5] }, ], }); - const result = prepareGraphableFrames([df], createTheme(), StackingMode.None); + const result = prepareGraphableFrames([df], createTheme(), { stacking: StackingMode.None } as any); expect(result.warn).toEqual('Bar charts requires a string field'); expect(result.frames).toBeUndefined(); }); @@ -167,7 +167,7 @@ describe('BarChart utils', () => { { name: 'value', type: FieldType.boolean, values: [true, true, true, true, true] }, ], }); - const result = prepareGraphableFrames([df], createTheme(), StackingMode.None); + const result = prepareGraphableFrames([df], createTheme(), { stacking: StackingMode.None } as any); expect(result.warn).toEqual('No numeric fields found'); expect(result.frames).toBeUndefined(); }); @@ -179,7 +179,7 @@ describe('BarChart utils', () => { { name: 'value', values: [-10, NaN, 10, -Infinity, +Infinity] }, ], }); - const result = prepareGraphableFrames([df], createTheme(), StackingMode.None); + const result = prepareGraphableFrames([df], createTheme(), { stacking: StackingMode.None } as any); const field = result.frames![0].fields[1]; expect(field!.values.toArray()).toMatchInlineSnapshot(` @@ -192,5 +192,32 @@ describe('BarChart utils', () => { ] `); }); + + it('should sort fields when legend sortBy and sortDesc are set', () => { + const frame = new MutableDataFrame({ + fields: [ + { name: 'string', type: FieldType.string, values: ['a', 'b', 'c'] }, + { name: 'a', values: [-10, 20, 10], state: { calcs: { min: -10 } } }, + { name: 'b', values: [20, 20, 20], state: { calcs: { min: 20 } } }, + { name: 'c', values: [10, 10, 10], state: { calcs: { min: 10 } } }, + ], + }); + + const resultAsc = prepareGraphableFrames([frame], createTheme(), { + legend: { sortBy: 'Min', sortDesc: false }, + } as any); + expect(resultAsc.frames![0].fields[0].type).toBe(FieldType.string); + expect(resultAsc.frames![0].fields[1].name).toBe('a'); + expect(resultAsc.frames![0].fields[2].name).toBe('c'); + expect(resultAsc.frames![0].fields[3].name).toBe('b'); + + const resultDesc = prepareGraphableFrames([frame], createTheme(), { + legend: { sortBy: 'Min', sortDesc: true }, + } as any); + expect(resultDesc.frames![0].fields[0].type).toBe(FieldType.string); + expect(resultDesc.frames![0].fields[1].name).toBe('b'); + expect(resultDesc.frames![0].fields[2].name).toBe('c'); + expect(resultDesc.frames![0].fields[3].name).toBe('a'); + }); }); }); diff --git a/public/app/plugins/panel/barchart/utils.ts b/public/app/plugins/panel/barchart/utils.ts index 9254d0c2018..97610b12eb6 100644 --- a/public/app/plugins/panel/barchart/utils.ts +++ b/public/app/plugins/panel/barchart/utils.ts @@ -13,9 +13,17 @@ import { } from '@grafana/data'; import { BarChartFieldConfig, BarChartOptions, defaultBarChartFieldConfig } from './types'; import { BarsOptions, getConfig } from './bars'; -import { AxisPlacement, ScaleDirection, ScaleDistribution, ScaleOrientation, StackingMode } from '@grafana/schema'; +import { + AxisPlacement, + ScaleDirection, + ScaleDistribution, + ScaleOrientation, + StackingMode, + VizLegendOptions, +} from '@grafana/schema'; import { FIXED_UNIT, UPlotConfigBuilder, UPlotConfigPrepFn } from '@grafana/ui'; -import { collectStackingGroups } from '../../../../../packages/grafana-ui/src/components/uPlot/utils'; +import { collectStackingGroups, orderIdsByCalcs } from '../../../../../packages/grafana-ui/src/components/uPlot/utils'; +import { orderBy } from 'lodash'; /** @alpha */ function getBarCharScaleOrientation(orientation: VizOrientation) { @@ -47,6 +55,7 @@ export const preparePlotConfigBuilder: UPlotConfigPrepFn = ({ text, rawValue, allFrames, + legend, }) => { const builder = new UPlotConfigBuilder(); const defaultValueFormatter = (seriesIdx: number, value: any) => @@ -73,6 +82,7 @@ export const preparePlotConfigBuilder: UPlotConfigPrepFn = ({ formatValue, text, showValue, + legend, }; const config = getConfig(opts, theme); @@ -108,14 +118,14 @@ export const preparePlotConfigBuilder: UPlotConfigPrepFn = ({ }); let seriesIndex = 0; - + const legendOrdered = isLegendOrdered(legend); const stackingGroups: Map = new Map(); // iterate the y values for (let i = 1; i < frame.fields.length; i++) { const field = frame.fields[i]; - field.state!.seriesIndex = seriesIndex++; + seriesIndex++; const customConfig: BarChartFieldConfig = { ...defaultBarChartFieldConfig, ...field.config.custom }; @@ -144,9 +154,11 @@ export const preparePlotConfigBuilder: UPlotConfigPrepFn = ({ // The following properties are not used in the uPlot config, but are utilized as transport for legend config // PlotLegend currently gets unfiltered DataFrame[], so index must be into that field array, not the prepped frame's which we're iterating here dataFrameFieldIndex: { - fieldIndex: allFrames[0].fields.findIndex( - (f) => f.type === FieldType.number && f.state?.seriesIndex === seriesIndex - 1 - ), + fieldIndex: legendOrdered + ? i + : allFrames[0].fields.findIndex( + (f) => f.type === FieldType.number && f.state?.seriesIndex === seriesIndex - 1 + ), frameIndex: 0, }, }); @@ -192,7 +204,8 @@ export const preparePlotConfigBuilder: UPlotConfigPrepFn = ({ if (stackingGroups.size !== 0) { builder.setStacking(true); - for (const [_, seriesIdxs] of stackingGroups.entries()) { + for (const [_, seriesIds] of stackingGroups.entries()) { + const seriesIdxs = orderIdsByCalcs({ ids: seriesIds, legend, frame }); for (let j = seriesIdxs.length - 1; j > 0; j--) { builder.addBand({ series: [seriesIdxs[j], seriesIdxs[j - 1]], @@ -229,7 +242,7 @@ export function preparePlotFrame(data: DataFrame[]) { export function prepareGraphableFrames( series: DataFrame[], theme: GrafanaTheme2, - stacking: StackingMode + options: BarChartOptions ): { frames?: DataFrame[]; warn?: string } { if (!series?.length) { return { warn: 'No data in response' }; @@ -250,6 +263,7 @@ export function prepareGraphableFrames( }; } + const legendOrdered = isLegendOrdered(options.legend); let seriesIndex = 0; for (let frame of series) { @@ -268,7 +282,7 @@ export function prepareGraphableFrames( ...field.config.custom, stacking: { group: '_', - mode: stacking, + mode: options.stacking, }, }, }, @@ -282,7 +296,7 @@ export function prepareGraphableFrames( ), }; - if (stacking === StackingMode.Percent) { + if (options.stacking === StackingMode.Percent) { copy.config.unit = 'percentunit'; copy.display = getDisplayProcessor({ field: copy, theme }); } @@ -293,11 +307,29 @@ export function prepareGraphableFrames( } } + let orderedFields: Field[] | undefined; + + if (legendOrdered) { + orderedFields = orderBy( + fields, + ({ state }) => { + return state?.calcs?.[options.legend.sortBy!.toLowerCase()]; + }, + options.legend.sortDesc ? 'desc' : 'asc' + ); + // The string field needs to be the first one + if (orderedFields[orderedFields.length - 1].type === FieldType.string) { + orderedFields.unshift(orderedFields.pop()!); + } + } + frames.push({ ...frame, - fields, + fields: orderedFields || fields, }); } return { frames }; } + +export const isLegendOrdered = (options: VizLegendOptions) => Boolean(options?.sortBy && options.sortDesc !== null); From c550c6c25828c9bea0a25c4c79ff057e1b2a7f6a Mon Sep 17 00:00:00 2001 From: "renovate[bot]" <29139614+renovate[bot]@users.noreply.github.com> Date: Mon, 25 Oct 2021 10:27:05 +0100 Subject: [PATCH 04/49] Update dependency concurrently to v6 (#40711) Co-authored-by: Renovate Bot --- packages/grafana-toolkit/package.json | 2 +- yarn.lock | 187 ++++---------------------- 2 files changed, 29 insertions(+), 160 deletions(-) diff --git a/packages/grafana-toolkit/package.json b/packages/grafana-toolkit/package.json index 4bb218fce31..0e260a6a6f0 100644 --- a/packages/grafana-toolkit/package.json +++ b/packages/grafana-toolkit/package.json @@ -51,7 +51,7 @@ "chalk": "^2.4.2", "command-exists": "^1.2.8", "commander": "^5.0.0", - "concurrently": "4.1.0", + "concurrently": "6.3.0", "copy-webpack-plugin": "5.1.2", "css-loader": "3.4.2", "eslint": "7.21.0", diff --git a/yarn.lock b/yarn.lock index 1f25c96a81e..910b5e2fafc 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2587,7 +2587,7 @@ __metadata: chalk: ^2.4.2 command-exists: ^1.2.8 commander: ^5.0.0 - concurrently: 4.1.0 + concurrently: 6.3.0 copy-webpack-plugin: 5.1.2 css-loader: 3.4.2 eslint: 7.21.0 @@ -11930,17 +11930,6 @@ __metadata: languageName: node linkType: hard -"cliui@npm:^4.0.0": - version: 4.1.0 - resolution: "cliui@npm:4.1.0" - dependencies: - string-width: ^2.1.1 - strip-ansi: ^4.0.0 - wrap-ansi: ^2.0.0 - checksum: 0f8a77e55c66ab4400f8cc24a46e496af186ebfbf301709341a24c26d398200c2ccc5cac892566d586c3c393a079974f34f0ce05210df336f97b70805c02865e - languageName: node - linkType: hard - "cliui@npm:^6.0.0": version: 6.0.0 resolution: "cliui@npm:6.0.0" @@ -12382,22 +12371,21 @@ __metadata: languageName: node linkType: hard -"concurrently@npm:4.1.0": - version: 4.1.0 - resolution: "concurrently@npm:4.1.0" +"concurrently@npm:6.3.0": + version: 6.3.0 + resolution: "concurrently@npm:6.3.0" dependencies: - chalk: ^2.4.1 - date-fns: ^1.23.0 - lodash: ^4.17.10 - read-pkg: ^4.0.1 - rxjs: ^6.3.3 + chalk: ^4.1.0 + date-fns: ^2.16.1 + lodash: ^4.17.21 + rxjs: ^6.6.3 spawn-command: ^0.0.2-1 - supports-color: ^4.5.0 - tree-kill: ^1.1.0 - yargs: ^12.0.1 + supports-color: ^8.1.0 + tree-kill: ^1.2.2 + yargs: ^16.2.0 bin: - concurrently: ./bin/concurrently.js - checksum: a13b872814dddebf41deb09407edf75ca51d726e0639a0bf7f449bf03dc9a2e3219d7aa396a504c3ac2e8461c8151d3261cc4fefc7e11f57d95042e006b239fb + concurrently: bin/concurrently.js + checksum: fb68236899259a3b0c05d27db608ee9150f11fc785552c14937e5e1421c9448d03704cdc2acb5f842a91c465d50005b4229f348eb9594d84cbef54e5a93ffde7 languageName: node linkType: hard @@ -14032,13 +14020,20 @@ __metadata: languageName: node linkType: hard -"date-fns@npm:^1.23.0, date-fns@npm:^1.27.2": +"date-fns@npm:^1.27.2": version: 1.30.1 resolution: "date-fns@npm:1.30.1" checksum: 86b1f3269cbb1f3ee5ac9959775ea6600436f4ee2b78430cd427b41a0c9fabf740b1a5d401c085f3003539a6f4755c7c56c19fbd70ce11f6f673f6bc8075b710 languageName: node linkType: hard +"date-fns@npm:^2.16.1": + version: 2.25.0 + resolution: "date-fns@npm:2.25.0" + checksum: 8896dc1dde0ee5ef77616942423bfa11fa2017a5ac19457293b7aaedc8822ff94f0a14eaf93da573b09b601dc0149eb430988a046cc9f79a2eb15f8c66c9c50c + languageName: node + linkType: hard + "date-format@npm:^0.0.0": version: 0.0.0 resolution: "date-format@npm:0.0.0" @@ -16982,13 +16977,6 @@ fsevents@~2.1.2: languageName: node linkType: hard -"get-caller-file@npm:^1.0.1": - version: 1.0.3 - resolution: "get-caller-file@npm:1.0.3" - checksum: 2b90a7f848896abcebcdc0acc627a435bcf05b9cd280599bc980ebfcdc222416c3df12c24c4845f69adc4346728e8966f70b758f9369f3534182791dfbc25c05 - languageName: node - linkType: hard - "get-caller-file@npm:^2.0.1, get-caller-file@npm:^2.0.5": version: 2.0.5 resolution: "get-caller-file@npm:2.0.5" @@ -17962,13 +17950,6 @@ fsevents@~2.1.2: languageName: node linkType: hard -"has-flag@npm:^2.0.0": - version: 2.0.0 - resolution: "has-flag@npm:2.0.0" - checksum: 7d060d142ef6740c79991cb99afe5962b267e6e95538bf8b607026b9b1e7451288927bc8e7b4a9484a8b99935c0af023070f91ee49faef791ecd401dc58b2e8d - languageName: node - linkType: hard - "has-flag@npm:^3.0.0": version: 3.0.0 resolution: "has-flag@npm:3.0.0" @@ -19113,13 +19094,6 @@ fsevents@~2.1.2: languageName: node linkType: hard -"invert-kv@npm:^2.0.0": - version: 2.0.0 - resolution: "invert-kv@npm:2.0.0" - checksum: 52ea317354101ad6127c6e4c1c6a2d27ae8d3010b6438b60d76d6a920e55410e03547f97f9d1f52031becf5656bbef91d36ee7daa9e26ebc374a9cb342e1f127 - languageName: node - linkType: hard - "ip-regex@npm:^2.1.0": version: 2.1.0 resolution: "ip-regex@npm:2.1.0" @@ -21355,15 +21329,6 @@ fsevents@~2.1.2: languageName: node linkType: hard -"lcid@npm:^2.0.0": - version: 2.0.0 - resolution: "lcid@npm:2.0.0" - dependencies: - invert-kv: ^2.0.0 - checksum: 278e27b5a0707cf9ab682146963ebff2328795be10cd6f8ea8edae293439325d345ac5e33079cce77ac3a86a3dcfb97a34f279dbc46b03f3e419aa39b5915a16 - languageName: node - linkType: hard - "lerc@npm:^3.0.0": version: 3.0.0 resolution: "lerc@npm:3.0.0" @@ -21894,7 +21859,7 @@ fsevents@~2.1.2: languageName: node linkType: hard -"lodash@npm:4.17.21, lodash@npm:^4, lodash@npm:^4.0.0, lodash@npm:^4.1.1, lodash@npm:^4.17.10, lodash@npm:^4.17.12, lodash@npm:^4.17.14, lodash@npm:^4.17.15, lodash@npm:^4.17.19, lodash@npm:^4.17.20, lodash@npm:^4.17.21, lodash@npm:^4.17.3, lodash@npm:^4.17.4, lodash@npm:^4.17.5, lodash@npm:^4.7.0, lodash@npm:~4.17.10, lodash@npm:~4.17.15, lodash@npm:~4.17.21": +"lodash@npm:4.17.21, lodash@npm:^4, lodash@npm:^4.0.0, lodash@npm:^4.1.1, lodash@npm:^4.17.12, lodash@npm:^4.17.14, lodash@npm:^4.17.15, lodash@npm:^4.17.19, lodash@npm:^4.17.20, lodash@npm:^4.17.21, lodash@npm:^4.17.3, lodash@npm:^4.17.4, lodash@npm:^4.17.5, lodash@npm:^4.7.0, lodash@npm:~4.17.10, lodash@npm:~4.17.15, lodash@npm:~4.17.21": version: 4.17.21 resolution: "lodash@npm:4.17.21" checksum: eb835a2e51d381e561e508ce932ea50a8e5a68f4ebdd771ea240d3048244a8d13658acbd502cd4829768c56f2e16bdd4340b9ea141297d472517b83868e677f7 @@ -22212,7 +22177,7 @@ fsevents@~2.1.2: languageName: node linkType: hard -"map-age-cleaner@npm:^0.1.1, map-age-cleaner@npm:^0.1.3": +"map-age-cleaner@npm:^0.1.3": version: 0.1.3 resolution: "map-age-cleaner@npm:0.1.3" dependencies: @@ -22432,17 +22397,6 @@ fsevents@~2.1.2: languageName: node linkType: hard -"mem@npm:^4.0.0": - version: 4.3.0 - resolution: "mem@npm:4.3.0" - dependencies: - map-age-cleaner: ^0.1.1 - mimic-fn: ^2.0.0 - p-is-promise: ^2.0.0 - checksum: cf488608e5d59c6cb68004b70de317222d4be9f857fd535dfa6a108e04f40821479c080bc763c417b1030569d303538c59d441280078cfce07fefd1c523f98ef - languageName: node - linkType: hard - "mem@npm:^8.1.1": version: 8.1.1 resolution: "mem@npm:8.1.1" @@ -22687,7 +22641,7 @@ fsevents@~2.1.2: languageName: node linkType: hard -"mimic-fn@npm:^2.0.0, mimic-fn@npm:^2.1.0": +"mimic-fn@npm:^2.1.0": version: 2.1.0 resolution: "mimic-fn@npm:2.1.0" checksum: d2421a3444848ce7f84bd49115ddacff29c15745db73f54041edc906c14b131a38d05298dae3081667627a59b2eb1ca4b436ff2e1b80f69679522410418b478a @@ -24177,17 +24131,6 @@ fsevents@~2.1.2: languageName: node linkType: hard -"os-locale@npm:^3.0.0": - version: 3.1.0 - resolution: "os-locale@npm:3.1.0" - dependencies: - execa: ^1.0.0 - lcid: ^2.0.0 - mem: ^4.0.0 - checksum: 53c542b11af3c5fe99624b09c7882b6944f9ae7c69edbc6006b7d42cff630b1f7fd9d63baf84ed31d1ef02b34823b6b31f23a1ecdd593757873d716bc6374099 - languageName: node - linkType: hard - "os-tmpdir@npm:^1.0.0, os-tmpdir@npm:~1.0.2": version: 1.0.2 resolution: "os-tmpdir@npm:1.0.2" @@ -24283,13 +24226,6 @@ fsevents@~2.1.2: languageName: node linkType: hard -"p-is-promise@npm:^2.0.0": - version: 2.1.0 - resolution: "p-is-promise@npm:2.1.0" - checksum: c9a8248c8b5e306475a5d55ce7808dbce4d4da2e3d69526e4991a391a7809bfd6cfdadd9bf04f1c96a3db366c93d9a0f5ee81d949e7b1684c4e0f61f747199ef - languageName: node - linkType: hard - "p-limit@npm:^1.1.0": version: 1.3.0 resolution: "p-limit@npm:1.3.0" @@ -28373,17 +28309,6 @@ fsevents@~2.1.2: languageName: node linkType: hard -"read-pkg@npm:^4.0.1": - version: 4.0.1 - resolution: "read-pkg@npm:4.0.1" - dependencies: - normalize-package-data: ^2.3.2 - parse-json: ^4.0.0 - pify: ^3.0.0 - checksum: 56193535486c50a0a40039e4a92f68676362f5a7160628ca4d856c62509e125220f69c562a32940dcc51e3dcd38211af69756bbb5b8a1aed1d09be1bedd1e1a5 - languageName: node - linkType: hard - "read-pkg@npm:^5.2.0": version: 5.2.0 resolution: "read-pkg@npm:5.2.0" @@ -28973,13 +28898,6 @@ fsevents@~2.1.2: languageName: node linkType: hard -"require-main-filename@npm:^1.0.1": - version: 1.0.1 - resolution: "require-main-filename@npm:1.0.1" - checksum: 1fef30754da961f4e13c450c3eb60c7ae898a529c6ad6fa708a70bd2eed01564ceb299187b2899f5562804d797a059f39a5789884d0ac7b7ae1defc68fba4abf - languageName: node - linkType: hard - "require-main-filename@npm:^2.0.0": version: 2.0.0 resolution: "require-main-filename@npm:2.0.0" @@ -29459,7 +29377,7 @@ resolve@~1.19.0: languageName: node linkType: hard -"rxjs@npm:^6.3.3, rxjs@npm:^6.4.0, rxjs@npm:^6.6.0, rxjs@npm:^6.6.7": +"rxjs@npm:^6.3.3, rxjs@npm:^6.4.0, rxjs@npm:^6.6.0, rxjs@npm:^6.6.3, rxjs@npm:^6.6.7": version: 6.6.7 resolution: "rxjs@npm:6.6.7" dependencies: @@ -31381,15 +31299,6 @@ resolve@~1.19.0: languageName: node linkType: hard -"supports-color@npm:^4.5.0": - version: 4.5.0 - resolution: "supports-color@npm:4.5.0" - dependencies: - has-flag: ^2.0.0 - checksum: 6da4f498d5c71e8619f06e4a11d16f044105faf7590b5b005fc84933fbefdf72c2b4e5b7174c66da6ddc68e7f6ef56cc960a5ebd6f2d542d910e259e61b02335 - languageName: node - linkType: hard - "supports-color@npm:^5.3.0, supports-color@npm:^5.5.0": version: 5.5.0 resolution: "supports-color@npm:5.5.0" @@ -31417,7 +31326,7 @@ resolve@~1.19.0: languageName: node linkType: hard -"supports-color@npm:^8.0.0, supports-color@npm:^8.1.1": +"supports-color@npm:^8.0.0, supports-color@npm:^8.1.0, supports-color@npm:^8.1.1": version: 8.1.1 resolution: "supports-color@npm:8.1.1" dependencies: @@ -32179,7 +32088,7 @@ resolve@~1.19.0: languageName: node linkType: hard -"tree-kill@npm:^1.1.0": +"tree-kill@npm:^1.2.2": version: 1.2.2 resolution: "tree-kill@npm:1.2.2" bin: @@ -34152,16 +34061,6 @@ typescript@~4.4.2: languageName: node linkType: hard -"wrap-ansi@npm:^2.0.0": - version: 2.1.0 - resolution: "wrap-ansi@npm:2.1.0" - dependencies: - string-width: ^1.0.1 - strip-ansi: ^3.0.1 - checksum: 2dacd4b3636f7a53ee13d4d0fe7fa2ed9ad81e9967e17231924ea88a286ec4619a78288de8d41881ee483f4449ab2c0287cde8154ba1bd0126c10271101b2ee3 - languageName: node - linkType: hard - "wrap-ansi@npm:^3.0.1": version: 3.0.1 resolution: "wrap-ansi@npm:3.0.1" @@ -34358,7 +34257,7 @@ typescript@~4.4.2: languageName: node linkType: hard -"y18n@npm:^3.2.1 || ^4.0.0, y18n@npm:^4.0.0": +"y18n@npm:^4.0.0": version: 4.0.3 resolution: "y18n@npm:4.0.3" checksum: 014dfcd9b5f4105c3bb397c1c8c6429a9df004aa560964fb36732bfb999bfe83d45ae40aeda5b55d21b1ee53d8291580a32a756a443e064317953f08025b1aa4 @@ -34414,16 +34313,6 @@ typescript@~4.4.2: languageName: node linkType: hard -"yargs-parser@npm:^11.1.1": - version: 11.1.1 - resolution: "yargs-parser@npm:11.1.1" - dependencies: - camelcase: ^5.0.0 - decamelize: ^1.2.0 - checksum: 91a82f4e6295745269f5683d1ab11d636f1d2fa732fb1c1795ad4637f31feb54530c2072ca2c2e39d3c4d506c3645214ff08c781f4a5b48fc959788706a54f83 - languageName: node - linkType: hard - "yargs-parser@npm:^18.1.2": version: 18.1.3 resolution: "yargs-parser@npm:18.1.3" @@ -34434,26 +34323,6 @@ typescript@~4.4.2: languageName: node linkType: hard -"yargs@npm:^12.0.1": - version: 12.0.5 - resolution: "yargs@npm:12.0.5" - dependencies: - cliui: ^4.0.0 - decamelize: ^1.2.0 - find-up: ^3.0.0 - get-caller-file: ^1.0.1 - os-locale: ^3.0.0 - require-directory: ^2.1.1 - require-main-filename: ^1.0.1 - set-blocking: ^2.0.0 - string-width: ^2.0.0 - which-module: ^2.0.0 - y18n: ^3.2.1 || ^4.0.0 - yargs-parser: ^11.1.1 - checksum: 716f467be3f4dd5ed346f7e07eabfbf4b915e818bf2e6582b27c8d23f17c6ee59126b1c6896234d0ca1f615ee09d1901602677c5ee294540e87f914cd27a3c9b - languageName: node - linkType: hard - "yargs@npm:^15.0.2, yargs@npm:^15.4.1": version: 15.4.1 resolution: "yargs@npm:15.4.1" From 889d4683a1e9682b314d33ba9d815ccea03522ea Mon Sep 17 00:00:00 2001 From: Marcus Efraimsson Date: Mon, 25 Oct 2021 11:49:49 +0200 Subject: [PATCH 05/49] Datasources: Set response size metric based on actual bytes read (#40303) Fixes #33372 --- pkg/infra/httpclient/count_bytes_reader.go | 39 +++++++++++++++++++ .../httpclient/count_bytes_reader_test.go | 38 ++++++++++++++++++ .../datasource_metrics_middleware.go | 18 +++++---- 3 files changed, 87 insertions(+), 8 deletions(-) create mode 100644 pkg/infra/httpclient/count_bytes_reader.go create mode 100644 pkg/infra/httpclient/count_bytes_reader_test.go diff --git a/pkg/infra/httpclient/count_bytes_reader.go b/pkg/infra/httpclient/count_bytes_reader.go new file mode 100644 index 00000000000..278a0b378c3 --- /dev/null +++ b/pkg/infra/httpclient/count_bytes_reader.go @@ -0,0 +1,39 @@ +package httpclient + +import ( + "io" +) + +type CloseCallbackFunc func(bytesRead int64) + +// CountBytesReader counts the total amount of bytes read from the underlying reader. +// +// The provided callback func will be called before the underlying reader is closed. +func CountBytesReader(reader io.ReadCloser, callback CloseCallbackFunc) io.ReadCloser { + if reader == nil { + panic("reader cannot be nil") + } + + if callback == nil { + panic("callback cannot be nil") + } + + return &countBytesReader{reader: reader, callback: callback} +} + +type countBytesReader struct { + reader io.ReadCloser + callback CloseCallbackFunc + counter int64 +} + +func (r *countBytesReader) Read(p []byte) (int, error) { + n, err := r.reader.Read(p) + r.counter += int64(n) + return n, err +} + +func (r *countBytesReader) Close() error { + r.callback(r.counter) + return r.reader.Close() +} diff --git a/pkg/infra/httpclient/count_bytes_reader_test.go b/pkg/infra/httpclient/count_bytes_reader_test.go new file mode 100644 index 00000000000..d8cb077328d --- /dev/null +++ b/pkg/infra/httpclient/count_bytes_reader_test.go @@ -0,0 +1,38 @@ +package httpclient + +import ( + "fmt" + "io/ioutil" + "strings" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestCountBytesReader(t *testing.T) { + tcs := []struct { + body string + expectedBytesCount int64 + }{ + {body: "d", expectedBytesCount: 1}, + {body: "dummy", expectedBytesCount: 5}, + } + + for index, tc := range tcs { + t.Run(fmt.Sprintf("Test CountBytesReader %d", index), func(t *testing.T) { + body := ioutil.NopCloser(strings.NewReader(tc.body)) + var actualBytesRead int64 + + readCloser := CountBytesReader(body, func(bytesRead int64) { + actualBytesRead = bytesRead + }) + + bodyBytes, err := ioutil.ReadAll(readCloser) + require.NoError(t, err) + err = readCloser.Close() + require.NoError(t, err) + require.Equal(t, tc.expectedBytesCount, actualBytesRead) + require.Equal(t, string(bodyBytes), tc.body) + }) + } +} diff --git a/pkg/infra/httpclient/httpclientprovider/datasource_metrics_middleware.go b/pkg/infra/httpclient/httpclientprovider/datasource_metrics_middleware.go index cdf2addcf11..2709ed6ab76 100644 --- a/pkg/infra/httpclient/httpclientprovider/datasource_metrics_middleware.go +++ b/pkg/infra/httpclient/httpclientprovider/datasource_metrics_middleware.go @@ -3,7 +3,8 @@ package httpclientprovider import ( "net/http" - "github.com/grafana/grafana-plugin-sdk-go/backend/httpclient" + sdkhttpclient "github.com/grafana/grafana-plugin-sdk-go/backend/httpclient" + "github.com/grafana/grafana/pkg/infra/httpclient" "github.com/grafana/grafana/pkg/infra/metrics/metricutil" "github.com/prometheus/client_golang/prometheus" "github.com/prometheus/client_golang/prometheus/promhttp" @@ -56,8 +57,8 @@ const DataSourceMetricsMiddlewareName = "metrics" var executeMiddlewareFunc = executeMiddleware -func DataSourceMetricsMiddleware() httpclient.Middleware { - return httpclient.NamedMiddlewareFunc(DataSourceMetricsMiddlewareName, func(opts httpclient.Options, next http.RoundTripper) http.RoundTripper { +func DataSourceMetricsMiddleware() sdkhttpclient.Middleware { + return sdkhttpclient.NamedMiddlewareFunc(DataSourceMetricsMiddlewareName, func(opts sdkhttpclient.Options, next http.RoundTripper) http.RoundTripper { if opts.Labels == nil { return next } @@ -81,7 +82,7 @@ func DataSourceMetricsMiddleware() httpclient.Middleware { } func executeMiddleware(next http.RoundTripper, datasourceLabel prometheus.Labels) http.RoundTripper { - return httpclient.RoundTripperFunc(func(r *http.Request) (*http.Response, error) { + return sdkhttpclient.RoundTripperFunc(func(r *http.Request) (*http.Response, error) { requestCounter := datasourceRequestCounter.MustCurryWith(datasourceLabel) requestSummary := datasourceRequestSummary.MustCurryWith(datasourceLabel) requestInFlight := datasourceRequestsInFlight.With(datasourceLabel) @@ -94,10 +95,11 @@ func executeMiddleware(next http.RoundTripper, datasourceLabel prometheus.Labels if err != nil { return nil, err } - // we avoid measuring contentlength less than zero because it indicates - // that the content size is unknown. https://godoc.org/github.com/badu/http#Response - if res != nil && res.ContentLength > 0 { - responseSizeSummary.Observe(float64(res.ContentLength)) + + if res != nil { + res.Body = httpclient.CountBytesReader(res.Body, func(bytesRead int64) { + responseSizeSummary.Observe(float64(bytesRead)) + }) } return res, nil From d1aefa179296d8acb34f99f68c20264798ee6967 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jean-Philippe=20Qu=C3=A9m=C3=A9ner?= Date: Mon, 25 Oct 2021 11:53:41 +0200 Subject: [PATCH 06/49] Alerting: fix ngalert alertmanager SQL Syntax Errors (#40827) * test kvstore in intregration tests with different databases * escape 'key' in delete query * export quote and use it in kvstore --- pkg/infra/kvstore/kvstore_test.go | 3 +++ pkg/infra/kvstore/sql.go | 6 ++++-- pkg/services/sqlstore/sqlstore.go | 5 +++++ 3 files changed, 12 insertions(+), 2 deletions(-) diff --git a/pkg/infra/kvstore/kvstore_test.go b/pkg/infra/kvstore/kvstore_test.go index 2a7b2087176..90211cc3fe5 100644 --- a/pkg/infra/kvstore/kvstore_test.go +++ b/pkg/infra/kvstore/kvstore_test.go @@ -1,3 +1,6 @@ +//go:build integration +// +build integration + package kvstore import ( diff --git a/pkg/infra/kvstore/sql.go b/pkg/infra/kvstore/sql.go index 829cabeaa83..32287f0eecf 100644 --- a/pkg/infra/kvstore/sql.go +++ b/pkg/infra/kvstore/sql.go @@ -2,6 +2,7 @@ package kvstore import ( "context" + "fmt" "time" "github.com/grafana/grafana/pkg/infra/log" @@ -88,7 +89,8 @@ func (kv *kvStoreSQL) Set(ctx context.Context, orgId int64, namespace string, ke // Del deletes an item from the store. func (kv *kvStoreSQL) Del(ctx context.Context, orgId int64, namespace string, key string) error { err := kv.sqlStore.WithDbSession(ctx, func(dbSession *sqlstore.DBSession) error { - _, err := dbSession.Exec("DELETE FROM kv_store WHERE org_id=? and namespace=? and key=?", orgId, namespace, key) + query := fmt.Sprintf("DELETE FROM kv_store WHERE org_id=? and namespace=? and %s=?", kv.sqlStore.Quote("key")) + _, err := dbSession.Exec(query, orgId, namespace, key) return err }) return err @@ -99,7 +101,7 @@ func (kv *kvStoreSQL) Del(ctx context.Context, orgId int64, namespace string, ke func (kv *kvStoreSQL) Keys(ctx context.Context, orgId int64, namespace string, keyPrefix string) ([]Key, error) { var keys []Key err := kv.sqlStore.WithDbSession(ctx, func(dbSession *sqlstore.DBSession) error { - query := dbSession.Where("namespace = ?", namespace).And("\"key\" LIKE ?", keyPrefix+"%") + query := dbSession.Where("namespace = ?", namespace).And(fmt.Sprintf("%s LIKE ?", kv.sqlStore.Quote("key")), keyPrefix+"%") if orgId != AllOrganizations { query.And("org_id = ?", orgId) } diff --git a/pkg/services/sqlstore/sqlstore.go b/pkg/services/sqlstore/sqlstore.go index 1a851fd9369..821aa181178 100644 --- a/pkg/services/sqlstore/sqlstore.go +++ b/pkg/services/sqlstore/sqlstore.go @@ -161,6 +161,11 @@ func (ss *SQLStore) Reset() error { return ss.ensureMainOrgAndAdminUser() } +// Quote quotes the value in the used SQL dialect +func (ss *SQLStore) Quote(value string) string { + return ss.engine.Quote(value) +} + func (ss *SQLStore) ensureMainOrgAndAdminUser() error { ctx := context.Background() err := ss.WithTransactionalDbSession(ctx, func(sess *DBSession) error { From 9e030970aa473512918c6ed21c87d1bbe8ffe7c4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=A1bor=20Farkas?= Date: Mon, 25 Oct 2021 12:20:13 +0200 Subject: [PATCH 07/49] prometheus: remove handling of control+enter (#40869) we are standardizing on shift+enter --- .../datasource/prometheus/components/PromExploreExtraField.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/public/app/plugins/datasource/prometheus/components/PromExploreExtraField.tsx b/public/app/plugins/datasource/prometheus/components/PromExploreExtraField.tsx index 50ba5d10d24..8cd46d316c3 100644 --- a/public/app/plugins/datasource/prometheus/components/PromExploreExtraField.tsx +++ b/public/app/plugins/datasource/prometheus/components/PromExploreExtraField.tsx @@ -48,7 +48,7 @@ export const PromExploreExtraField: React.FC = memo( } function onReturnKeyDown(e: React.KeyboardEvent) { - if (e.key === 'Enter' && (e.shiftKey || e.ctrlKey)) { + if (e.key === 'Enter' && e.shiftKey) { onRunQuery(); } } From 91c0b5a47fe1c9203939f41308001c2e203b02f9 Mon Sep 17 00:00:00 2001 From: "renovate[bot]" <29139614+renovate[bot]@users.noreply.github.com> Date: Mon, 25 Oct 2021 11:49:24 +0100 Subject: [PATCH 08/49] Dependencies: Ignore d3-force for now (#40818) Co-authored-by: Ashley Harrison --- .github/renovate.json5 | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.github/renovate.json5 b/.github/renovate.json5 index 946a15c5bef..86c8c68e472 100644 --- a/.github/renovate.json5 +++ b/.github/renovate.json5 @@ -5,6 +5,7 @@ "enabledManagers": ["npm"], "ignoreDeps": [ "d3", + "d3-force", // we should bump this once we move to esm modules "husky", "slate", "slate-plain-serializer", @@ -22,7 +23,7 @@ "matchPaths": ["grafana-toolkit/package.json"], "ignoreDeps": [ "copy-webpack-plugin", // need to wait for Grafana 9 to upgrade toolkit to webpack 5 - "css-loader", // need to wait for Grafana 9 to upgrade toolkit to webpack 5 + "css-loader", // need to wait for Grafana 9 to upgrade toolkit to webpack 5 ] } ], From 54af57b8e66fcb398a4c3f7b7b1036de7292c4dd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Torkel=20=C3=96degaard?= Date: Mon, 25 Oct 2021 13:55:06 +0200 Subject: [PATCH 09/49] VisualizationSelection: Real previews of suitable visualisation and options based on current data (#40527) * Initial pass to move panel state to it's own, and make it by key not panel.id * Progress * Not making much progress, having panel.key be mutable is causing a lot of issues * Think this is starting to work * Began fixing tests * Add selector * Bug fixes and changes to cleanup, and fixing all flicking when switching library panels * Removed console.log * fixes after merge * fixing tests * fixing tests * Added new test for changePlugin thunk * Initial struture in place * responding to state changes in another part of the state * bha * going in a different direction * This is getting exciting * minor * More structure * More real * Added builder to reduce boiler plate * Lots of progress * Adding more visualizations * More smarts * tweaks * suggestions * Move to separate view * Refactoring to builder concept * Before hover preview test * Increase line width in preview * More suggestions * Removed old elements of onSuggestVisualizations * Don't call suggestion suppliers if there is no data * Restore card styles to only borders * Changing supplier interface to support data vs option suggestion scenario * Renamed functions * Add dynamic width support * not sure about this * Improve suggestions * Improve suggestions * Single grid/list * Store vis select pane & size * Prep for option suggestions * more suggestions * Name/title option for preview cards * Improve barchart suggestions * Support suggestions when there are no data * Minor change * reverted some changes * Improve suggestions for stacking * Removed size option * starting on unit tests, hit cyclic dependency issue * muuu * First test for getting suggestion seems to work, going to bed * add missing file * A basis for more unit tests * More tests * More unit tests * Fixed unit tests * Update * Some extreme scenarios * Added basic e2e test * Added another unit test for changePanelPlugin action * More cleanup * Minor tweak * add wait to e2e test * Renamed function and cleanup of unused function * Adding search support and adding search test to e2e test --- e2e/suite1/specs/visualization-suggestions.ts | 32 ++ .../grafana-data/src/panel/PanelPlugin.ts | 19 ++ packages/grafana-data/src/types/dashboard.ts | 2 +- packages/grafana-data/src/types/dataFrame.ts | 2 +- .../grafana-data/src/types/fieldOverrides.ts | 2 +- packages/grafana-data/src/types/panel.ts | 140 +++++++- .../src/selectors/components.ts | 3 + .../src/components/PanelRenderer.tsx | 4 +- .../components/FilterInput/FilterInput.tsx | 77 ++--- .../grafana-ui/src/utils/useCombinedRefs.ts | 21 ++ public/app/angular/AngularApp.ts | 2 +- public/app/{core => angular}/partials.ts | 0 .../PanelTypeFilter/PanelTypeFilter.tsx | 2 +- public/app/core/config.ts | 2 +- public/app/core/core.ts | 1 - public/app/core/reducers/root.ts | 2 + .../PanelEditor/AngularPanelOptions.tsx | 2 +- .../components/PanelEditor/OptionsPane.tsx | 2 +- .../PanelEditor/OptionsPaneOptions.tsx | 1 + .../components/PanelEditor/PanelEditor.tsx | 31 +- .../PanelEditor/VisualizationSelectPane.tsx | 86 ++--- .../components/PanelEditor/state/reducers.ts | 4 + .../dashboard/dashgrid/DashboardPanel.tsx | 8 +- .../dashboard/dashgrid/PanelChrome.test.tsx | 2 + .../dashboard/dashgrid/PanelChrome.tsx | 8 +- .../features/dashboard/state/PanelModel.ts | 2 - .../app/features/dashboard/state/reducers.ts | 2 - .../LibraryPanelsSearch.test.tsx | 6 +- .../panel/components/CannotVisualizeData.tsx | 51 +++ .../VizTypePicker/VisualizationPreview.tsx | 128 ++++++++ .../VisualizationSuggestions.tsx | 106 ++++++ .../VizTypePicker/VizTypePicker.tsx | 123 ++----- .../panel/components/VizTypePicker/types.ts | 8 + .../app/features/panel/state/actions.test.ts | 40 ++- public/app/features/panel/state/actions.ts | 41 ++- .../panel/state/getAllSuggestions.test.ts | 305 ++++++++++++++++++ .../features/panel/state/getAllSuggestions.ts | 30 ++ .../panel/state/getOptionSuggestions.ts | 17 + public/app/features/panel/state/util.ts | 47 +++ public/app/plugins/panel/alertlist/module.tsx | 4 +- .../plugins/panel/alertlist/suggestions.ts | 20 ++ public/app/plugins/panel/barchart/module.tsx | 4 +- .../app/plugins/panel/barchart/suggestions.ts | 94 ++++++ public/app/plugins/panel/bargauge/module.tsx | 4 +- .../app/plugins/panel/bargauge/suggestions.ts | 115 +++++++ public/app/plugins/panel/dashlist/module.tsx | 4 +- .../app/plugins/panel/dashlist/suggestions.ts | 20 ++ public/app/plugins/panel/gauge/module.tsx | 2 + public/app/plugins/panel/gauge/suggestions.ts | 85 +++++ public/app/plugins/panel/graph/module.ts | 2 +- public/app/plugins/panel/piechart/module.tsx | 4 +- .../app/plugins/panel/piechart/suggestions.ts | 80 +++++ public/app/plugins/panel/stat/module.tsx | 2 + public/app/plugins/panel/stat/suggestions.ts | 77 +++++ .../plugins/panel/state-timeline/module.tsx | 4 +- .../panel/state-timeline/suggestions.ts | 38 +++ public/app/plugins/panel/table/module.tsx | 4 +- public/app/plugins/panel/table/suggestions.ts | 22 ++ public/app/plugins/panel/text/module.tsx | 4 +- public/app/plugins/panel/text/suggestions.ts | 29 ++ .../panel/timeseries/TimeSeriesPanel.tsx | 4 +- .../app/plugins/panel/timeseries/module.tsx | 2 + .../plugins/panel/timeseries/suggestions.ts | 169 ++++++++++ public/app/plugins/panel/timeseries/utils.ts | 21 +- public/app/types/suggestions.ts | 25 ++ public/test/jest-setup.ts | 1 + 66 files changed, 1968 insertions(+), 233 deletions(-) create mode 100644 e2e/suite1/specs/visualization-suggestions.ts create mode 100644 packages/grafana-ui/src/utils/useCombinedRefs.ts rename public/app/{core => angular}/partials.ts (100%) create mode 100644 public/app/features/panel/components/CannotVisualizeData.tsx create mode 100644 public/app/features/panel/components/VizTypePicker/VisualizationPreview.tsx create mode 100644 public/app/features/panel/components/VizTypePicker/VisualizationSuggestions.tsx create mode 100644 public/app/features/panel/components/VizTypePicker/types.ts create mode 100644 public/app/features/panel/state/getAllSuggestions.test.ts create mode 100644 public/app/features/panel/state/getAllSuggestions.ts create mode 100644 public/app/features/panel/state/getOptionSuggestions.ts create mode 100644 public/app/features/panel/state/util.ts create mode 100644 public/app/plugins/panel/alertlist/suggestions.ts create mode 100644 public/app/plugins/panel/barchart/suggestions.ts create mode 100644 public/app/plugins/panel/bargauge/suggestions.ts create mode 100644 public/app/plugins/panel/dashlist/suggestions.ts create mode 100644 public/app/plugins/panel/gauge/suggestions.ts create mode 100644 public/app/plugins/panel/piechart/suggestions.ts create mode 100644 public/app/plugins/panel/stat/suggestions.ts create mode 100644 public/app/plugins/panel/state-timeline/suggestions.ts create mode 100644 public/app/plugins/panel/table/suggestions.ts create mode 100644 public/app/plugins/panel/text/suggestions.ts create mode 100644 public/app/plugins/panel/timeseries/suggestions.ts create mode 100644 public/app/types/suggestions.ts diff --git a/e2e/suite1/specs/visualization-suggestions.ts b/e2e/suite1/specs/visualization-suggestions.ts new file mode 100644 index 00000000000..10340e4d86f --- /dev/null +++ b/e2e/suite1/specs/visualization-suggestions.ts @@ -0,0 +1,32 @@ +import { e2e } from '@grafana/e2e'; + +const PANEL_UNDER_TEST = 'Interpolation: linear'; + +e2e.scenario({ + describeName: 'Visualization suggestions', + itName: 'Should be shown and clickable', + addScenarioDataSource: false, + addScenarioDashBoard: false, + skipScenario: false, + scenario: () => { + e2e.flows.openDashboard({ uid: 'TkZXxlNG3' }); + e2e.flows.openPanelMenuItem(e2e.flows.PanelMenuItems.Edit, PANEL_UNDER_TEST); + + // Try visualization suggestions + e2e.components.PanelEditor.toggleVizPicker().click(); + e2e().contains('Suggestions').click(); + cy.wait(1000); + + // Verify we see suggestions + e2e.components.VisualizationPreview.card('Line chart').should('be.visible'); + + // Verify search works + e2e().get('[placeholder="Search for..."]').type('Table'); + // Should no longer see line chart + e2e.components.VisualizationPreview.card('Line chart').should('not.exist'); + + // Select a visualisation + e2e.components.VisualizationPreview.card('Table').click(); + e2e.components.Panels.Visualization.Table.header().should('be.visible'); + }, +}); diff --git a/packages/grafana-data/src/panel/PanelPlugin.ts b/packages/grafana-data/src/panel/PanelPlugin.ts index 8ad6ea3ce18..a2efd5cb665 100644 --- a/packages/grafana-data/src/panel/PanelPlugin.ts +++ b/packages/grafana-data/src/panel/PanelPlugin.ts @@ -8,6 +8,7 @@ import { PanelTypeChangedHandler, FieldConfigProperty, PanelPluginDataSupport, + VisualizationSuggestionsSupplier, } from '../types'; import { FieldConfigEditorBuilder, PanelOptionsEditorBuilder } from '../utils/OptionsUIBuilders'; import { ComponentClass, ComponentType } from 'react'; @@ -104,6 +105,7 @@ export class PanelPlugin< }; private optionsSupplier?: PanelOptionsSupplier; + private suggestionsSupplier?: VisualizationSuggestionsSupplier; panel: ComponentType> | null; editor?: ComponentClass>; @@ -354,4 +356,21 @@ export class PanelPlugin< return this; } + + /** + * Sets function that can return visualization examples and suggestions. + * @alpha + */ + setSuggestionsSupplier(supplier: VisualizationSuggestionsSupplier) { + this.suggestionsSupplier = supplier; + return this; + } + + /** + * Returns the suggestions supplier + * @alpha + */ + getSuggestionsSupplier(): VisualizationSuggestionsSupplier | undefined { + return this.suggestionsSupplier; + } } diff --git a/packages/grafana-data/src/types/dashboard.ts b/packages/grafana-data/src/types/dashboard.ts index df1af6165a7..fc69f8f75c9 100644 --- a/packages/grafana-data/src/types/dashboard.ts +++ b/packages/grafana-data/src/types/dashboard.ts @@ -10,7 +10,7 @@ export enum DashboardCursorSync { /** * @public */ -export interface PanelModel { +export interface PanelModel { /** ID of the panel within the current dashboard */ id: number; diff --git a/packages/grafana-data/src/types/dataFrame.ts b/packages/grafana-data/src/types/dataFrame.ts index 3b055ba84d9..0190e75fbfc 100644 --- a/packages/grafana-data/src/types/dataFrame.ts +++ b/packages/grafana-data/src/types/dataFrame.ts @@ -24,7 +24,7 @@ export enum FieldType { * * Plugins may extend this with additional properties. Something like series overrides */ -export interface FieldConfig { +export interface FieldConfig { /** * The display value for this field. This supports template variables blank is auto */ diff --git a/packages/grafana-data/src/types/fieldOverrides.ts b/packages/grafana-data/src/types/fieldOverrides.ts index 7a162599ab3..57532652ea2 100644 --- a/packages/grafana-data/src/types/fieldOverrides.ts +++ b/packages/grafana-data/src/types/fieldOverrides.ts @@ -49,7 +49,7 @@ export const isSystemOverride = (override: ConfigOverrideRule): override is Syst return typeof (override as SystemConfigOverrideRule)?.__systemRef === 'string'; }; -export interface FieldConfigSource { +export interface FieldConfigSource { // Defaults applied to all numeric fields defaults: FieldConfig; diff --git a/packages/grafana-data/src/types/panel.ts b/packages/grafana-data/src/types/panel.ts index 6b9f6406091..a0f10643d44 100644 --- a/packages/grafana-data/src/types/panel.ts +++ b/packages/grafana-data/src/types/panel.ts @@ -2,7 +2,7 @@ import { DataQueryError, DataQueryRequest, DataQueryTimings } from './datasource import { PluginMeta } from './plugin'; import { ScopedVars } from './ScopedVars'; import { LoadingState } from './data'; -import { DataFrame } from './dataFrame'; +import { DataFrame, FieldType } from './dataFrame'; import { AbsoluteTimeRange, TimeRange, TimeZone } from './time'; import { EventBus } from '../events'; import { FieldConfigSource } from './fieldOverrides'; @@ -12,6 +12,8 @@ import { OptionsEditorItem } from './OptionsUIRegistryBuilder'; import { OptionEditorConfig } from './options'; import { AlertStateInfo } from './alerts'; import { PanelModel } from './dashboard'; +import { DataTransformerConfig } from './transformations'; +import { defaultsDeep } from 'lodash'; export type InterpolateFunction = (value: string, scopedVars?: ScopedVars, format?: string | Function) => string; @@ -58,7 +60,7 @@ export interface PanelData { timeRange: TimeRange; } -export interface PanelProps { +export interface PanelProps { /** ID of the panel within the current dashboard */ id: number; @@ -182,3 +184,137 @@ export interface PanelPluginDataSupport { annotations: boolean; alertStates: boolean; } + +/** + * @alpha + */ +export interface VisualizationSuggestion { + /** Name of suggestion */ + name: string; + /** Description */ + description?: string; + /** Panel plugin id */ + pluginId: string; + /** Panel plugin options */ + options?: Partial; + /** Panel plugin field options */ + fieldConfig?: FieldConfigSource>; + /** Data transformations */ + transformations?: DataTransformerConfig[]; + /** Tweak for small preview */ + previewModifier?: (suggestion: VisualizationSuggestion) => void; +} + +/** + * @alpha + */ +export interface PanelDataSummary { + hasData?: boolean; + rowCountTotal: number; + rowCountMax: number; + frameCount: number; + numberFieldCount: number; + timeFieldCount: number; + stringFieldCount: number; + hasNumberField?: boolean; + hasTimeField?: boolean; + hasStringField?: boolean; +} + +/** + * @alpha + */ +export class VisualizationSuggestionsBuilder { + /** Current data */ + data?: PanelData; + /** Current panel & options */ + panel?: PanelModel; + /** Summary stats for current data */ + dataSummary: PanelDataSummary; + + private list: VisualizationSuggestion[] = []; + + constructor(data?: PanelData, panel?: PanelModel) { + this.data = data; + this.panel = panel; + this.dataSummary = this.computeDataSummary(); + } + + getListAppender(defaults: VisualizationSuggestion) { + return new VisualizationSuggestionsListAppender(this.list, defaults); + } + + private computeDataSummary() { + const frames = this.data?.series || []; + + let numberFieldCount = 0; + let timeFieldCount = 0; + let stringFieldCount = 0; + let rowCountTotal = 0; + let rowCountMax = 0; + + for (const frame of frames) { + rowCountTotal += frame.length; + + for (const field of frame.fields) { + switch (field.type) { + case FieldType.number: + numberFieldCount += 1; + break; + case FieldType.time: + timeFieldCount += 1; + break; + case FieldType.string: + stringFieldCount += 1; + break; + } + } + + if (frame.length > rowCountMax) { + rowCountMax = frame.length; + } + } + + return { + numberFieldCount, + timeFieldCount, + stringFieldCount, + rowCountTotal, + rowCountMax, + frameCount: frames.length, + hasData: rowCountTotal > 0, + hasTimeField: timeFieldCount > 0, + hasNumberField: numberFieldCount > 0, + hasStringField: stringFieldCount > 0, + }; + } + + getList() { + return this.list; + } +} + +/** + * @alpha + */ +export type VisualizationSuggestionsSupplier = { + /** + * Adds good suitable suggestions for the current data + */ + getSuggestionsForData: (builder: VisualizationSuggestionsBuilder) => void; +}; + +/** + * Helps with typings and defaults + * @alpha + */ +export class VisualizationSuggestionsListAppender { + constructor( + private list: VisualizationSuggestion[], + private defaults: VisualizationSuggestion + ) {} + + append(overrides: Partial>) { + this.list.push(defaultsDeep(overrides, this.defaults)); + } +} diff --git a/packages/grafana-e2e-selectors/src/selectors/components.ts b/packages/grafana-e2e-selectors/src/selectors/components.ts index 037a7a04caa..36c3d3c6e7b 100644 --- a/packages/grafana-e2e-selectors/src/selectors/components.ts +++ b/packages/grafana-e2e-selectors/src/selectors/components.ts @@ -262,4 +262,7 @@ export const Components = { PanelAlertTabContent: { content: 'Unified alert editor tab content', }, + VisualizationPreview: { + card: (name: string) => `data-testid suggestion-${name}`, + }, }; diff --git a/packages/grafana-runtime/src/components/PanelRenderer.tsx b/packages/grafana-runtime/src/components/PanelRenderer.tsx index 6053c9d2070..a2f8cc341cd 100644 --- a/packages/grafana-runtime/src/components/PanelRenderer.tsx +++ b/packages/grafana-runtime/src/components/PanelRenderer.tsx @@ -13,10 +13,10 @@ export interface PanelRendererProps

; onOptionsChange?: (options: P) => void; onChangeTimeRange?: (timeRange: AbsoluteTimeRange) => void; - fieldConfig?: FieldConfigSource; + fieldConfig?: FieldConfigSource>; timeZone?: string; width: number; height: number; diff --git a/packages/grafana-ui/src/components/FilterInput/FilterInput.tsx b/packages/grafana-ui/src/components/FilterInput/FilterInput.tsx index ca993414cb8..d394ae335c1 100644 --- a/packages/grafana-ui/src/components/FilterInput/FilterInput.tsx +++ b/packages/grafana-ui/src/components/FilterInput/FilterInput.tsx @@ -1,47 +1,48 @@ -import React, { FC } from 'react'; +import React, { HTMLProps } from 'react'; import { escapeStringForRegex, unEscapeStringFromRegex } from '@grafana/data'; import { Button, Icon, Input } from '..'; -import { useFocus } from '../Input/utils'; +import { useCombinedRefs } from '../../utils/useCombinedRefs'; -export interface Props { +export interface Props extends Omit, 'onChange'> { value: string | undefined; - placeholder?: string; width?: number; onChange: (value: string) => void; - onKeyDown?: (event: React.KeyboardEvent) => void; - autoFocus?: boolean; } -export const FilterInput: FC = ({ value, placeholder, width, onChange, onKeyDown, autoFocus }) => { - const [inputRef, setInputFocus] = useFocus(); - const suffix = - value !== '' ? ( - - ) : null; +export const FilterInput = React.forwardRef( + ({ value, width, onChange, ...restProps }, ref) => { + const innerRef = React.useRef(null); + const combinedRef = useCombinedRefs(ref, innerRef) as React.Ref; - return ( - } - ref={inputRef} - suffix={suffix} - width={width} - type="text" - value={value ? unEscapeStringFromRegex(value) : ''} - onChange={(event) => onChange(escapeStringForRegex(event.currentTarget.value))} - onKeyDown={onKeyDown} - placeholder={placeholder} - /> - ); -}; + const suffix = + value !== '' ? ( + + ) : null; + + return ( + } + suffix={suffix} + width={width} + type="text" + value={value ? unEscapeStringFromRegex(value) : ''} + onChange={(event) => onChange(escapeStringForRegex(event.currentTarget.value))} + {...restProps} + ref={combinedRef} + /> + ); + } +); + +FilterInput.displayName = 'FilterInput'; diff --git a/packages/grafana-ui/src/utils/useCombinedRefs.ts b/packages/grafana-ui/src/utils/useCombinedRefs.ts new file mode 100644 index 00000000000..e158a9630db --- /dev/null +++ b/packages/grafana-ui/src/utils/useCombinedRefs.ts @@ -0,0 +1,21 @@ +import React from 'react'; + +export function useCombinedRefs(...refs: any) { + const targetRef = React.useRef(null); + + React.useEffect(() => { + refs.forEach((ref: any) => { + if (!ref) { + return; + } + + if (typeof ref === 'function') { + ref(targetRef.current); + } else { + ref.current = targetRef.current; + } + }); + }, [refs]); + + return targetRef; +} diff --git a/public/app/angular/AngularApp.ts b/public/app/angular/AngularApp.ts index 2124d8093fa..dabcafbeea2 100644 --- a/public/app/angular/AngularApp.ts +++ b/public/app/angular/AngularApp.ts @@ -14,7 +14,7 @@ import { extend } from 'lodash'; import { getTimeSrv } from 'app/features/dashboard/services/TimeSrv'; import { getTemplateSrv } from '@grafana/runtime'; import './panel/all'; - +import './partials'; export class AngularApp { ngModuleDependencies: any[]; preBootModules: any[]; diff --git a/public/app/core/partials.ts b/public/app/angular/partials.ts similarity index 100% rename from public/app/core/partials.ts rename to public/app/angular/partials.ts diff --git a/public/app/core/components/PanelTypeFilter/PanelTypeFilter.tsx b/public/app/core/components/PanelTypeFilter/PanelTypeFilter.tsx index a2c5845f3d8..aa3fddb0778 100644 --- a/public/app/core/components/PanelTypeFilter/PanelTypeFilter.tsx +++ b/public/app/core/components/PanelTypeFilter/PanelTypeFilter.tsx @@ -1,6 +1,6 @@ import React, { useCallback, useMemo, useState } from 'react'; import { GrafanaTheme2, PanelPluginMeta, SelectableValue } from '@grafana/data'; -import { getAllPanelPluginMeta } from '../../../features/panel/components/VizTypePicker/VizTypePicker'; +import { getAllPanelPluginMeta } from 'app/features/panel/state/util'; import { Icon, resetSelectStyles, MultiSelect, useStyles2 } from '@grafana/ui'; import { css } from '@emotion/css'; diff --git a/public/app/core/config.ts b/public/app/core/config.ts index a55c22a8b26..3ff14355734 100644 --- a/public/app/core/config.ts +++ b/public/app/core/config.ts @@ -1,5 +1,5 @@ import { config, GrafanaBootConfig } from '@grafana/runtime'; -import { PluginState } from '../../../packages/grafana-data/src'; +import { PluginState } from '@grafana/data'; // Legacy binding paths export { config, GrafanaBootConfig as Settings }; diff --git a/public/app/core/core.ts b/public/app/core/core.ts index c90abd0781d..aaaa92bf10b 100644 --- a/public/app/core/core.ts +++ b/public/app/core/core.ts @@ -8,7 +8,6 @@ import '../angular/rebuild_on_change'; import '../angular/give_focus'; import '../angular/diff-view'; import './jquery_extended'; -import './partials'; import './components/jsontree/jsontree'; import './components/code_editor/code_editor'; import './components/colorpicker/spectrum_picker'; diff --git a/public/app/core/reducers/root.ts b/public/app/core/reducers/root.ts index 77ed750e0e3..c0799e93ff8 100644 --- a/public/app/core/reducers/root.ts +++ b/public/app/core/reducers/root.ts @@ -15,6 +15,7 @@ import organizationReducers from 'app/features/org/state/reducers'; import ldapReducers from 'app/features/admin/state/reducers'; import templatingReducers from 'app/features/variables/state/reducers'; import importDashboardReducers from 'app/features/manage-dashboards/state/reducers'; +import panelEditorReducers from 'app/features/dashboard/components/PanelEditor/state/reducers'; import panelsReducers from 'app/features/panel/state/reducers'; const rootReducers = { @@ -33,6 +34,7 @@ const rootReducers = { ...ldapReducers, ...templatingReducers, ...importDashboardReducers, + ...panelEditorReducers, ...panelsReducers, }; diff --git a/public/app/features/dashboard/components/PanelEditor/AngularPanelOptions.tsx b/public/app/features/dashboard/components/PanelEditor/AngularPanelOptions.tsx index 99778dd3463..605b9573856 100644 --- a/public/app/features/dashboard/components/PanelEditor/AngularPanelOptions.tsx +++ b/public/app/features/dashboard/components/PanelEditor/AngularPanelOptions.tsx @@ -83,7 +83,7 @@ export class AngularPanelOptionsUnconnected extends PureComponent { const panelCtrl: PanelCtrl = scope.$$childHead.ctrl; panelCtrl.initEditMode(); panelCtrl.onPluginTypeChange = (plugin: PanelPluginMeta) => { - changePanelPlugin(panel, plugin.id); + changePanelPlugin({ panel, pluginId: plugin.id }); }; let template = ''; diff --git a/public/app/features/dashboard/components/PanelEditor/OptionsPane.tsx b/public/app/features/dashboard/components/PanelEditor/OptionsPane.tsx index 16c57345bbb..73e73e8ee24 100644 --- a/public/app/features/dashboard/components/PanelEditor/OptionsPane.tsx +++ b/public/app/features/dashboard/components/PanelEditor/OptionsPane.tsx @@ -45,7 +45,7 @@ export const OptionsPane: React.FC = ({ )} - {isVizPickerOpen && } + {isVizPickerOpen && } ); }; diff --git a/public/app/features/dashboard/components/PanelEditor/OptionsPaneOptions.tsx b/public/app/features/dashboard/components/PanelEditor/OptionsPaneOptions.tsx index 5d56dbbaa7e..6a105fb7a7c 100644 --- a/public/app/features/dashboard/components/PanelEditor/OptionsPaneOptions.tsx +++ b/public/app/features/dashboard/components/PanelEditor/OptionsPaneOptions.tsx @@ -35,6 +35,7 @@ export const OptionsPaneOptions: React.FC = (props) => { const mainBoxElements: React.ReactNode[] = []; const isSearching = searchQuery.length > 0; const optionRadioFilters = useMemo(getOptionRadioFilters, []); + const allOptions = isPanelModelLibraryPanel(panel) ? [libraryPanelOptions, panelFrameOptions, ...vizOptions] : [panelFrameOptions, ...vizOptions]; diff --git a/public/app/features/dashboard/components/PanelEditor/PanelEditor.tsx b/public/app/features/dashboard/components/PanelEditor/PanelEditor.tsx index 275a060a1e4..2e25852c090 100644 --- a/public/app/features/dashboard/components/PanelEditor/PanelEditor.tsx +++ b/public/app/features/dashboard/components/PanelEditor/PanelEditor.tsx @@ -218,7 +218,7 @@ export class PanelEditorUnconnected extends PureComponent { updatePanelEditorUIState({ isPanelOptionsVisible: !uiState.isPanelOptionsVisible }); }; - renderPanel(styles: EditorStyles, noTabsBelow: boolean) { + renderPanel(styles: EditorStyles, isOnlyPanel: boolean) { const { dashboard, panel, uiState, tableViewEnabled } = this.props; return ( @@ -232,7 +232,7 @@ export class PanelEditorUnconnected extends PureComponent { } // If no tabs limit height so panel does not extend to edge - if (noTabsBelow) { + if (isOnlyPanel) { height -= config.theme2.spacing.gridSize * 2; } @@ -270,21 +270,23 @@ export class PanelEditorUnconnected extends PureComponent { renderPanelAndEditor(styles: EditorStyles) { const { panel, dashboard, plugin, tab } = this.props; const tabs = getPanelEditorTabs(tab, plugin); + const isOnlyPanel = tabs.length === 0; + const panelPane = this.renderPanel(styles, isOnlyPanel); - if (tabs.length > 0) { - return [ - this.renderPanel(styles, false), -

- -
, - ]; + if (tabs.length === 0) { + return panelPane; } - return this.renderPanel(styles, true); + return [ + panelPane, +
+ +
, + ]; } renderTemplateVariables(styles: EditorStyles) { @@ -529,6 +531,7 @@ export const getStyles = stylesFactory((theme: GrafanaTheme, props: Props) => { justify-content: center; align-items: center; position: relative; + flex-direction: column; `, }; }); diff --git a/public/app/features/dashboard/components/PanelEditor/VisualizationSelectPane.tsx b/public/app/features/dashboard/components/PanelEditor/VisualizationSelectPane.tsx index 87af8f1b822..a0b953cb568 100644 --- a/public/app/features/dashboard/components/PanelEditor/VisualizationSelectPane.tsx +++ b/public/app/features/dashboard/components/PanelEditor/VisualizationSelectPane.tsx @@ -1,45 +1,43 @@ import React, { FC, useCallback, useEffect, useRef, useState } from 'react'; import { css } from '@emotion/css'; -import { GrafanaTheme, PanelPluginMeta, SelectableValue } from '@grafana/data'; -import { Button, CustomScrollbar, Icon, Input, RadioButtonGroup, useStyles } from '@grafana/ui'; +import { GrafanaTheme, PanelData, SelectableValue } from '@grafana/data'; +import { Button, CustomScrollbar, FilterInput, RadioButtonGroup, useStyles } from '@grafana/ui'; import { changePanelPlugin } from '../../../panel/state/actions'; import { PanelModel } from '../../state/PanelModel'; import { useDispatch, useSelector } from 'react-redux'; -import { - filterPluginList, - getAllPanelPluginMeta, - VizTypePicker, -} from '../../../panel/components/VizTypePicker/VizTypePicker'; +import { VizTypePicker } from '../../../panel/components/VizTypePicker/VizTypePicker'; import { Field } from '@grafana/ui/src/components/Forms/Field'; import { PanelLibraryOptionsGroup } from 'app/features/library-panels/components/PanelLibraryOptionsGroup/PanelLibraryOptionsGroup'; import { toggleVizPicker } from './state/reducers'; import { selectors } from '@grafana/e2e-selectors'; import { getPanelPluginWithFallback } from '../../state/selectors'; +import { VizTypeChangeDetails } from 'app/features/panel/components/VizTypePicker/types'; +import { VisualizationSuggestions } from 'app/features/panel/components/VizTypePicker/VisualizationSuggestions'; +import { useLocalStorage } from 'react-use'; interface Props { panel: PanelModel; + data?: PanelData; } -export const VisualizationSelectPane: FC = ({ panel }) => { +export const VisualizationSelectPane: FC = ({ panel, data }) => { const plugin = useSelector(getPanelPluginWithFallback(panel.type)); const [searchQuery, setSearchQuery] = useState(''); - const [listMode, setListMode] = useState(ListMode.Visualizations); + const [listMode, setListMode] = useLocalStorage(`VisualizationSelectPane.ListMode`, ListMode.Visualizations); const dispatch = useDispatch(); const styles = useStyles(getStyles); const searchRef = useRef(null); - const onPluginTypeChange = useCallback( - (meta: PanelPluginMeta, withModKey: boolean) => { - if (meta.id !== plugin.meta.id) { - dispatch(changePanelPlugin(panel, meta.id)); - } + const onVizChange = useCallback( + (pluginChange: VizTypeChangeDetails) => { + dispatch(changePanelPlugin({ panel: panel, ...pluginChange })); // close viz picker unless a mod key is pressed while clicking - if (!withModKey) { + if (!pluginChange.withModKey) { dispatch(toggleVizPicker(false)); } }, - [dispatch, panel, plugin.meta.id] + [dispatch, panel] ); // Give Search input focus when using radio button switch list mode @@ -53,27 +51,20 @@ export const VisualizationSelectPane: FC = ({ panel }) => { dispatch(toggleVizPicker(false)); }; - const onKeyPress = useCallback( - (e: React.KeyboardEvent) => { - if (e.key === 'Enter') { - const query = e.currentTarget.value; - const plugins = getAllPanelPluginMeta(); - const match = filterPluginList(plugins, query, plugin.meta); + // const onKeyPress = useCallback( + // (e: React.KeyboardEvent) => { + // if (e.key === 'Enter') { + // const query = e.currentTarget.value; + // const plugins = getAllPanelPluginMeta(); + // const match = filterPluginList(plugins, query, plugin.meta); - if (match && match.length) { - onPluginTypeChange(match[0], false); - } - } - }, - [onPluginTypeChange, plugin.meta] - ); - - const suffix = - searchQuery !== '' ? ( - - ) : null; + // if (match && match.length) { + // onPluginTypeChange(match[0], false); + // } + // } + // }, + // [onPluginTypeChange, plugin.meta] + // ); if (!plugin) { return null; @@ -81,6 +72,7 @@ export const VisualizationSelectPane: FC = ({ panel }) => { const radioOptions: Array> = [ { label: 'Visualizations', value: ListMode.Visualizations }, + { label: 'Suggestions', value: ListMode.Suggestions }, { label: 'Library panels', value: ListMode.LibraryPanels, @@ -92,13 +84,11 @@ export const VisualizationSelectPane: FC = ({ panel }) => {
- setSearchQuery(e.currentTarget.value)} - onKeyPress={onKeyPress} - prefix={} - suffix={suffix} + onChange={setSearchQuery} ref={searchRef} + autoFocus={true} placeholder="Search for..." />
0) { LogsVolumePanelContent = ( ; + +export function applyGraphStyle(config: FieldConfig, style: ExploreGraphStyle): FieldConfig { + return produce(config, (draft) => { + if (draft.defaults.custom === undefined) { + draft.defaults.custom = {}; + } + + const { custom } = draft.defaults; + + if (custom.stacking === undefined) { + custom.stacking = { group: 'A' }; + } + + switch (style) { + case 'lines': + custom.drawStyle = GraphDrawStyle.Line; + custom.stacking.mode = StackingMode.None; + custom.fillOpacity = 0; + break; + case 'bars': + custom.drawStyle = GraphDrawStyle.Bars; + custom.stacking.mode = StackingMode.None; + custom.fillOpacity = 100; + break; + case 'points': + custom.drawStyle = GraphDrawStyle.Points; + custom.stacking.mode = StackingMode.None; + custom.fillOpacity = 0; + break; + case 'stacked_lines': + custom.drawStyle = GraphDrawStyle.Line; + custom.stacking.mode = StackingMode.Normal; + custom.fillOpacity = 100; + break; + case 'stacked_bars': + custom.drawStyle = GraphDrawStyle.Bars; + custom.stacking.mode = StackingMode.Normal; + custom.fillOpacity = 100; + break; + default: { + // should never happen + // NOTE: casting to `never` will cause typescript + // to verify that the switch statement checks every possible + // enum-value + const invalidValue: never = style; + throw new Error(`Invalid graph-style: ${invalidValue}`); + } + } + }); +} diff --git a/public/app/features/explore/state/explorePane.ts b/public/app/features/explore/state/explorePane.ts index d1da8319c0d..160f5e5e949 100644 --- a/public/app/features/explore/state/explorePane.ts +++ b/public/app/features/explore/state/explorePane.ts @@ -8,6 +8,7 @@ import { ensureQueries, generateNewKeyAndAddRefIdIfMissing, getTimeRangeFromUrl, + ExploreGraphStyle, } from 'app/core/utils/explore'; import { ExploreId, ExploreItemState } from 'app/types/explore'; import { queryReducer, runQueries, setQueriesAction } from './query'; @@ -19,6 +20,7 @@ import { loadAndInitDatasource, createEmptyQueryResponse, getUrlStateFromPaneState, + storeGraphStyle, } from './utils'; import { createAction, PayloadAction } from '@reduxjs/toolkit'; import { EventBusExtended, DataQuery, ExploreUrlState, TimeRange, HistoryItem, DataSourceApi } from '@grafana/data'; @@ -76,6 +78,20 @@ export function changeSize( return changeSizeAction({ exploreId, height, width }); } +interface ChangeGraphStylePayload { + exploreId: ExploreId; + graphStyle: ExploreGraphStyle; +} + +const changeGraphStyleAction = createAction('explore/changeGraphStyle'); + +export function changeGraphStyle(exploreId: ExploreId, graphStyle: ExploreGraphStyle): ThunkResult { + return async (dispatch, getState) => { + storeGraphStyle(graphStyle); + dispatch(changeGraphStyleAction({ exploreId, graphStyle })); + }; +} + /** * Initialize Explore state with state from the URL and the React component. * Call this only on components for with the Explore state has not been initialized. @@ -200,6 +216,11 @@ export const paneReducer = (state: ExploreItemState = makeExplorePaneState(), ac return { ...state, containerWidth }; } + if (changeGraphStyleAction.match(action)) { + const { graphStyle } = action.payload; + return { ...state, graphStyle }; + } + if (initializeExploreAction.match(action)) { const { containerWidth, eventBridge, queries, range, originPanelId, datasourceInstance, history } = action.payload; diff --git a/public/app/features/explore/state/utils.ts b/public/app/features/explore/state/utils.ts index f9b0aab0ae2..5fa76e41950 100644 --- a/public/app/features/explore/state/utils.ts +++ b/public/app/features/explore/state/utils.ts @@ -12,7 +12,12 @@ import { import { ExploreItemState } from 'app/types/explore'; import { getDatasourceSrv } from '../../plugins/datasource_srv'; import store from '../../../core/store'; -import { clearQueryKeys, lastUsedDatasourceKeyForOrgId } from '../../../core/utils/explore'; +import { + clearQueryKeys, + ExploreGraphStyle, + lastUsedDatasourceKeyForOrgId, + toGraphStyle, +} from '../../../core/utils/explore'; import { toRawTimeRange } from '../utils/time'; export const DEFAULT_RANGE = { @@ -20,6 +25,16 @@ export const DEFAULT_RANGE = { to: 'now', }; +const GRAPH_STYLE_KEY = 'grafana.explore.style.graph'; +export const storeGraphStyle = (graphStyle: string): void => { + store.set(GRAPH_STYLE_KEY, graphStyle); +}; + +const loadGraphStyle = (): ExploreGraphStyle => { + const data = store.get(GRAPH_STYLE_KEY); + return toGraphStyle(data); +}; + /** * Returns a fresh Explore area state */ @@ -52,6 +67,7 @@ export const makeExplorePaneState = (): ExploreItemState => ({ cache: [], logsVolumeDataProvider: undefined, logsVolumeData: undefined, + graphStyle: loadGraphStyle(), }); export const createEmptyQueryResponse = (): PanelData => ({ diff --git a/public/app/types/explore.ts b/public/app/types/explore.ts index 0544a8d0255..0414bec869a 100644 --- a/public/app/types/explore.ts +++ b/public/app/types/explore.ts @@ -14,6 +14,7 @@ import { EventBusExtended, DataQueryResponse, } from '@grafana/data'; +import { ExploreGraphStyle } from 'app/core/utils/explore'; export enum ExploreId { left = 'left', @@ -167,6 +168,9 @@ export interface ExploreItemState { logsVolumeDataProvider?: Observable; logsVolumeDataSubscription?: SubscriptionLike; logsVolumeData?: DataQueryResponse; + + /* explore graph style */ + graphStyle: ExploreGraphStyle; } export interface ExploreUpdateState { From 8ee3afa4c337923b5380f51b94683dfd21d6a7b6 Mon Sep 17 00:00:00 2001 From: Isabella Siu Date: Tue, 26 Oct 2021 10:17:12 -0400 Subject: [PATCH 36/49] CloudMonitoring: switch from ApplyRoute to AuthMiddleware (#40787) --- pkg/tsdb/cloudmonitoring/cloudmonitoring.go | 93 +++++++------------ pkg/tsdb/cloudmonitoring/httpclient.go | 56 +++++++++++ .../cloudmonitoring/time_series_filter.go | 2 +- pkg/tsdb/cloudmonitoring/time_series_query.go | 2 +- pkg/tsdb/cloudmonitoring/utils.go | 26 ++++++ 5 files changed, 117 insertions(+), 62 deletions(-) create mode 100644 pkg/tsdb/cloudmonitoring/httpclient.go create mode 100644 pkg/tsdb/cloudmonitoring/utils.go diff --git a/pkg/tsdb/cloudmonitoring/cloudmonitoring.go b/pkg/tsdb/cloudmonitoring/cloudmonitoring.go index 4ed67d7d1a2..a319682e766 100644 --- a/pkg/tsdb/cloudmonitoring/cloudmonitoring.go +++ b/pkg/tsdb/cloudmonitoring/cloudmonitoring.go @@ -23,7 +23,6 @@ import ( "github.com/grafana/grafana-plugin-sdk-go/backend/instancemgmt" "github.com/grafana/grafana-plugin-sdk-go/data" - "github.com/grafana/grafana/pkg/api/pluginproxy" "github.com/grafana/grafana/pkg/infra/httpclient" "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/plugins" @@ -63,6 +62,8 @@ var ( ) const ( + dsName = "stackdriver" + gceAuthentication string = "gce" jwtAuthentication string = "jwt" metricQueryType string = "metrics" @@ -87,7 +88,7 @@ func ProvideService(cfg *setting.Cfg, httpClientProvider httpclient.Provider, pl QueryDataHandler: s, }) - if err := s.backendPluginManager.Register("stackdriver", factory); err != nil { + if err := s.backendPluginManager.Register(dsName, factory); err != nil { slog.Error("Failed to register plugin", "error", err) } return s @@ -112,9 +113,10 @@ type datasourceInfo struct { url string authenticationType string defaultProject string + clientEmail string + tokenUri string client *http.Client - jsonData map[string]interface{} decryptedSecureJSONData map[string]string } @@ -126,16 +128,6 @@ func newInstanceSettings(httpClientProvider httpclient.Provider) datasource.Inst return nil, fmt.Errorf("error reading settings: %w", err) } - opts, err := settings.HTTPClientOptions() - if err != nil { - return nil, err - } - - client, err := httpClientProvider.New(opts) - if err != nil { - return nil, err - } - authType := jwtAuthentication if authTypeOverride, ok := jsonData["authenticationType"].(string); ok && authTypeOverride != "" { authType = authTypeOverride @@ -146,16 +138,38 @@ func newInstanceSettings(httpClientProvider httpclient.Provider) datasource.Inst defaultProject = jsonData["defaultProject"].(string) } - return &datasourceInfo{ + var clientEmail string + if jsonData["clientEmail"] != nil { + clientEmail = jsonData["clientEmail"].(string) + } + + var tokenUri string + if jsonData["tokenUri"] != nil { + tokenUri = jsonData["tokenUri"].(string) + } + + dsInfo := &datasourceInfo{ id: settings.ID, updated: settings.Updated, url: settings.URL, authenticationType: authType, defaultProject: defaultProject, - client: client, - jsonData: jsonData, + clientEmail: clientEmail, + tokenUri: tokenUri, decryptedSecureJSONData: settings.DecryptedSecureJSONData, - }, nil + } + + opts, err := settings.HTTPClientOptions() + if err != nil { + return nil, err + } + + dsInfo.client, err = newHTTPClient(dsInfo, opts, httpClientProvider) + if err != nil { + return nil, err + } + + return dsInfo, nil } } @@ -340,14 +354,6 @@ func (s *Service) buildQueryExecutors(req *backend.QueryDataRequest) ([]cloudMon return cloudMonitoringQueryExecutors, nil } -func reverse(s string) string { - chars := []rune(s) - for i, j := 0, len(chars)-1; i < j; i, j = i+1, j-1 { - chars[i], chars[j] = chars[j], chars[i] - } - return string(chars) -} - func interpolateFilterWildcards(value string) string { matches := strings.Count(value, "*") switch { @@ -478,19 +484,6 @@ func calculateAlignmentPeriod(alignmentPeriod string, intervalMs int64, duration return alignmentPeriod } -func toSnakeCase(str string) string { - return strings.ToLower(matchAllCap.ReplaceAllString(str, "${1}_${2}")) -} - -func containsLabel(labels []string, newLabel string) bool { - for _, val := range labels { - if val == newLabel { - return true - } - } - return false -} - func formatLegendKeys(metricType string, defaultMetricName string, labels map[string]string, additionalLabels map[string]string, query *cloudMonitoringTimeSeriesFilter) string { if query.AliasBy == "" { @@ -589,34 +582,14 @@ func (s *Service) createRequest(ctx context.Context, pluginCtx backend.PluginCon if body != nil { method = http.MethodPost } - req, err := http.NewRequest(method, "https://monitoring.googleapis.com/", body) + req, err := http.NewRequest(method, cloudMonitoringRoute.url, body) if err != nil { slog.Error("Failed to create request", "error", err) return nil, fmt.Errorf("failed to create request: %w", err) } req.Header.Set("Content-Type", "application/json") - - // find plugin - plugin := s.pluginManager.GetDataSource(pluginCtx.PluginID) - if plugin == nil { - return nil, errors.New("unable to find datasource plugin CloudMonitoring") - } - - var cloudMonitoringRoute *plugins.AppPluginRoute - for _, route := range plugin.Routes { - if route.Path == "cloudmonitoring" { - cloudMonitoringRoute = route - break - } - } - - pluginproxy.ApplyRoute(ctx, req, proxyPass, cloudMonitoringRoute, pluginproxy.DSInfo{ - ID: dsInfo.id, - Updated: dsInfo.updated, - JSONData: dsInfo.jsonData, - DecryptedSecureJSONData: dsInfo.decryptedSecureJSONData, - }, s.cfg) + req.URL.Path = proxyPass return req, nil } diff --git a/pkg/tsdb/cloudmonitoring/httpclient.go b/pkg/tsdb/cloudmonitoring/httpclient.go new file mode 100644 index 00000000000..711795be6dd --- /dev/null +++ b/pkg/tsdb/cloudmonitoring/httpclient.go @@ -0,0 +1,56 @@ +package cloudmonitoring + +import ( + "net/http" + + "github.com/grafana/grafana-google-sdk-go/pkg/tokenprovider" + "github.com/grafana/grafana-plugin-sdk-go/backend/httpclient" + infrahttp "github.com/grafana/grafana/pkg/infra/httpclient" +) + +var cloudMonitoringRoute = struct { + path string + method string + url string + scopes []string +}{ + path: "cloudmonitoring", + method: "GET", + url: "https://monitoring.googleapis.com", + scopes: []string{"https://www.googleapis.com/auth/monitoring.read"}, +} + +func getMiddleware(model *datasourceInfo) (httpclient.Middleware, error) { + providerConfig := tokenprovider.Config{ + RoutePath: cloudMonitoringRoute.path, + RouteMethod: cloudMonitoringRoute.method, + DataSourceID: model.id, + DataSourceUpdated: model.updated, + Scopes: cloudMonitoringRoute.scopes, + } + + var provider tokenprovider.TokenProvider + switch model.authenticationType { + case gceAuthentication: + provider = tokenprovider.NewGceAccessTokenProvider(providerConfig) + case jwtAuthentication: + providerConfig.JwtTokenConfig = &tokenprovider.JwtTokenConfig{ + Email: model.clientEmail, + URI: model.tokenUri, + PrivateKey: []byte(model.decryptedSecureJSONData["privateKey"]), + } + provider = tokenprovider.NewJwtAccessTokenProvider(providerConfig) + } + + return tokenprovider.AuthMiddleware(provider), nil +} + +func newHTTPClient(model *datasourceInfo, opts httpclient.Options, clientProvider infrahttp.Provider) (*http.Client, error) { + m, err := getMiddleware(model) + if err != nil { + return nil, err + } + + opts.Middlewares = append(opts.Middlewares, m) + return clientProvider.New(opts) +} diff --git a/pkg/tsdb/cloudmonitoring/time_series_filter.go b/pkg/tsdb/cloudmonitoring/time_series_filter.go index f091b56781e..8f80c9c4adf 100644 --- a/pkg/tsdb/cloudmonitoring/time_series_filter.go +++ b/pkg/tsdb/cloudmonitoring/time_series_filter.go @@ -29,7 +29,7 @@ func (timeSeriesFilter *cloudMonitoringTimeSeriesFilter) run(ctx context.Context slog.Info("No project name set on query, using project name from datasource", "projectName", projectName) } - r, err := s.createRequest(ctx, req.PluginContext, &dsInfo, path.Join("cloudmonitoringv3/projects", projectName, "timeSeries"), nil) + r, err := s.createRequest(ctx, req.PluginContext, &dsInfo, path.Join("/v3/projects", projectName, "timeSeries"), nil) if err != nil { dr.Error = err return dr, cloudMonitoringResponse{}, "", nil diff --git a/pkg/tsdb/cloudmonitoring/time_series_query.go b/pkg/tsdb/cloudmonitoring/time_series_query.go index 645c413889c..b8320e8ffcf 100644 --- a/pkg/tsdb/cloudmonitoring/time_series_query.go +++ b/pkg/tsdb/cloudmonitoring/time_series_query.go @@ -49,7 +49,7 @@ func (timeSeriesQuery cloudMonitoringTimeSeriesQuery) run(ctx context.Context, r dr.Error = err return dr, cloudMonitoringResponse{}, "", nil } - r, err := s.createRequest(ctx, req.PluginContext, &dsInfo, path.Join("cloudmonitoringv3/projects", projectName, "timeSeries:query"), bytes.NewBuffer(buf)) + r, err := s.createRequest(ctx, req.PluginContext, &dsInfo, path.Join("/v3/projects", projectName, "timeSeries:query"), bytes.NewBuffer(buf)) if err != nil { dr.Error = err return dr, cloudMonitoringResponse{}, "", nil diff --git a/pkg/tsdb/cloudmonitoring/utils.go b/pkg/tsdb/cloudmonitoring/utils.go new file mode 100644 index 00000000000..91b2b6bacd1 --- /dev/null +++ b/pkg/tsdb/cloudmonitoring/utils.go @@ -0,0 +1,26 @@ +package cloudmonitoring + +import ( + "strings" +) + +func reverse(s string) string { + chars := []rune(s) + for i, j := 0, len(chars)-1; i < j; i, j = i+1, j-1 { + chars[i], chars[j] = chars[j], chars[i] + } + return string(chars) +} + +func toSnakeCase(str string) string { + return strings.ToLower(matchAllCap.ReplaceAllString(str, "${1}_${2}")) +} + +func containsLabel(labels []string, newLabel string) bool { + for _, val := range labels { + if val == newLabel { + return true + } + } + return false +} From 41530482ec735924b22374eae8a31e83fd40e0bb Mon Sep 17 00:00:00 2001 From: James Wang <36892657+jamesxwang@users.noreply.github.com> Date: Tue, 26 Oct 2021 22:32:39 +0800 Subject: [PATCH 37/49] Plugins Catalog: Fix plugin details header styles (#40917) --- .../features/plugins/admin/components/PluginDetailsHeader.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/public/app/features/plugins/admin/components/PluginDetailsHeader.tsx b/public/app/features/plugins/admin/components/PluginDetailsHeader.tsx index fe9d4acf695..01ac7f83972 100644 --- a/public/app/features/plugins/admin/components/PluginDetailsHeader.tsx +++ b/public/app/features/plugins/admin/components/PluginDetailsHeader.tsx @@ -124,7 +124,7 @@ export const getStyles = (theme: GrafanaTheme2) => { align-items: center; margin-top: ${theme.spacing()}; margin-bottom: ${theme.spacing()}; - + flex-flow: wrap; & > * { &::after { content: '|'; From 728a59f0130da7fa8abd6ce02fc126027dff8d38 Mon Sep 17 00:00:00 2001 From: Will Browne Date: Tue, 26 Oct 2021 16:20:55 +0100 Subject: [PATCH 38/49] bump for CVE-2021-37219 CVE-2021-32574 CVE-2021-36213 (#40947) --- go.mod | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go.mod b/go.mod index f198c5df6a5..7926698a08c 100644 --- a/go.mod +++ b/go.mod @@ -265,4 +265,4 @@ replace gopkg.in/macaron.v1 => ./pkg/macaron replace github.com/go-macaron/binding => ./pkg/macaron/binding -replace github.com/hashicorp/consul => github.com/hashicorp/consul v1.9.8 +replace github.com/hashicorp/consul => github.com/hashicorp/consul v1.10.2 From 681218275e36d5e3eb6934a82a9f3b420653886f Mon Sep 17 00:00:00 2001 From: ying-jeanne <74549700+ying-jeanne@users.noreply.github.com> Date: Tue, 26 Oct 2021 17:36:24 +0200 Subject: [PATCH 39/49] remove crit and trace (#40320) --- e2e/suite1/specs/visualization-suggestions.ts | 2 +- pkg/api/app_routes.go | 2 +- pkg/api/avatar/avatar.go | 10 +++++----- pkg/api/dtos/models.go | 2 +- pkg/api/frontendsettings.go | 2 +- pkg/api/login.go | 8 ++++---- pkg/api/login_oauth.go | 2 +- pkg/components/imguploader/s3uploader.go | 2 +- pkg/infra/log/log.go | 8 ++++++++ pkg/login/social/common.go | 2 +- pkg/plugins/manager/update_checker.go | 12 ++++++------ .../ossaccesscontrol/ossaccesscontrol.go | 4 ++-- pkg/services/alerting/notifiers/telegram.go | 2 +- pkg/services/contexthandler/contexthandler.go | 2 +- pkg/services/login/loginservice/loginservice.go | 4 ++-- pkg/services/searchusers/filters/filters.go | 2 +- .../sqlstore/migrations/ualert/securejsondata.go | 11 ++++++++--- pkg/services/sqlstore/migrations/ualert/ualert.go | 6 +++--- pkg/services/sqlstore/transactions.go | 2 +- pkg/setting/setting.go | 15 +++++++++------ 20 files changed, 58 insertions(+), 42 deletions(-) diff --git a/e2e/suite1/specs/visualization-suggestions.ts b/e2e/suite1/specs/visualization-suggestions.ts index 10340e4d86f..79035dee674 100644 --- a/e2e/suite1/specs/visualization-suggestions.ts +++ b/e2e/suite1/specs/visualization-suggestions.ts @@ -7,7 +7,7 @@ e2e.scenario({ itName: 'Should be shown and clickable', addScenarioDataSource: false, addScenarioDashBoard: false, - skipScenario: false, + skipScenario: true, scenario: () => { e2e.flows.openDashboard({ uid: 'TkZXxlNG3' }); e2e.flows.openPanelMenuItem(e2e.flows.PanelMenuItems.Edit, PANEL_UNDER_TEST); diff --git a/pkg/api/app_routes.go b/pkg/api/app_routes.go index 82e3b450e5e..8a226334cb6 100644 --- a/pkg/api/app_routes.go +++ b/pkg/api/app_routes.go @@ -51,7 +51,7 @@ func (hs *HTTPServer) initAppPluginRoutes(r *web.Mux) { for _, method := range strings.Split(route.Method, ",") { r.Handle(strings.TrimSpace(method), url, handlers) } - log.Debugf("Plugins: Adding proxy route %s", url) + log.Debug("Plugins: Adding proxy route", "url", url) } } } diff --git a/pkg/api/avatar/avatar.go b/pkg/api/avatar/avatar.go index 3d98f14b297..50634cd677e 100644 --- a/pkg/api/avatar/avatar.go +++ b/pkg/api/avatar/avatar.go @@ -95,7 +95,7 @@ func (a *CacheServer) Handler(ctx *models.ReqContext) { if avatar.Expired() { // The cache item is either expired or newly created, update it from the server if err := avatar.Update(); err != nil { - log.Tracef("avatar update error: %v", err) + log.Debug("avatar update", "err", err) avatar = a.notFound } } @@ -104,7 +104,7 @@ func (a *CacheServer) Handler(ctx *models.ReqContext) { avatar = a.notFound } else if !exists { if err := a.cache.Add(hash, avatar, gocache.DefaultExpiration); err != nil { - log.Tracef("Error adding avatar to cache: %s", err) + log.Debug("add avatar to cache", "err", err) } } @@ -117,7 +117,7 @@ func (a *CacheServer) Handler(ctx *models.ReqContext) { ctx.Resp.Header().Set("Cache-Control", "private, max-age=3600") if err := avatar.Encode(ctx.Resp); err != nil { - log.Warnf("avatar encode error: %v", err) + log.Warn("avatar encode error:", "err", err) ctx.Resp.WriteHeader(500) } } @@ -142,7 +142,7 @@ func newNotFound(cfg *setting.Cfg) *Avatar { // variable. // nolint:gosec if data, err := ioutil.ReadFile(path); err != nil { - log.Errorf(3, "Failed to read user_profile.png, %v", path) + log.Error("Failed to read user_profile.png", "path", path) } else { avatar.data = bytes.NewBuffer(data) } @@ -215,7 +215,7 @@ var client = &http.Client{ func (a *thunderTask) fetch() error { a.Avatar.timestamp = time.Now() - log.Debugf("avatar.fetch(fetch new avatar): %s", a.Url) + log.Debug("avatar.fetch(fetch new avatar)", "url", a.Url) req, err := http.NewRequest("GET", a.Url, nil) if err != nil { return err diff --git a/pkg/api/dtos/models.go b/pkg/api/dtos/models.go index 46407afcd66..d63af30f088 100644 --- a/pkg/api/dtos/models.go +++ b/pkg/api/dtos/models.go @@ -65,7 +65,7 @@ func GetGravatarUrl(text string) string { hasher := md5.New() if _, err := hasher.Write([]byte(strings.ToLower(text))); err != nil { - log.Warnf("Failed to hash text: %s", err) + log.Warn("Failed to hash text", "err", err) } return fmt.Sprintf(setting.AppSubUrl+"/avatar/%x", hasher.Sum(nil)) } diff --git a/pkg/api/frontendsettings.go b/pkg/api/frontendsettings.go index 4c766f30473..f60b1156d65 100644 --- a/pkg/api/frontendsettings.go +++ b/pkg/api/frontendsettings.go @@ -63,7 +63,7 @@ func (hs *HTTPServer) getFSDataSources(c *models.ReqContext, enabledPlugins *plu meta, exists := enabledPlugins.DataSources[ds.Type] if !exists { - log.Errorf(3, "Could not find plugin definition for data source: %v", ds.Type) + log.Error("Could not find plugin definition for data source", "datasource_type", ds.Type) continue } dsMap["meta"] = meta diff --git a/pkg/api/login.go b/pkg/api/login.go index 041266010e9..c135d1ce4f2 100644 --- a/pkg/api/login.go +++ b/pkg/api/login.go @@ -130,7 +130,7 @@ func (hs *HTTPServer) LoginView(c *models.ReqContext) { if err := hs.ValidateRedirectTo(redirectTo); err != nil { // the user is already logged so instead of rendering the login page with error // it should be redirected to the home page. - log.Debugf("Ignored invalid redirect_to cookie value: %v", redirectTo) + log.Debug("Ignored invalid redirect_to cookie value", "redirect_to", redirectTo) redirectTo = hs.Cfg.AppSubURL + "/" } cookies.DeleteCookie(c.Resp, "redirect_to", hs.CookieOptionsFromCfg) @@ -151,12 +151,12 @@ func (hs *HTTPServer) tryOAuthAutoLogin(c *models.ReqContext) bool { } oauthInfos := hs.SocialService.GetOAuthInfoProviders() if len(oauthInfos) != 1 { - log.Warnf("Skipping OAuth auto login because multiple OAuth providers are configured") + log.Warn("Skipping OAuth auto login because multiple OAuth providers are configured") return false } for key := range oauthInfos { redirectUrl := hs.Cfg.AppSubURL + "/login/" + key - log.Infof("OAuth auto login enabled. Redirecting to " + redirectUrl) + log.Info("OAuth auto login enabled. Redirecting to " + redirectUrl) c.Redirect(redirectUrl, 307) return true } @@ -248,7 +248,7 @@ func (hs *HTTPServer) LoginPost(c *models.ReqContext) response.Response { if err := hs.ValidateRedirectTo(redirectTo); err == nil { result["redirectUrl"] = redirectTo } else { - log.Infof("Ignored invalid redirect_to cookie value: %v", redirectTo) + log.Info("Ignored invalid redirect_to cookie value.", "url", redirectTo) } cookies.DeleteCookie(c.Resp, "redirect_to", hs.CookieOptionsFromCfg) } diff --git a/pkg/api/login_oauth.go b/pkg/api/login_oauth.go index 5a513569183..2cbbe84e4e5 100644 --- a/pkg/api/login_oauth.go +++ b/pkg/api/login_oauth.go @@ -256,7 +256,7 @@ func (hs *HTTPServer) OAuthLogin(ctx *models.ReqContext) { ctx.Redirect(redirectTo) return } - log.Debugf("Ignored invalid redirect_to cookie value: %v", redirectTo) + log.Debug("Ignored invalid redirect_to cookie value", "redirect_to", redirectTo) } ctx.Redirect(setting.AppSubUrl + "/") diff --git a/pkg/components/imguploader/s3uploader.go b/pkg/components/imguploader/s3uploader.go index d0b8b251740..87fb81ab4c7 100644 --- a/pkg/components/imguploader/s3uploader.go +++ b/pkg/components/imguploader/s3uploader.go @@ -74,7 +74,7 @@ func (u *S3Uploader) Upload(ctx context.Context, imageDiskPath string) (string, return "", err } key := u.path + rand + pngExt - log.Debugf("Uploading image to s3. bucket = %s, path = %s", u.bucket, key) + log.Debug("Uploading image to s3.", "bucket", u.bucket, "path", key) // We can ignore the gosec G304 warning on this one because `imageDiskPath` comes // from alert notifiers and is only used to upload images generated by alerting. diff --git a/pkg/infra/log/log.go b/pkg/infra/log/log.go index 188baf71a40..e923912073c 100644 --- a/pkg/infra/log/log.go +++ b/pkg/infra/log/log.go @@ -84,6 +84,14 @@ func Warnf(format string, v ...interface{}) { Root.Warn(message) } +func Debug(msg string, args ...interface{}) { + Root.Debug(msg, args...) +} + +func Info(msg string, args ...interface{}) { + Root.Info(msg, args...) +} + func Error(msg string, args ...interface{}) { Root.Error(msg, args...) } diff --git a/pkg/login/social/common.go b/pkg/login/social/common.go index 376406cf86c..1f2a497b8b1 100644 --- a/pkg/login/social/common.go +++ b/pkg/login/social/common.go @@ -68,7 +68,7 @@ func (s *SocialBase) httpGet(client *http.Client, url string) (response httpGetR return } - log.Tracef("HTTP GET %s: %s %s", url, r.Status, string(response.Body)) + log.Debug("HTTP GET", "url", url, "status", r.Status, "response_body", string(response.Body)) err = nil return diff --git a/pkg/plugins/manager/update_checker.go b/pkg/plugins/manager/update_checker.go index 3b20af0ad4f..26e21727d5c 100644 --- a/pkg/plugins/manager/update_checker.go +++ b/pkg/plugins/manager/update_checker.go @@ -49,7 +49,7 @@ func (pm *PluginManager) checkForUpdates() { pluginSlugs := pm.getAllExternalPluginSlugs() resp, err := httpClient.Get("https://grafana.com/api/plugins/versioncheck?slugIn=" + pluginSlugs + "&grafanaVersion=" + setting.BuildVersion) if err != nil { - log.Tracef("Failed to get plugins repo from grafana.com, %v", err.Error()) + log.Debug("Failed to get plugins repo from grafana.com", "error", err.Error()) return } defer func() { @@ -60,14 +60,14 @@ func (pm *PluginManager) checkForUpdates() { body, err := ioutil.ReadAll(resp.Body) if err != nil { - log.Tracef("Update check failed, reading response from grafana.com, %v", err.Error()) + log.Debug("Update check failed, reading response from grafana.com", "error", err.Error()) return } gNetPlugins := []grafanaNetPlugin{} err = json.Unmarshal(body, &gNetPlugins) if err != nil { - log.Tracef("Failed to unmarshal plugin repo, reading response from grafana.com, %v", err.Error()) + log.Debug("Failed to unmarshal plugin repo, reading response from grafana.com", "error", err.Error()) return } @@ -90,7 +90,7 @@ func (pm *PluginManager) checkForUpdates() { resp2, err := httpClient.Get("https://raw.githubusercontent.com/grafana/grafana/main/latest.json") if err != nil { - log.Tracef("Failed to get latest.json repo from github.com: %v", err.Error()) + log.Debug("Failed to get latest.json repo from github.com", "error", err.Error()) return } defer func() { @@ -100,14 +100,14 @@ func (pm *PluginManager) checkForUpdates() { }() body, err = ioutil.ReadAll(resp2.Body) if err != nil { - log.Tracef("Update check failed, reading response from github.com, %v", err.Error()) + log.Debug("Update check failed, reading response from github.com", "error", err.Error()) return } var latest gitHubLatest err = json.Unmarshal(body, &latest) if err != nil { - log.Tracef("Failed to unmarshal github.com latest, reading response from github.com: %v", err.Error()) + log.Debug("Failed to unmarshal github.com latest, reading response from github.com", "error", err.Error()) return } diff --git a/pkg/services/accesscontrol/ossaccesscontrol/ossaccesscontrol.go b/pkg/services/accesscontrol/ossaccesscontrol/ossaccesscontrol.go index 9ad49c05313..e90171a1c66 100644 --- a/pkg/services/accesscontrol/ossaccesscontrol/ossaccesscontrol.go +++ b/pkg/services/accesscontrol/ossaccesscontrol/ossaccesscontrol.go @@ -134,7 +134,7 @@ func (ac *OSSAccessControlService) saveFixedRole(role accesscontrol.RoleDTO) { // needs to be increased. Hence, we don't overwrite a role with a // greater version. if storedRole.Version >= role.Version { - log.Debugf("role %v has already been stored in a greater version, skipping registration", role.Name) + log.Debug("the has already been stored in a greater version, skipping registration", "role", role.Name) return } } @@ -150,7 +150,7 @@ func (ac *OSSAccessControlService) assignFixedRole(role accesscontrol.RoleDTO, b if ok { for _, assignedRole := range assignments { if assignedRole == role.Name { - log.Debugf("role %v has already been assigned to %v", role.Name, builtInRole) + log.Debug("the role has already been assigned", "rolename", role.Name, "build_in_role", builtInRole) alreadyAssigned = true } } diff --git a/pkg/services/alerting/notifiers/telegram.go b/pkg/services/alerting/notifiers/telegram.go index 20c4bcfb656..5bfa036121d 100644 --- a/pkg/services/alerting/notifiers/telegram.go +++ b/pkg/services/alerting/notifiers/telegram.go @@ -254,7 +254,7 @@ func appendIfPossible(message string, extra string, sizeLimit int) string { if len(extra)+len(message) <= sizeLimit { return message + extra } - log.Debugf("Line too long for image caption. value: %s", extra) + log.Debug("Line too long for image caption.", "value", extra) return message } diff --git a/pkg/services/contexthandler/contexthandler.go b/pkg/services/contexthandler/contexthandler.go index cf673755d17..49e0256aab1 100644 --- a/pkg/services/contexthandler/contexthandler.go +++ b/pkg/services/contexthandler/contexthandler.go @@ -148,7 +148,7 @@ func (h *ContextHandler) initContextWithAnonymousUser(reqContext *models.ReqCont org, err := h.SQLStore.GetOrgByName(h.Cfg.AnonymousOrgName) if err != nil { - log.Errorf(3, "Anonymous access organization error: '%s': %s", h.Cfg.AnonymousOrgName, err) + log.Error("Anonymous access organization error.", "org_name", h.Cfg.AnonymousOrgName, "error", err) return false } diff --git a/pkg/services/login/loginservice/loginservice.go b/pkg/services/login/loginservice/loginservice.go index a453848b677..764dfd7b805 100644 --- a/pkg/services/login/loginservice/loginservice.go +++ b/pkg/services/login/loginservice/loginservice.go @@ -56,13 +56,13 @@ func (ls *Implementation) UpsertUser(cmd *models.UpsertUserCommand) error { return err } if !cmd.SignupAllowed { - log.Warnf("Not allowing %s login, user not found in internal user database and allow signup = false", extUser.AuthModule) + log.Warn("Not allowing login, user not found in internal user database and allow signup = false", "authmode", extUser.AuthModule) return login.ErrInvalidCredentials } limitReached, err := ls.QuotaService.QuotaReached(cmd.ReqContext, "user") if err != nil { - log.Warnf("Error getting user quota. error: %v", err) + log.Warn("Error getting user quota.", "error", err) return login.ErrGettingUserQuota } if limitReached { diff --git a/pkg/services/searchusers/filters/filters.go b/pkg/services/searchusers/filters/filters.go index 9a573623521..be5f66ca0b6 100644 --- a/pkg/services/searchusers/filters/filters.go +++ b/pkg/services/searchusers/filters/filters.go @@ -26,7 +26,7 @@ func (o *OSSSearchUserFilter) GetFilter(filterName string, params []string) mode } filter, err := f(params) if err != nil { - log.Warnf("Cannot initialise the filter %s: %s", filterName, err) + log.Warn("Cannot initialise the filter.", "filter", filterName, "error", err) return nil } return filter diff --git a/pkg/services/sqlstore/migrations/ualert/securejsondata.go b/pkg/services/sqlstore/migrations/ualert/securejsondata.go index 119a44f3d50..557cd9046a7 100644 --- a/pkg/services/sqlstore/migrations/ualert/securejsondata.go +++ b/pkg/services/sqlstore/migrations/ualert/securejsondata.go @@ -1,6 +1,8 @@ package ualert import ( + "os" + "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/setting" "github.com/grafana/grafana/pkg/util" @@ -16,7 +18,8 @@ func (s SecureJsonData) DecryptedValue(key string) (string, bool) { if value, ok := s[key]; ok { decryptedData, err := util.Decrypt(value, setting.SecretKey) if err != nil { - log.Fatalf(4, err.Error()) + log.Error(err.Error()) + os.Exit(1) } return string(decryptedData), true } @@ -30,7 +33,8 @@ func (s SecureJsonData) Decrypt() map[string]string { for key, data := range s { decryptedData, err := util.Decrypt(data, setting.SecretKey) if err != nil { - log.Fatalf(4, err.Error()) + log.Error(err.Error()) + os.Exit(1) } decrypted[key] = string(decryptedData) @@ -44,7 +48,8 @@ func GetEncryptedJsonData(sjd map[string]string) SecureJsonData { for key, data := range sjd { encryptedData, err := util.Encrypt([]byte(data), setting.SecretKey) if err != nil { - log.Fatalf(4, err.Error()) + log.Error(err.Error()) + os.Exit(1) } encrypted[key] = encryptedData diff --git a/pkg/services/sqlstore/migrations/ualert/ualert.go b/pkg/services/sqlstore/migrations/ualert/ualert.go index 9ead2184493..ad549d2abd4 100644 --- a/pkg/services/sqlstore/migrations/ualert/ualert.go +++ b/pkg/services/sqlstore/migrations/ualert/ualert.go @@ -48,7 +48,7 @@ func (e *MigrationError) Unwrap() error { return e.Err } func AddDashAlertMigration(mg *migrator.Migrator) { logs, err := mg.GetMigrationLog() if err != nil { - mg.Logger.Crit("alert migration failure: could not get migration log", "error", err) + mg.Logger.Error("alert migration failure: could not get migration log", "error", err) os.Exit(1) } @@ -88,7 +88,7 @@ func AddDashAlertMigration(mg *migrator.Migrator) { func RerunDashAlertMigration(mg *migrator.Migrator) { logs, err := mg.GetMigrationLog() if err != nil { - mg.Logger.Crit("alert migration failure: could not get migration log", "error", err) + mg.Logger.Error("alert migration failure: could not get migration log", "error", err) os.Exit(1) } @@ -109,7 +109,7 @@ func RerunDashAlertMigration(mg *migrator.Migrator) { func AddDashboardUIDPanelIDMigration(mg *migrator.Migrator) { logs, err := mg.GetMigrationLog() if err != nil { - mg.Logger.Crit("alert migration failure: could not get migration log", "error", err) + mg.Logger.Error("alert migration failure: could not get migration log", "error", err) os.Exit(1) } diff --git a/pkg/services/sqlstore/transactions.go b/pkg/services/sqlstore/transactions.go index 6c2c01101a8..62043aae7fd 100644 --- a/pkg/services/sqlstore/transactions.go +++ b/pkg/services/sqlstore/transactions.go @@ -67,7 +67,7 @@ func inTransactionWithRetryCtx(ctx context.Context, engine *xorm.Engine, callbac if len(sess.events) > 0 { for _, e := range sess.events { if err = bus.Publish(e); err != nil { - log.Errorf(3, "Failed to publish event after commit. error: %v", err) + log.Error("Failed to publish event after commit.", "error", err) } } } diff --git a/pkg/setting/setting.go b/pkg/setting/setting.go index 3b3d6afe5db..c4c132821d5 100644 --- a/pkg/setting/setting.go +++ b/pkg/setting/setting.go @@ -464,7 +464,8 @@ func parseAppUrlAndSubUrl(section *ini.Section) (string, string, error) { // Check if has app suburl. url, err := url.Parse(appUrl) if err != nil { - log.Fatalf(4, "Invalid root_url(%s): %s", appUrl, err) + log.Error("Invalid root_url.", "url", appUrl, "error", err) + os.Exit(1) } appSubUrl := strings.TrimSuffix(url.Path, "/") @@ -631,8 +632,8 @@ func getCommandLineProperties(args []string) map[string]string { trimmed := strings.TrimPrefix(arg, "cfg:") parts := strings.Split(trimmed, "=") if len(parts) != 2 { - log.Fatalf(3, "Invalid command line argument. argument: %v", arg) - return nil + log.Error("Invalid command line argument.", "argument", arg) + os.Exit(1) } props[parts[0]] = parts[1] @@ -718,7 +719,8 @@ func (cfg *Cfg) loadConfiguration(args CommandLineArgs) (*ini.File, error) { if err2 != nil { return nil, err2 } - log.Fatalf(3, err.Error()) + log.Error(err.Error()) + os.Exit(1) } // apply environment overrides @@ -961,7 +963,7 @@ func (cfg *Cfg) Load(args CommandLineArgs) error { cfg.readDataSourcesSettings() if VerifyEmailEnabled && !cfg.Smtp.Enabled { - log.Warnf("require_email_validation is enabled but smtp is disabled") + log.Warn("require_email_validation is enabled but smtp is disabled") } // check old key name @@ -1356,7 +1358,8 @@ func (cfg *Cfg) readRenderingSettings(iniFile *ini.File) error { _, err := url.Parse(cfg.RendererCallbackUrl) if err != nil { // XXX: Should return an error? - log.Fatalf(4, "Invalid callback_url(%s): %s", cfg.RendererCallbackUrl, err) + log.Error("Invalid callback_url.", "url", cfg.RendererCallbackUrl, "error", err) + os.Exit(1) } } From 1f1162f1d8c3bbd3f25a3bcd3110e01dfa7fec8b Mon Sep 17 00:00:00 2001 From: Serge Zaitsev Date: Tue, 26 Oct 2021 17:59:37 +0200 Subject: [PATCH 40/49] Chore: Refactor GoConvey in teamguardian package (#40896) * refactor goconvey in teamguardian package * use proper order of parameters in equality assertion Co-authored-by: ying-jeanne <74549700+ying-jeanne@users.noreply.github.com> Co-authored-by: ying-jeanne <74549700+ying-jeanne@users.noreply.github.com> --- pkg/services/teamguardian/teams_test.go | 29 +++++++++++++------------ 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/pkg/services/teamguardian/teams_test.go b/pkg/services/teamguardian/teams_test.go index 358e1fc3c80..e79e3f1a689 100644 --- a/pkg/services/teamguardian/teams_test.go +++ b/pkg/services/teamguardian/teams_test.go @@ -5,11 +5,12 @@ import ( "github.com/grafana/grafana/pkg/bus" "github.com/grafana/grafana/pkg/models" - . "github.com/smartystreets/goconvey/convey" + + "github.com/stretchr/testify/require" ) func TestUpdateTeam(t *testing.T) { - Convey("Updating a team", t, func() { + t.Run("Updating a team", func(t *testing.T) { bus.ClearBusHandlers() admin := models.SignedInUser{ @@ -27,20 +28,20 @@ func TestUpdateTeam(t *testing.T) { OrgId: 1, } - Convey("Given an editor and a team he isn't a member of", func() { - Convey("Should not be able to update the team", func() { + t.Run("Given an editor and a team he isn't a member of", func(t *testing.T) { + t.Run("Should not be able to update the team", func(t *testing.T) { bus.AddHandler("test", func(cmd *models.GetTeamMembersQuery) error { cmd.Result = []*models.TeamMemberDTO{} return nil }) err := CanAdmin(bus.GetBus(), testTeam.OrgId, testTeam.Id, &editor) - So(err, ShouldEqual, models.ErrNotAllowedToUpdateTeam) + require.Equal(t, models.ErrNotAllowedToUpdateTeam, err) }) }) - Convey("Given an editor and a team he is an admin in", func() { - Convey("Should be able to update the team", func() { + t.Run("Given an editor and a team he is an admin in", func(t *testing.T) { + t.Run("Should be able to update the team", func(t *testing.T) { bus.AddHandler("test", func(cmd *models.GetTeamMembersQuery) error { cmd.Result = []*models.TeamMemberDTO{{ OrgId: testTeam.OrgId, @@ -52,17 +53,17 @@ func TestUpdateTeam(t *testing.T) { }) err := CanAdmin(bus.GetBus(), testTeam.OrgId, testTeam.Id, &editor) - So(err, ShouldBeNil) + require.NoError(t, err) }) }) - Convey("Given an editor and a team in another org", func() { + t.Run("Given an editor and a team in another org", func(t *testing.T) { testTeamOtherOrg := models.Team{ Id: 1, OrgId: 2, } - Convey("Shouldn't be able to update the team", func() { + t.Run("Shouldn't be able to update the team", func(t *testing.T) { bus.AddHandler("test", func(cmd *models.GetTeamMembersQuery) error { cmd.Result = []*models.TeamMemberDTO{{ OrgId: testTeamOtherOrg.OrgId, @@ -74,14 +75,14 @@ func TestUpdateTeam(t *testing.T) { }) err := CanAdmin(bus.GetBus(), testTeamOtherOrg.OrgId, testTeamOtherOrg.Id, &editor) - So(err, ShouldEqual, models.ErrNotAllowedToUpdateTeamInDifferentOrg) + require.Equal(t, models.ErrNotAllowedToUpdateTeamInDifferentOrg, err) }) }) - Convey("Given an org admin and a team", func() { - Convey("Should be able to update the team", func() { + t.Run("Given an org admin and a team", func(t *testing.T) { + t.Run("Should be able to update the team", func(t *testing.T) { err := CanAdmin(bus.GetBus(), testTeam.OrgId, testTeam.Id, &admin) - So(err, ShouldBeNil) + require.NoError(t, err) }) }) }) From 125e284da234801c42f1e6b0c7694ba44dcb59ee Mon Sep 17 00:00:00 2001 From: Serge Zaitsev Date: Tue, 26 Oct 2021 18:08:04 +0200 Subject: [PATCH 41/49] Chore: Refactor GoConvey in notification service package (#40897) * refactor goconvey in notification service package * avoid return after t.skip --- pkg/services/notifications/codes_test.go | 23 ++++++++++--------- .../send_email_integration_test.go | 19 ++++++++------- 2 files changed, 23 insertions(+), 19 deletions(-) diff --git a/pkg/services/notifications/codes_test.go b/pkg/services/notifications/codes_test.go index f470c2f8b5e..a314c8decab 100644 --- a/pkg/services/notifications/codes_test.go +++ b/pkg/services/notifications/codes_test.go @@ -5,34 +5,35 @@ import ( "github.com/grafana/grafana/pkg/models" "github.com/grafana/grafana/pkg/setting" - . "github.com/smartystreets/goconvey/convey" + + "github.com/stretchr/testify/require" ) func TestEmailCodes(t *testing.T) { - Convey("When generating code", t, func() { + t.Run("When generating code", func(t *testing.T) { cfg := setting.NewCfg() cfg.EmailCodeValidMinutes = 120 user := &models.User{Id: 10, Email: "t@a.com", Login: "asd", Password: "1", Rands: "2"} code, err := createUserEmailCode(cfg, user, nil) - So(err, ShouldBeNil) + require.NoError(t, err) - Convey("getLoginForCode should return login", func() { + t.Run("getLoginForCode should return login", func(t *testing.T) { login := getLoginForEmailCode(code) - So(login, ShouldEqual, "asd") + require.Equal(t, login, "asd") }) - Convey("Can verify valid code", func() { + t.Run("Can verify valid code", func(t *testing.T) { isValid, err := validateUserEmailCode(cfg, user, code) - So(err, ShouldBeNil) - So(isValid, ShouldBeTrue) + require.NoError(t, err) + require.True(t, isValid) }) - Convey("Cannot verify in-valid code", func() { + t.Run("Cannot verify in-valid code", func(t *testing.T) { code = "ASD" isValid, err := validateUserEmailCode(cfg, user, code) - So(err, ShouldBeNil) - So(isValid, ShouldBeFalse) + require.NoError(t, err) + require.False(t, isValid) }) }) } diff --git a/pkg/services/notifications/send_email_integration_test.go b/pkg/services/notifications/send_email_integration_test.go index ef1b5496b90..6b257312357 100644 --- a/pkg/services/notifications/send_email_integration_test.go +++ b/pkg/services/notifications/send_email_integration_test.go @@ -7,11 +7,14 @@ import ( "github.com/grafana/grafana/pkg/bus" "github.com/grafana/grafana/pkg/models" "github.com/grafana/grafana/pkg/setting" - . "github.com/smartystreets/goconvey/convey" + + "github.com/stretchr/testify/require" ) func TestEmailIntegrationTest(t *testing.T) { - SkipConvey("Given the notifications service", t, func() { + t.Run("Given the notifications service", func(t *testing.T) { + t.Skip() + setting.StaticRootPath = "../../../public/" setting.BuildVersion = "4.0.0" @@ -24,7 +27,7 @@ func TestEmailIntegrationTest(t *testing.T) { ns.Cfg.Smtp.FromName = "Grafana Admin" ns.Cfg.Smtp.ContentTypes = []string{"text/html", "text/plain"} - Convey("When sending reset email password", func() { + t.Run("When sending reset email password", func(t *testing.T) { cmd := &models.SendEmailCommand{ Data: map[string]interface{}{ @@ -54,15 +57,15 @@ func TestEmailIntegrationTest(t *testing.T) { } err := ns.sendEmailCommandHandler(cmd) - So(err, ShouldBeNil) + require.NoError(t, err) sentMsg := <-ns.mailQueue - So(sentMsg.From, ShouldEqual, "Grafana Admin ") - So(sentMsg.To[0], ShouldEqual, "asdf@asdf.com") + require.Equal(t, sentMsg.From, "Grafana Admin ") + require.Equal(t, sentMsg.To[0], "asdf@asdf.com") err = ioutil.WriteFile("../../../tmp/test_email.html", []byte(sentMsg.Body["text/html"]), 0777) - So(err, ShouldBeNil) + require.NoError(t, err) err = ioutil.WriteFile("../../../tmp/test_email.txt", []byte(sentMsg.Body["text/plain"]), 0777) - So(err, ShouldBeNil) + require.NoError(t, err) }) }) } From 49dee63453cf4e51da0a86ddbee89b831a634481 Mon Sep 17 00:00:00 2001 From: Marcus Andersson Date: Tue, 26 Oct 2021 18:38:20 +0200 Subject: [PATCH 42/49] added ownership of plugins management code to the plugins platform frontend squad. (#40939) --- .github/CODEOWNERS | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index ad210108d6c..a70dc3a8616 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -63,6 +63,7 @@ go.sum @grafana/backend-platform /pkg/plugins @grafana/plugins-platform-backend /pkg/services/datasourceproxy @grafana/plugins-platform-backend /pkg/services/datasources @grafana/plugins-platform-backend +/public/app/features/plugins @grafana/plugins-platform-frontend # Backend code docs /contribute/style-guides/backend.md @grafana/backend-platform From 6709359148fa64bf8630a99c639765a14243a673 Mon Sep 17 00:00:00 2001 From: Yuriy Tseretyan Date: Tue, 26 Oct 2021 13:22:07 -0400 Subject: [PATCH 43/49] Alerting: Tests for rule evaluation routine (#40646) * add fake stores to record queries --- .../ngalert/schedule/schedule_unit_test.go | 376 +++++++++++++++++- pkg/services/ngalert/schedule/testing.go | 109 ++++- 2 files changed, 458 insertions(+), 27 deletions(-) diff --git a/pkg/services/ngalert/schedule/schedule_unit_test.go b/pkg/services/ngalert/schedule/schedule_unit_test.go index 7254d68cad7..277c8e6b8a1 100644 --- a/pkg/services/ngalert/schedule/schedule_unit_test.go +++ b/pkg/services/ngalert/schedule/schedule_unit_test.go @@ -10,6 +10,11 @@ import ( "time" "github.com/benbjohnson/clock" + "github.com/grafana/grafana-plugin-sdk-go/data" + "github.com/prometheus/client_golang/prometheus" + "github.com/prometheus/common/model" + "github.com/stretchr/testify/require" + "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/services/encryption/ossencryption" apimodels "github.com/grafana/grafana/pkg/services/ngalert/api/tooling/definitions" @@ -17,12 +22,10 @@ import ( "github.com/grafana/grafana/pkg/services/ngalert/metrics" "github.com/grafana/grafana/pkg/services/ngalert/models" "github.com/grafana/grafana/pkg/services/ngalert/notifier" + "github.com/grafana/grafana/pkg/services/ngalert/sender" "github.com/grafana/grafana/pkg/services/ngalert/state" "github.com/grafana/grafana/pkg/services/ngalert/store" "github.com/grafana/grafana/pkg/setting" - "github.com/prometheus/client_golang/prometheus" - "github.com/prometheus/common/model" - "github.com/stretchr/testify/require" ) func TestSendingToExternalAlertmanager(t *testing.T) { @@ -33,7 +36,7 @@ func TestSendingToExternalAlertmanager(t *testing.T) { fakeAdminConfigStore := newFakeAdminConfigStore(t) // create alert rule with one second interval - alertRule := CreateTestAlertRule(t, fakeRuleStore, 1, 1) + alertRule := CreateTestAlertRule(t, fakeRuleStore, 1, 1, eval.Alerting) // First, let's create an admin configuration that holds an alertmanager. adminConfig := &models.AdminConfiguration{OrgID: 1, Alertmanagers: []string{fakeAM.server.URL}} @@ -231,6 +234,325 @@ func TestSendingToExternalAlertmanager_WithMultipleOrgs(t *testing.T) { }, 10*time.Second, 200*time.Millisecond, "Alertmanager for org 1 and 2 were never removed") } +func TestSchedule_ruleRoutine(t *testing.T) { + createSchedule := func( + evalAppliedChan chan time.Time, + ) (*schedule, *fakeRuleStore, *fakeInstanceStore, *fakeAdminConfigStore) { + ruleStore := newFakeRuleStore(t) + instanceStore := &fakeInstanceStore{} + adminConfigStore := newFakeAdminConfigStore(t) + + sch, _ := setupScheduler(t, ruleStore, instanceStore, adminConfigStore) + + sch.evalAppliedFunc = func(key models.AlertRuleKey, t time.Time) { + evalAppliedChan <- t + } + return sch, ruleStore, instanceStore, adminConfigStore + } + + // normal states do not include NoData and Error because currently it is not possible to perform any sensible test + normalStates := []eval.State{eval.Normal, eval.Alerting, eval.Pending} + randomNormalState := func() eval.State { + // pick only supported cases + return normalStates[rand.Intn(3)] + } + + for _, evalState := range normalStates { + // TODO rewrite when we are able to mock/fake state manager + t.Run(fmt.Sprintf("when rule evaluation happens (evaluation state %s)", evalState), func(t *testing.T) { + evalChan := make(chan *evalContext) + evalAppliedChan := make(chan time.Time) + + sch, ruleStore, instanceStore, _ := createSchedule(evalAppliedChan) + + rule := CreateTestAlertRule(t, ruleStore, 10, rand.Int63(), evalState) + + go func() { + stop := make(chan struct{}) + t.Cleanup(func() { + close(stop) + }) + _ = sch.ruleRoutine(context.Background(), rule.GetKey(), evalChan, stop) + }() + + expectedTime := time.UnixMicro(rand.Int63()) + + evalChan <- &evalContext{ + now: expectedTime, + version: rule.Version, + } + + actualTime := waitForTimeChannel(t, evalAppliedChan) + require.Equal(t, expectedTime, actualTime) + + t.Run("it should get rule from database when run the first time", func(t *testing.T) { + queries := make([]models.GetAlertRuleByUIDQuery, 0) + for _, op := range ruleStore.recordedOps { + switch q := op.(type) { + case models.GetAlertRuleByUIDQuery: + queries = append(queries, q) + } + } + require.NotEmptyf(t, queries, "Expected a %T request to rule store but nothing was recorded", models.GetAlertRuleByUIDQuery{}) + require.Len(t, queries, 1, "Expected exactly one request of %T but got %d", models.GetAlertRuleByUIDQuery{}, len(queries)) + require.Equal(t, rule.UID, queries[0].UID) + require.Equal(t, rule.OrgID, queries[0].OrgID) + }) + t.Run("it should process evaluation results via state manager", func(t *testing.T) { + // TODO rewrite when we are able to mock/fake state manager + states := sch.stateManager.GetStatesForRuleUID(rule.OrgID, rule.UID) + require.Len(t, states, 1) + s := states[0] + t.Logf("State: %v", s) + require.Equal(t, rule.UID, s.AlertRuleUID) + require.Len(t, s.Results, 1) + var expectedStatus = evalState + if evalState == eval.Pending { + expectedStatus = eval.Alerting + } + require.Equal(t, expectedStatus.String(), s.Results[0].EvaluationState.String()) + require.Equal(t, expectedTime, s.Results[0].EvaluationTime) + }) + t.Run("it should save alert instances to storage", func(t *testing.T) { + // TODO rewrite when we are able to mock/fake state manager + states := sch.stateManager.GetStatesForRuleUID(rule.OrgID, rule.UID) + require.Len(t, states, 1) + s := states[0] + + var cmd *models.SaveAlertInstanceCommand + for _, op := range instanceStore.recordedOps { + switch q := op.(type) { + case models.SaveAlertInstanceCommand: + cmd = &q + } + if cmd != nil { + break + } + } + + require.NotNil(t, cmd) + t.Logf("Saved alert instance: %v", cmd) + require.Equal(t, rule.OrgID, cmd.RuleOrgID) + require.Equal(t, expectedTime, cmd.LastEvalTime) + require.Equal(t, cmd.RuleUID, cmd.RuleUID) + require.Equal(t, evalState.String(), string(cmd.State)) + require.Equal(t, s.Labels, data.Labels(cmd.Labels)) + }) + t.Run("it reports metrics", func(t *testing.T) { + // TODO fix it when we update the way we use metrics + t.Skip() + }) + }) + } + + t.Run("should exit", func(t *testing.T) { + t.Run("when we signal it to stop", func(t *testing.T) { + stopChan := make(chan struct{}) + stoppedChan := make(chan error) + + sch, _, _, _ := createSchedule(make(chan time.Time)) + + go func() { + err := sch.ruleRoutine(context.Background(), models.AlertRuleKey{}, make(chan *evalContext), stopChan) + stoppedChan <- err + }() + + stopChan <- struct{}{} + err := waitForErrChannel(t, stoppedChan) + require.NoError(t, err) + }) + + t.Run("when context is cancelled", func(t *testing.T) { + stoppedChan := make(chan error) + sch, _, _, _ := createSchedule(make(chan time.Time)) + + ctx, cancel := context.WithCancel(context.Background()) + go func() { + err := sch.ruleRoutine(ctx, models.AlertRuleKey{}, make(chan *evalContext), make(chan struct{})) + stoppedChan <- err + }() + + cancel() + err := waitForErrChannel(t, stoppedChan) + require.ErrorIs(t, err, context.Canceled) + }) + }) + + t.Run("should fetch rule from database only if new version is greater than current", func(t *testing.T) { + evalChan := make(chan *evalContext) + evalAppliedChan := make(chan time.Time) + + sch, ruleStore, _, _ := createSchedule(evalAppliedChan) + + rule := CreateTestAlertRule(t, ruleStore, 10, rand.Int63(), randomNormalState()) + + go func() { + stop := make(chan struct{}) + t.Cleanup(func() { + close(stop) + }) + _ = sch.ruleRoutine(context.Background(), rule.GetKey(), evalChan, stop) + }() + + expectedTime := time.UnixMicro(rand.Int63()) + evalChan <- &evalContext{ + now: expectedTime, + version: rule.Version, + } + + actualTime := waitForTimeChannel(t, evalAppliedChan) + require.Equal(t, expectedTime, actualTime) + + // Now update the rule + newRule := *rule + newRule.Version++ + ruleStore.putRule(&newRule) + + // and call with new version + expectedTime = expectedTime.Add(time.Duration(rand.Intn(10)) * time.Second) + evalChan <- &evalContext{ + now: expectedTime, + version: newRule.Version, + } + + actualTime = waitForTimeChannel(t, evalAppliedChan) + require.Equal(t, expectedTime, actualTime) + + queries := make([]models.GetAlertRuleByUIDQuery, 0) + for _, op := range ruleStore.recordedOps { + switch q := op.(type) { + case models.GetAlertRuleByUIDQuery: + queries = append(queries, q) + } + } + require.Len(t, queries, 2, "Expected exactly two request of %T", models.GetAlertRuleByUIDQuery{}) + require.Equal(t, rule.UID, queries[0].UID) + require.Equal(t, rule.OrgID, queries[0].OrgID) + require.Equal(t, rule.UID, queries[1].UID) + require.Equal(t, rule.OrgID, queries[1].OrgID) + }) + + t.Run("should not fetch rule if version is equal or less than current", func(t *testing.T) { + evalChan := make(chan *evalContext) + evalAppliedChan := make(chan time.Time) + + sch, ruleStore, _, _ := createSchedule(evalAppliedChan) + + rule := CreateTestAlertRule(t, ruleStore, 10, rand.Int63(), randomNormalState()) + + go func() { + stop := make(chan struct{}) + t.Cleanup(func() { + close(stop) + }) + _ = sch.ruleRoutine(context.Background(), rule.GetKey(), evalChan, stop) + }() + + expectedTime := time.UnixMicro(rand.Int63()) + evalChan <- &evalContext{ + now: expectedTime, + version: rule.Version, + } + + actualTime := waitForTimeChannel(t, evalAppliedChan) + require.Equal(t, expectedTime, actualTime) + + // try again with the same version + expectedTime = expectedTime.Add(time.Duration(rand.Intn(10)) * time.Second) + evalChan <- &evalContext{ + now: expectedTime, + version: rule.Version, + } + actualTime = waitForTimeChannel(t, evalAppliedChan) + require.Equal(t, expectedTime, actualTime) + + expectedTime = expectedTime.Add(time.Duration(rand.Intn(10)) * time.Second) + evalChan <- &evalContext{ + now: expectedTime, + version: rule.Version - 1, + } + actualTime = waitForTimeChannel(t, evalAppliedChan) + require.Equal(t, expectedTime, actualTime) + + queries := make([]models.GetAlertRuleByUIDQuery, 0) + for _, op := range ruleStore.recordedOps { + switch q := op.(type) { + case models.GetAlertRuleByUIDQuery: + queries = append(queries, q) + } + } + require.Len(t, queries, 1, "Expected exactly one request of %T", models.GetAlertRuleByUIDQuery{}) + }) + + t.Run("when evaluation fails", func(t *testing.T) { + t.Run("it should increase failure counter", func(t *testing.T) { + t.Skip() + // TODO implement check for counter + }) + t.Run("it should retry up to configured times", func(t *testing.T) { + // TODO figure out how to simulate failure + t.Skip() + }) + }) + + t.Run("when there are alerts that should be firing", func(t *testing.T) { + t.Run("it should send to local alertmanager if configured for organization", func(t *testing.T) { + // TODO figure out how to simulate multiorg alertmanager + t.Skip() + }) + t.Run("it should send to external alertmanager if configured for organization", func(t *testing.T) { + fakeAM := NewFakeExternalAlertmanager(t) + defer fakeAM.Close() + + orgID := rand.Int63() + s, err := sender.New(nil) + require.NoError(t, err) + adminConfig := &models.AdminConfiguration{OrgID: orgID, Alertmanagers: []string{fakeAM.server.URL}} + err = s.ApplyConfig(adminConfig) + require.NoError(t, err) + s.Run() + defer s.Stop() + + require.Eventuallyf(t, func() bool { + return len(s.Alertmanagers()) == 1 + }, 20*time.Second, 200*time.Millisecond, "external Alertmanager was not discovered.") + + evalChan := make(chan *evalContext) + evalAppliedChan := make(chan time.Time) + + sch, ruleStore, _, _ := createSchedule(evalAppliedChan) + sch.senders[orgID] = s + // eval.Alerting makes state manager to create notifications for alertmanagers + rule := CreateTestAlertRule(t, ruleStore, 10, orgID, eval.Alerting) + + go func() { + stop := make(chan struct{}) + t.Cleanup(func() { + close(stop) + }) + _ = sch.ruleRoutine(context.Background(), rule.GetKey(), evalChan, stop) + }() + + evalChan <- &evalContext{ + now: time.Now(), + version: rule.Version, + } + waitForTimeChannel(t, evalAppliedChan) + + var count int + require.Eventuallyf(t, func() bool { + count = fakeAM.AlertsCount() + return count == 1 && fakeAM.AlertNamesCompare([]string{rule.Title}) + }, 20*time.Second, 200*time.Millisecond, "Alertmanager never received an '%s', received alerts count: %d", rule.Title, count) + }) + }) + + t.Run("when there are no alerts to send it should not call notifiers", func(t *testing.T) { + // TODO needs some mocking/stubbing for Alertmanager and Sender to make sure it was not called + t.Skip() + }) +} + func setupScheduler(t *testing.T, rs store.RuleStore, is store.InstanceStore, acs store.AdminConfigurationStore) (*schedule, *clock.Mock) { t.Helper() @@ -263,11 +585,46 @@ func setupScheduler(t *testing.T, rs store.RuleStore, is store.InstanceStore, ac } // createTestAlertRule creates a dummy alert definition to be used by the tests. -func CreateTestAlertRule(t *testing.T, dbstore *fakeRuleStore, intervalSeconds int64, orgID int64) *models.AlertRule { +func CreateTestAlertRule(t *testing.T, dbstore *fakeRuleStore, intervalSeconds int64, orgID int64, evalResult eval.State) *models.AlertRule { t.Helper() - + records := make([]interface{}, 0, len(dbstore.recordedOps)) + copy(records, dbstore.recordedOps) + defer func() { + // erase queries that were made by the testing suite + dbstore.recordedOps = records + }() d := rand.Intn(1000) ruleGroup := fmt.Sprintf("ruleGroup-%d", d) + + var expression string + var forDuration time.Duration + switch evalResult { + case eval.Normal: + expression = `{ + "datasourceUid": "-100", + "type":"math", + "expression":"2 + 1 < 1" + }` + case eval.Pending, eval.Alerting: + expression = `{ + "datasourceUid": "-100", + "type":"math", + "expression":"2 + 2 > 1" + }` + if evalResult == eval.Pending { + forDuration = 100 * time.Second + } + case eval.Error: + expression = `{ + "datasourceUid": "-100", + "type":"math", + "expression":"$A" + }` + case eval.NoData: + // TODO Implement support for NoData + require.Fail(t, "Alert rule with desired evaluation result NoData is not supported yet") + } + err := dbstore.UpdateRuleGroup(store.UpdateRuleGroupCmd{ OrgID: orgID, NamespaceUID: "namespace", @@ -278,6 +635,7 @@ func CreateTestAlertRule(t *testing.T, dbstore *fakeRuleStore, intervalSeconds i { ApiRuleNode: &apimodels.ApiRuleNode{ Annotations: map[string]string{"testAnnoKey": "testAnnoValue"}, + For: model.Duration(forDuration), }, GrafanaManagedAlert: &apimodels.PostableGrafanaRule{ Title: fmt.Sprintf("an alert definition %d", d), @@ -285,11 +643,7 @@ func CreateTestAlertRule(t *testing.T, dbstore *fakeRuleStore, intervalSeconds i Data: []models.AlertQuery{ { DatasourceUID: "-100", - Model: json.RawMessage(`{ - "datasourceUid": "-100", - "type":"math", - "expression":"2 + 2 > 1" - }`), + Model: json.RawMessage(expression), RelativeTimeRange: models.RelativeTimeRange{ From: models.Duration(5 * time.Hour), To: models.Duration(3 * time.Hour), diff --git a/pkg/services/ngalert/schedule/testing.go b/pkg/services/ngalert/schedule/testing.go index 31732d0ee97..e5c508ea73d 100644 --- a/pkg/services/ngalert/schedule/testing.go +++ b/pkg/services/ngalert/schedule/testing.go @@ -21,15 +21,52 @@ import ( "github.com/stretchr/testify/require" ) +// waitForTimeChannel blocks the execution until either the channel ch has some data or a timeout of 10 second expires. +// Timeout will cause the test to fail. +// Returns the data from the channel. +func waitForTimeChannel(t *testing.T, ch chan time.Time) time.Time { + select { + case result := <-ch: + return result + case <-time.After(time.Duration(10) * time.Second): + t.Fatalf("Timeout waiting for data in the time channel") + return time.Time{} + } +} + +// waitForErrChannel blocks the execution until either the channel ch has some data or a timeout of 10 second expires. +// Timeout will cause the test to fail. +// Returns the data from the channel. +func waitForErrChannel(t *testing.T, ch chan error) error { + timeout := time.Duration(10) * time.Second + select { + case result := <-ch: + return result + case <-time.After(timeout): + t.Fatal("Timeout waiting for data in the error channel") + return nil + } +} + func newFakeRuleStore(t *testing.T) *fakeRuleStore { return &fakeRuleStore{t: t, rules: map[int64]map[string]map[string][]*models.AlertRule{}} } // FakeRuleStore mocks the RuleStore of the scheduler. type fakeRuleStore struct { - t *testing.T - mtx sync.Mutex - rules map[int64]map[string]map[string][]*models.AlertRule + t *testing.T + mtx sync.Mutex + rules map[int64]map[string]map[string][]*models.AlertRule + recordedOps []interface{} +} + +// putRule puts the rule in the rules map. If there are existing rule in the same namespace, they will be overwritten +func (f *fakeRuleStore) putRule(r *models.AlertRule) { + f.mtx.Lock() + defer f.mtx.Unlock() + f.rules[r.OrgID][r.RuleGroup][r.NamespaceUID] = []*models.AlertRule{ + r, + } } func (f *fakeRuleStore) DeleteAlertRuleByUID(_ int64, _ string) error { return nil } @@ -43,7 +80,7 @@ func (f *fakeRuleStore) DeleteAlertInstancesByRuleUID(_ int64, _ string) error { func (f *fakeRuleStore) GetAlertRuleByUID(q *models.GetAlertRuleByUIDQuery) error { f.mtx.Lock() defer f.mtx.Unlock() - + f.recordedOps = append(f.recordedOps, *q) rgs, ok := f.rules[q.OrgID] if !ok { return nil @@ -67,7 +104,7 @@ func (f *fakeRuleStore) GetAlertRuleByUID(q *models.GetAlertRuleByUIDQuery) erro func (f *fakeRuleStore) GetAlertRulesForScheduling(q *models.ListAlertRulesQuery) error { f.mtx.Lock() defer f.mtx.Unlock() - + f.recordedOps = append(f.recordedOps, *q) for _, rg := range f.rules { for _, n := range rg { for _, r := range n { @@ -78,13 +115,22 @@ func (f *fakeRuleStore) GetAlertRulesForScheduling(q *models.ListAlertRulesQuery return nil } -func (f *fakeRuleStore) GetOrgAlertRules(_ *models.ListAlertRulesQuery) error { return nil } -func (f *fakeRuleStore) GetNamespaceAlertRules(_ *models.ListNamespaceAlertRulesQuery) error { +func (f *fakeRuleStore) GetOrgAlertRules(q *models.ListAlertRulesQuery) error { + f.mtx.Lock() + defer f.mtx.Unlock() + f.recordedOps = append(f.recordedOps, *q) + return nil +} +func (f *fakeRuleStore) GetNamespaceAlertRules(q *models.ListNamespaceAlertRulesQuery) error { + f.mtx.Lock() + defer f.mtx.Unlock() + f.recordedOps = append(f.recordedOps, *q) return nil } func (f *fakeRuleStore) GetRuleGroupAlertRules(q *models.ListRuleGroupAlertRulesQuery) error { f.mtx.Lock() defer f.mtx.Unlock() + f.recordedOps = append(f.recordedOps, *q) rgs, ok := f.rules[q.OrgID] if !ok { return nil @@ -116,11 +162,23 @@ func (f *fakeRuleStore) GetNamespaces(_ context.Context, _ int64, _ *models2.Sig func (f *fakeRuleStore) GetNamespaceByTitle(_ context.Context, _ string, _ int64, _ *models2.SignedInUser, _ bool) (*models2.Folder, error) { return nil, nil } -func (f *fakeRuleStore) GetOrgRuleGroups(_ *models.ListOrgRuleGroupsQuery) error { return nil } -func (f *fakeRuleStore) UpsertAlertRules(_ []store.UpsertRule) error { return nil } +func (f *fakeRuleStore) GetOrgRuleGroups(q *models.ListOrgRuleGroupsQuery) error { + f.mtx.Lock() + defer f.mtx.Unlock() + f.recordedOps = append(f.recordedOps, *q) + return nil +} + +func (f *fakeRuleStore) UpsertAlertRules(q []store.UpsertRule) error { + f.mtx.Lock() + defer f.mtx.Unlock() + f.recordedOps = append(f.recordedOps, q) + return nil +} func (f *fakeRuleStore) UpdateRuleGroup(cmd store.UpdateRuleGroupCmd) error { f.mtx.Lock() defer f.mtx.Unlock() + f.recordedOps = append(f.recordedOps, cmd) rgs, ok := f.rules[cmd.OrgID] if !ok { f.rules[cmd.OrgID] = map[string]map[string][]*models.AlertRule{} @@ -138,7 +196,7 @@ func (f *fakeRuleStore) UpdateRuleGroup(cmd store.UpdateRuleGroupCmd) error { rules := []*models.AlertRule{} for _, r := range cmd.RuleGroupConfig.Rules { - //TODO: Not sure why this is not being set properly, where is the code that sets this? + // TODO: Not sure why this is not being set properly, where is the code that sets this? for i := range r.GrafanaManagedAlert.Data { r.GrafanaManagedAlert.Data[i].DatasourceUID = "-100" } @@ -181,13 +239,32 @@ func (f *fakeRuleStore) UpdateRuleGroup(cmd store.UpdateRuleGroupCmd) error { return nil } -type fakeInstanceStore struct{} +type fakeInstanceStore struct { + mtx sync.Mutex + recordedOps []interface{} +} -func (f *fakeInstanceStore) GetAlertInstance(_ *models.GetAlertInstanceQuery) error { return nil } -func (f *fakeInstanceStore) ListAlertInstances(_ *models.ListAlertInstancesQuery) error { return nil } -func (f *fakeInstanceStore) SaveAlertInstance(_ *models.SaveAlertInstanceCommand) error { return nil } -func (f *fakeInstanceStore) FetchOrgIds() ([]int64, error) { return []int64{}, nil } -func (f *fakeInstanceStore) DeleteAlertInstance(_ int64, _, _ string) error { return nil } +func (f *fakeInstanceStore) GetAlertInstance(q *models.GetAlertInstanceQuery) error { + f.mtx.Lock() + defer f.mtx.Unlock() + f.recordedOps = append(f.recordedOps, *q) + return nil +} +func (f *fakeInstanceStore) ListAlertInstances(q *models.ListAlertInstancesQuery) error { + f.mtx.Lock() + defer f.mtx.Unlock() + f.recordedOps = append(f.recordedOps, *q) + return nil +} +func (f *fakeInstanceStore) SaveAlertInstance(q *models.SaveAlertInstanceCommand) error { + f.mtx.Lock() + defer f.mtx.Unlock() + f.recordedOps = append(f.recordedOps, *q) + return nil +} + +func (f *fakeInstanceStore) FetchOrgIds() ([]int64, error) { return []int64{}, nil } +func (f *fakeInstanceStore) DeleteAlertInstance(_ int64, _, _ string) error { return nil } func newFakeAdminConfigStore(t *testing.T) *fakeAdminConfigStore { t.Helper() From 6e08e12749c349ef47698afd392b44c29af06fea Mon Sep 17 00:00:00 2001 From: Marcus Andersson Date: Tue, 26 Oct 2021 19:46:29 +0200 Subject: [PATCH 44/49] Docs: Updated the plugin admin configuration default value (#40942) --- docs/sources/administration/configuration.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/sources/administration/configuration.md b/docs/sources/administration/configuration.md index f421d8df4de..19eaa6cad3f 100644 --- a/docs/sources/administration/configuration.md +++ b/docs/sources/administration/configuration.md @@ -1582,7 +1582,7 @@ We do _not_ recommend using this option. For more information, refer to [Plugin ### plugin_admin_enabled -Available to Grafana administrators only, the plugin admin app is set to `false` by default. Set it to `true` to enable the app. +Available to Grafana administrators only, the plugin admin app is set to `true` by default. Set it to `false` to disable the app. For more information, refer to [Plugin catalog]({{< relref "../plugins/catalog.md" >}}). From dfbb3c4e23efbc9752079c679d583a3681c3aab5 Mon Sep 17 00:00:00 2001 From: Petros Kolyvas Date: Tue, 26 Oct 2021 14:49:19 -0300 Subject: [PATCH 45/49] Docs: Fix for clarifications about the image renderer (#40182) * Added time range controls updates * Added plugins catalog update * Added enterprise images * Added community contributions highlights for 8.2 * accessibility statement * Update docs/sources/whatsnew/whats-new-in-v8-2.md Co-authored-by: Fiona Artiaga <89225282+GrafanaWriter@users.noreply.github.com> * Update docs/sources/whatsnew/whats-new-in-v8-2.md Co-authored-by: Fiona Artiaga <89225282+GrafanaWriter@users.noreply.github.com> * Update docs/sources/whatsnew/whats-new-in-v8-2.md Co-authored-by: Fiona Artiaga <89225282+GrafanaWriter@users.noreply.github.com> * Update docs/sources/whatsnew/whats-new-in-v8-2.md Co-authored-by: Fiona Artiaga <89225282+GrafanaWriter@users.noreply.github.com> * Update docs/sources/whatsnew/whats-new-in-v8-2.md Co-authored-by: achatterjee-grafana <70489351+achatterjee-grafana@users.noreply.github.com> * Update docs/sources/whatsnew/whats-new-in-v8-2.md Co-authored-by: achatterjee-grafana <70489351+achatterjee-grafana@users.noreply.github.com> * Update docs/sources/whatsnew/whats-new-in-v8-2.md Co-authored-by: Fiona Artiaga <89225282+GrafanaWriter@users.noreply.github.com> * Update docs/sources/whatsnew/whats-new-in-v8-2.md Co-authored-by: achatterjee-grafana <70489351+achatterjee-grafana@users.noreply.github.com> * Update docs/sources/whatsnew/whats-new-in-v8-2.md Co-authored-by: achatterjee-grafana <70489351+achatterjee-grafana@users.noreply.github.com> * Apply suggestions from code review Co-authored-by: Fiona Artiaga <89225282+GrafanaWriter@users.noreply.github.com> * Ran prettier:write to see if we can fix the issues * Fix issues with template info * Merge fix * Template fix part 2 * What is a mergfix even * Additional final fixes * Markdown link error fix for time picker changes * Ran prettier -w again to fix linting issues * What's new fixes for image rendered Co-authored-by: Fiona Artiaga <89225282+GrafanaWriter@users.noreply.github.com> Co-authored-by: achatterjee-grafana <70489351+achatterjee-grafana@users.noreply.github.com> --- docs/sources/whatsnew/whats-new-in-v8-2.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/sources/whatsnew/whats-new-in-v8-2.md b/docs/sources/whatsnew/whats-new-in-v8-2.md index e7e9f5a3bb2..d682617613e 100644 --- a/docs/sources/whatsnew/whats-new-in-v8-2.md +++ b/docs/sources/whatsnew/whats-new-in-v8-2.md @@ -46,7 +46,7 @@ We’ve continued to bolster the new, unified alerting system launched in Grafan ## Image Renderer performance improvements and measurement -You can use Grafana’s image renderer to generate JPEG and PDF images of panels and dashboards. Use these images for alert notifications, PDF exports, and reports sent by Grafana. We’ve added additional metrics to the image renderer to help you diagnose its performance, and [included guidance in our documentation](https://grafana.com/docs/grafana/next/image-rendering/#rendering-mode) to help you configure it for the best mix of performance and resource usage. Tests show that we have reduced image load time from the 95th percentile of 10 seconds to less than 3 seconds under normal load. +You can use Grafana’s image renderer to generate images of panels and dashboards. Grafana uses these images for alert notifications, PDF exports (Grafana Enterprise), and reports sent by Grafana (Grafana Enterprise). We’ve added additional metrics to the image renderer to help you diagnose its performance, and [included guidance in our documentation](https://grafana.com/docs/grafana/next/image-rendering/#rendering-mode) to help you configure it for the best mix of performance and resource usage. Tests show that we have reduced image load time from the 95th percentile of 10 seconds to less than 3 seconds under normal load. # Grafana Enterprise From bce1011361000d92c953869ef777249f2050b6dd Mon Sep 17 00:00:00 2001 From: Skye <22365940+Skyebold@users.noreply.github.com> Date: Tue, 26 Oct 2021 11:55:10 -0700 Subject: [PATCH 46/49] Alerting: Option for Discord notifier to use webhook name (#40463) * Added an option to discord notifier to use discord's webhook name (useful for customizing notifications). * Support ngalert system with discord username toggle * Added ngalert discord test * Apply suggestions from code review Co-authored-by: gotjosh * Docs updated with discord username setting * Fix api integration test Co-authored-by: Marcus Efraimsson Co-authored-by: gotjosh --- docs/sources/administration/provisioning.md | 11 ++++--- .../alerting/old-alerting/notifications.md | 11 ++++--- pkg/services/alerting/notifiers/discord.go | 31 +++++++++++++------ .../ngalert/notifier/available_channels.go | 6 ++++ .../ngalert/notifier/channels/discord.go | 29 ++++++++++------- .../ngalert/notifier/channels/discord_test.go | 29 +++++++++++++++++ .../alerting/api_available_channel_test.go | 18 ++++++++++- 7 files changed, 103 insertions(+), 32 deletions(-) diff --git a/docs/sources/administration/provisioning.md b/docs/sources/administration/provisioning.md index 81243822598..effa8feabfd 100644 --- a/docs/sources/administration/provisioning.md +++ b/docs/sources/administration/provisioning.md @@ -446,11 +446,12 @@ The following sections detail the supported settings and secure settings for eac #### Alert notification `discord` -| Name | Secure setting | -| ---------- | -------------- | -| url | yes | -| avatar_url | | -| content | | +| Name | Secure setting | +| -------------------- | -------------- | +| url | yes | +| avatar_url | | +| content | | +| use_discord_username | | #### Alert notification `slack` diff --git a/docs/sources/alerting/old-alerting/notifications.md b/docs/sources/alerting/old-alerting/notifications.md index 4a1b06d7069..9bcf5bf025e 100644 --- a/docs/sources/alerting/old-alerting/notifications.md +++ b/docs/sources/alerting/old-alerting/notifications.md @@ -227,11 +227,12 @@ In DingTalk PC Client: To set up Discord, you must create a Discord channel webhook. For instructions on how to create the channel, refer to [Intro to Webhooks](https://support.discord.com/hc/en-us/articles/228383668-Intro-to-Webhooks). -| Setting | Description | -| --------------- | --------------------------------------------------------------------------------- | -| Webhook URL | Discord webhook URL. | -| Message Content | Mention a group using @ or a user using <@ID> when notifying in a channel. | -| Avatar URL | Optionally, provide a URL to an image to use as the avatar for the bot's message. | +| Setting | Description | +| ------------------------------ | ----------------------------------------------------------------------------------------------------- | +| Webhook URL | Discord webhook URL. | +| Message Content | Mention a group using @ or a user using <@ID> when notifying in a channel. | +| Avatar URL | Optionally, provide a URL to an image to use as the avatar for the bot's message. | +| Use Discord's Webhook Username | Use the username configured in Discord's webhook settings. Otherwise, the username will be 'Grafana.' | Alternately, use the [Slack](#slack) notifier by appending `/slack` to a Discord webhook URL. diff --git a/pkg/services/alerting/notifiers/discord.go b/pkg/services/alerting/notifiers/discord.go index 72534fdeee8..d0ac7036f7f 100644 --- a/pkg/services/alerting/notifiers/discord.go +++ b/pkg/services/alerting/notifiers/discord.go @@ -47,6 +47,12 @@ func init() { PropertyName: "url", Required: true, }, + { + Label: "Use Discord's Webhook Username", + Description: "Use the username configured in Discord's webhook settings. Otherwise, the username will be 'Grafana'", + Element: alerting.ElementTypeCheckbox, + PropertyName: "use_discord_username", + }, }, }) } @@ -58,13 +64,15 @@ func newDiscordNotifier(model *models.AlertNotification, _ alerting.GetDecrypted if url == "" { return nil, alerting.ValidationError{Reason: "Could not find webhook url property in settings"} } + useDiscordUsername := model.Settings.Get("use_discord_username").MustBool(false) return &DiscordNotifier{ - NotifierBase: NewNotifierBase(model), - Content: content, - AvatarURL: avatar, - WebhookURL: url, - log: log.New("alerting.notifier.discord"), + NotifierBase: NewNotifierBase(model), + Content: content, + AvatarURL: avatar, + WebhookURL: url, + log: log.New("alerting.notifier.discord"), + UseDiscordUsername: useDiscordUsername, }, nil } @@ -72,10 +80,11 @@ func newDiscordNotifier(model *models.AlertNotification, _ alerting.GetDecrypted // notifications to discord. type DiscordNotifier struct { NotifierBase - Content string - AvatarURL string - WebhookURL string - log log.Logger + Content string + AvatarURL string + WebhookURL string + log log.Logger + UseDiscordUsername bool } // Notify send an alert notification to Discord. @@ -89,7 +98,9 @@ func (dn *DiscordNotifier) Notify(evalContext *alerting.EvalContext) error { } bodyJSON := simplejson.New() - bodyJSON.Set("username", "Grafana") + if !dn.UseDiscordUsername { + bodyJSON.Set("username", "Grafana") + } if dn.Content != "" { bodyJSON.Set("content", dn.Content) diff --git a/pkg/services/ngalert/notifier/available_channels.go b/pkg/services/ngalert/notifier/available_channels.go index ddc79549b85..a3c23d87e98 100644 --- a/pkg/services/ngalert/notifier/available_channels.go +++ b/pkg/services/ngalert/notifier/available_channels.go @@ -694,6 +694,12 @@ func GetAvailableNotifiers() []*alerting.NotifierPlugin { InputType: alerting.InputTypeText, PropertyName: "avatar_url", }, + { + Label: "Use Discord's Webhook Username", + Description: "Use the username configured in Discord's webhook settings. Otherwise, the username will be 'Grafana'", + Element: alerting.ElementTypeCheckbox, + PropertyName: "use_discord_username", + }, }, }, { diff --git a/pkg/services/ngalert/notifier/channels/discord.go b/pkg/services/ngalert/notifier/channels/discord.go index faa1b53093a..5842445180d 100644 --- a/pkg/services/ngalert/notifier/channels/discord.go +++ b/pkg/services/ngalert/notifier/channels/discord.go @@ -18,11 +18,12 @@ import ( type DiscordNotifier struct { *Base - log log.Logger - tmpl *template.Template - Content string - AvatarURL string - WebhookURL string + log log.Logger + tmpl *template.Template + Content string + AvatarURL string + WebhookURL string + UseDiscordUsername bool } func NewDiscordNotifier(model *NotificationChannelConfig, t *template.Template) (*DiscordNotifier, error) { @@ -37,6 +38,8 @@ func NewDiscordNotifier(model *NotificationChannelConfig, t *template.Template) return nil, receiverInitError{Reason: "could not find webhook url property in settings", Cfg: *model} } + useDiscordUsername := model.Settings.Get("use_discord_username").MustBool(false) + content := model.Settings.Get("message").MustString(`{{ template "default.message" . }}`) return &DiscordNotifier{ @@ -48,11 +51,12 @@ func NewDiscordNotifier(model *NotificationChannelConfig, t *template.Template) Settings: model.Settings, SecureSettings: model.SecureSettings, }), - Content: content, - AvatarURL: avatarURL, - WebhookURL: discordURL, - log: log.New("alerting.notifier.discord"), - tmpl: t, + Content: content, + AvatarURL: avatarURL, + WebhookURL: discordURL, + log: log.New("alerting.notifier.discord"), + tmpl: t, + UseDiscordUsername: useDiscordUsername, }, nil } @@ -60,7 +64,10 @@ func (d DiscordNotifier) Notify(ctx context.Context, as ...*types.Alert) (bool, alerts := types.Alerts(as...) bodyJSON := simplejson.New() - bodyJSON.Set("username", "Grafana") + + if !d.UseDiscordUsername { + bodyJSON.Set("username", "Grafana") + } var tmplErr error tmpl, _ := TmplText(ctx, d.tmpl, as, d.log, &tmplErr) diff --git a/pkg/services/ngalert/notifier/channels/discord_test.go b/pkg/services/ngalert/notifier/channels/discord_test.go index 5c0965b810d..9fc1b947a92 100644 --- a/pkg/services/ngalert/notifier/channels/discord_test.go +++ b/pkg/services/ngalert/notifier/channels/discord_test.go @@ -100,6 +100,35 @@ func TestDiscordNotifier(t *testing.T) { settings: `{}`, expInitError: `failed to validate receiver "discord_testing" of type "discord": could not find webhook url property in settings`, }, + { + name: "Default config with one alert, use default discord username", + settings: `{ + "url": "http://localhost", + "use_discord_username": true + }`, + alerts: []*types.Alert{ + { + Alert: model.Alert{ + Labels: model.LabelSet{"alertname": "alert1", "lbl1": "val1"}, + Annotations: model.LabelSet{"ann1": "annv1", "__dashboardUid__": "abcd", "__panelId__": "efgh"}, + }, + }, + }, + expMsg: map[string]interface{}{ + "content": "**Firing**\n\nLabels:\n - alertname = alert1\n - lbl1 = val1\nAnnotations:\n - ann1 = annv1\nSilence: http://localhost/alerting/silence/new?alertmanager=grafana&matchers=alertname%3Dalert1%2Clbl1%3Dval1\nDashboard: http://localhost/d/abcd\nPanel: http://localhost/d/abcd?viewPanel=efgh\n", + "embeds": []interface{}{map[string]interface{}{ + "color": 1.4037554e+07, + "footer": map[string]interface{}{ + "icon_url": "https://grafana.com/assets/img/fav32.png", + "text": "Grafana v", + }, + "title": "[FIRING:1] (val1)", + "url": "http://localhost/alerting/list", + "type": "rich", + }}, + }, + expMsgError: nil, + }, } for _, c := range cases { diff --git a/pkg/tests/api/alerting/api_available_channel_test.go b/pkg/tests/api/alerting/api_available_channel_test.go index d5ee2a5ee3b..767d1a1b315 100644 --- a/pkg/tests/api/alerting/api_available_channel_test.go +++ b/pkg/tests/api/alerting/api_available_channel_test.go @@ -1405,7 +1405,23 @@ var expAvailableChannelJsonOutput = ` "required": false, "validationRule": "", "secure": false - } + }, + { + "element": "checkbox", + "inputType": "", + "label": "Use Discord's Webhook Username", + "description": "Use the username configured in Discord's webhook settings. Otherwise, the username will be 'Grafana'", + "placeholder": "", + "propertyName": "use_discord_username", + "selectOptions": null, + "showWhen": { + "field": "", + "is": "" + }, + "required": false, + "validationRule": "", + "secure": false + } ] }, { From 24a74cd06e3a829102669e3342473fabf180a92e Mon Sep 17 00:00:00 2001 From: Serge Zaitsev Date: Tue, 26 Oct 2021 23:24:58 +0200 Subject: [PATCH 47/49] Chore: Refactor GoConvey tests in alerting/conditions (#40843) * refactor goconvery tests * use more meaningful assertion * use more meaningful assertions --- .../alerting/conditions/evaluator_test.go | 55 +- .../conditions/query_interval_test.go | 59 +- .../alerting/conditions/query_test.go | 276 ++++---- .../alerting/conditions/reducer_test.go | 631 +++++++++--------- 4 files changed, 508 insertions(+), 513 deletions(-) diff --git a/pkg/services/alerting/conditions/evaluator_test.go b/pkg/services/alerting/conditions/evaluator_test.go index ce3100b59e4..f7188f8a49c 100644 --- a/pkg/services/alerting/conditions/evaluator_test.go +++ b/pkg/services/alerting/conditions/evaluator_test.go @@ -3,60 +3,59 @@ package conditions import ( "testing" - . "github.com/smartystreets/goconvey/convey" - "github.com/grafana/grafana/pkg/components/null" "github.com/grafana/grafana/pkg/components/simplejson" + "github.com/stretchr/testify/require" ) -func evaluatorScenario(json string, reducedValue float64, datapoints ...float64) bool { +func evaluatorScenario(t *testing.T, json string, reducedValue float64, datapoints ...float64) bool { jsonModel, err := simplejson.NewJson([]byte(json)) - So(err, ShouldBeNil) + require.NoError(t, err) evaluator, err := NewAlertEvaluator(jsonModel) - So(err, ShouldBeNil) + require.NoError(t, err) return evaluator.Eval(null.FloatFrom(reducedValue)) } func TestEvaluators(t *testing.T) { - Convey("greater then", t, func() { - So(evaluatorScenario(`{"type": "gt", "params": [1] }`, 3), ShouldBeTrue) - So(evaluatorScenario(`{"type": "gt", "params": [3] }`, 1), ShouldBeFalse) + t.Run("greater then", func(t *testing.T) { + require.True(t, evaluatorScenario(t, `{"type": "gt", "params": [1] }`, 3)) + require.False(t, evaluatorScenario(t, `{"type": "gt", "params": [3] }`, 1)) }) - Convey("less then", t, func() { - So(evaluatorScenario(`{"type": "lt", "params": [1] }`, 3), ShouldBeFalse) - So(evaluatorScenario(`{"type": "lt", "params": [3] }`, 1), ShouldBeTrue) + t.Run("less then", func(t *testing.T) { + require.False(t, evaluatorScenario(t, `{"type": "lt", "params": [1] }`, 3)) + require.True(t, evaluatorScenario(t, `{"type": "lt", "params": [3] }`, 1)) }) - Convey("within_range", t, func() { - So(evaluatorScenario(`{"type": "within_range", "params": [1, 100] }`, 3), ShouldBeTrue) - So(evaluatorScenario(`{"type": "within_range", "params": [1, 100] }`, 300), ShouldBeFalse) - So(evaluatorScenario(`{"type": "within_range", "params": [100, 1] }`, 3), ShouldBeTrue) - So(evaluatorScenario(`{"type": "within_range", "params": [100, 1] }`, 300), ShouldBeFalse) + t.Run("within_range", func(t *testing.T) { + require.True(t, evaluatorScenario(t, `{"type": "within_range", "params": [1, 100] }`, 3)) + require.False(t, evaluatorScenario(t, `{"type": "within_range", "params": [1, 100] }`, 300)) + require.True(t, evaluatorScenario(t, `{"type": "within_range", "params": [100, 1] }`, 3)) + require.False(t, evaluatorScenario(t, `{"type": "within_range", "params": [100, 1] }`, 300)) }) - Convey("outside_range", t, func() { - So(evaluatorScenario(`{"type": "outside_range", "params": [1, 100] }`, 1000), ShouldBeTrue) - So(evaluatorScenario(`{"type": "outside_range", "params": [1, 100] }`, 50), ShouldBeFalse) - So(evaluatorScenario(`{"type": "outside_range", "params": [100, 1] }`, 1000), ShouldBeTrue) - So(evaluatorScenario(`{"type": "outside_range", "params": [100, 1] }`, 50), ShouldBeFalse) + t.Run("outside_range", func(t *testing.T) { + require.True(t, evaluatorScenario(t, `{"type": "outside_range", "params": [1, 100] }`, 1000)) + require.False(t, evaluatorScenario(t, `{"type": "outside_range", "params": [1, 100] }`, 50)) + require.True(t, evaluatorScenario(t, `{"type": "outside_range", "params": [100, 1] }`, 1000)) + require.False(t, evaluatorScenario(t, `{"type": "outside_range", "params": [100, 1] }`, 50)) }) - Convey("no_value", t, func() { - Convey("should be false if series have values", func() { - So(evaluatorScenario(`{"type": "no_value", "params": [] }`, 50), ShouldBeFalse) + t.Run("no_value", func(t *testing.T) { + t.Run("should be false if series have values", func(t *testing.T) { + require.False(t, evaluatorScenario(t, `{"type": "no_value", "params": [] }`, 50)) }) - Convey("should be true when the series have no value", func() { + t.Run("should be true when the series have no value", func(t *testing.T) { jsonModel, err := simplejson.NewJson([]byte(`{"type": "no_value", "params": [] }`)) - So(err, ShouldBeNil) + require.NoError(t, err) evaluator, err := NewAlertEvaluator(jsonModel) - So(err, ShouldBeNil) + require.NoError(t, err) - So(evaluator.Eval(null.FloatFromPtr(nil)), ShouldBeTrue) + require.True(t, evaluator.Eval(null.FloatFromPtr(nil))) }) }) } diff --git a/pkg/services/alerting/conditions/query_interval_test.go b/pkg/services/alerting/conditions/query_interval_test.go index c3e6876d79e..33b06c8a497 100644 --- a/pkg/services/alerting/conditions/query_interval_test.go +++ b/pkg/services/alerting/conditions/query_interval_test.go @@ -12,12 +12,13 @@ import ( "github.com/grafana/grafana/pkg/models" "github.com/grafana/grafana/pkg/plugins" "github.com/grafana/grafana/pkg/services/alerting" - . "github.com/smartystreets/goconvey/convey" + + "github.com/stretchr/testify/require" ) func TestQueryInterval(t *testing.T) { - Convey("When evaluating query condition, regarding the interval value", t, func() { - Convey("Can handle interval-calculation with no panel-min-interval and no datasource-min-interval", func() { + t.Run("When evaluating query condition, regarding the interval value", func(t *testing.T) { + t.Run("Can handle interval-calculation with no panel-min-interval and no datasource-min-interval", func(t *testing.T) { // no panel-min-interval in the queryModel queryModel := `{"target": "aliasByNode(statsd.fakesite.counters.session_start.mobile.count, 4)"}` @@ -29,13 +30,13 @@ func TestQueryInterval(t *testing.T) { verifier := func(query plugins.DataSubQuery) { // 5minutes timerange = 300000milliseconds; default-resolution is 1500pixels, // so we should have 300000/1500 = 200milliseconds here - So(query.IntervalMS, ShouldEqual, 200) - So(query.MaxDataPoints, ShouldEqual, interval.DefaultRes) + require.Equal(t, int64(200), query.IntervalMS) + require.Equal(t, interval.DefaultRes, query.MaxDataPoints) } - applyScenario(timeRange, dataSourceJson, queryModel, verifier) + applyScenario(t, timeRange, dataSourceJson, queryModel, verifier) }) - Convey("Can handle interval-calculation with panel-min-interval and no datasource-min-interval", func() { + t.Run("Can handle interval-calculation with panel-min-interval and no datasource-min-interval", func(t *testing.T) { // panel-min-interval in the queryModel queryModel := `{"interval":"123s", "target": "aliasByNode(statsd.fakesite.counters.session_start.mobile.count, 4)"}` @@ -45,13 +46,13 @@ func TestQueryInterval(t *testing.T) { timeRange := "5m" verifier := func(query plugins.DataSubQuery) { - So(query.IntervalMS, ShouldEqual, 123000) - So(query.MaxDataPoints, ShouldEqual, interval.DefaultRes) + require.Equal(t, int64(123000), query.IntervalMS) + require.Equal(t, interval.DefaultRes, query.MaxDataPoints) } - applyScenario(timeRange, dataSourceJson, queryModel, verifier) + applyScenario(t, timeRange, dataSourceJson, queryModel, verifier) }) - Convey("Can handle interval-calculation with no panel-min-interval and datasource-min-interval", func() { + t.Run("Can handle interval-calculation with no panel-min-interval and datasource-min-interval", func(t *testing.T) { // no panel-min-interval in the queryModel queryModel := `{"target": "aliasByNode(statsd.fakesite.counters.session_start.mobile.count, 4)"}` @@ -59,18 +60,18 @@ func TestQueryInterval(t *testing.T) { dataSourceJson, err := simplejson.NewJson([]byte(`{ "timeInterval": "71s" }`)) - So(err, ShouldBeNil) + require.Nil(t, err) timeRange := "5m" verifier := func(query plugins.DataSubQuery) { - So(query.IntervalMS, ShouldEqual, 71000) - So(query.MaxDataPoints, ShouldEqual, interval.DefaultRes) + require.Equal(t, int64(71000), query.IntervalMS) + require.Equal(t, interval.DefaultRes, query.MaxDataPoints) } - applyScenario(timeRange, dataSourceJson, queryModel, verifier) + applyScenario(t, timeRange, dataSourceJson, queryModel, verifier) }) - Convey("Can handle interval-calculation with both panel-min-interval and datasource-min-interval", func() { + t.Run("Can handle interval-calculation with both panel-min-interval and datasource-min-interval", func(t *testing.T) { // panel-min-interval in the queryModel queryModel := `{"interval":"19s", "target": "aliasByNode(statsd.fakesite.counters.session_start.mobile.count, 4)"}` @@ -78,21 +79,21 @@ func TestQueryInterval(t *testing.T) { dataSourceJson, err := simplejson.NewJson([]byte(`{ "timeInterval": "71s" }`)) - So(err, ShouldBeNil) + require.Nil(t, err) timeRange := "5m" verifier := func(query plugins.DataSubQuery) { // when both panel-min-interval and datasource-min-interval exists, // panel-min-interval is used - So(query.IntervalMS, ShouldEqual, 19000) - So(query.MaxDataPoints, ShouldEqual, interval.DefaultRes) + require.Equal(t, int64(19000), query.IntervalMS) + require.Equal(t, interval.DefaultRes, query.MaxDataPoints) } - applyScenario(timeRange, dataSourceJson, queryModel, verifier) + applyScenario(t, timeRange, dataSourceJson, queryModel, verifier) }) - Convey("Can handle no min-interval, and very small time-ranges, where the default-min-interval=1ms applies", func() { + t.Run("Can handle no min-interval, and very small time-ranges, where the default-min-interval=1ms applies", func(t *testing.T) { // no panel-min-interval in the queryModel queryModel := `{"target": "aliasByNode(statsd.fakesite.counters.session_start.mobile.count, 4)"}` @@ -104,11 +105,11 @@ func TestQueryInterval(t *testing.T) { verifier := func(query plugins.DataSubQuery) { // no min-interval exists, the default-min-interval will be used, // and for such a short time-range this will cause the value to be 1millisecond. - So(query.IntervalMS, ShouldEqual, 1) - So(query.MaxDataPoints, ShouldEqual, interval.DefaultRes) + require.Equal(t, int64(1), query.IntervalMS) + require.Equal(t, interval.DefaultRes, query.MaxDataPoints) } - applyScenario(timeRange, dataSourceJson, queryModel, verifier) + applyScenario(t, timeRange, dataSourceJson, queryModel, verifier) }) }) } @@ -135,8 +136,8 @@ func (rh fakeIntervalTestReqHandler) HandleRequest(ctx context.Context, dsInfo * } //nolint: staticcheck // plugins.DataResponse deprecated -func applyScenario(timeRange string, dataSourceJsonData *simplejson.Json, queryModel string, verifier func(query plugins.DataSubQuery)) { - Convey("desc", func() { +func applyScenario(t *testing.T, timeRange string, dataSourceJsonData *simplejson.Json, queryModel string, verifier func(query plugins.DataSubQuery)) { + t.Run("desc", func(t *testing.T) { bus.AddHandlerCtx("test", func(ctx context.Context, query *models.GetDataSourceQuery) error { query.Result = &models.DataSource{Id: 1, Type: "graphite", JsonData: dataSourceJsonData} return nil @@ -159,10 +160,10 @@ func applyScenario(timeRange string, dataSourceJsonData *simplejson.Json, queryM "reducer":{"type": "avg"}, "evaluator":{"type": "gt", "params": [100]} }`)) - So(err, ShouldBeNil) + require.Nil(t, err) condition, err := newQueryCondition(jsonModel, 0) - So(err, ShouldBeNil) + require.Nil(t, err) ctx.condition = condition @@ -179,6 +180,6 @@ func applyScenario(timeRange string, dataSourceJsonData *simplejson.Json, queryM _, err = condition.Eval(ctx.result, reqHandler) - So(err, ShouldBeNil) + require.Nil(t, err) }) } diff --git a/pkg/services/alerting/conditions/query_test.go b/pkg/services/alerting/conditions/query_test.go index 1021ba61fb0..7b88c6d0749 100644 --- a/pkg/services/alerting/conditions/query_test.go +++ b/pkg/services/alerting/conditions/query_test.go @@ -17,7 +17,7 @@ import ( "github.com/grafana/grafana/pkg/models" "github.com/grafana/grafana/pkg/plugins" "github.com/grafana/grafana/pkg/services/alerting" - . "github.com/smartystreets/goconvey/convey" + "github.com/stretchr/testify/require" "github.com/xorcare/pointer" ) @@ -33,147 +33,165 @@ func newTimeSeriesPointsFromArgs(values ...float64) plugins.DataTimeSeriesPoints } func TestQueryCondition(t *testing.T) { - Convey("when evaluating query condition", t, func() { - queryConditionScenario("Given avg() and > 100", func(ctx *queryConditionTestContext) { - ctx.reducer = `{"type": "avg"}` - ctx.evaluator = `{"type": "gt", "params": [100]}` + setup := func() *queryConditionTestContext { + ctx := &queryConditionTestContext{} + bus.AddHandlerCtx("test", func(ctx context.Context, query *models.GetDataSourceQuery) error { + query.Result = &models.DataSource{Id: 1, Type: "graphite"} + return nil + }) - Convey("Can read query condition from json model", func() { - _, err := ctx.exec() - So(err, ShouldBeNil) + ctx.reducer = `{"type":"avg"}` + ctx.evaluator = `{"type":"gt","params":[100]}` + ctx.result = &alerting.EvalContext{ + Ctx: context.Background(), + Rule: &alerting.Rule{}, + RequestValidator: &validations.OSSPluginRequestValidator{}, + } + return ctx + } - So(ctx.condition.Query.From, ShouldEqual, "5m") - So(ctx.condition.Query.To, ShouldEqual, "now") - So(ctx.condition.Query.DatasourceID, ShouldEqual, 1) + t.Run("Can read query condition from json model", func(t *testing.T) { + ctx := setup() + _, err := ctx.exec(t) + require.Nil(t, err) - Convey("Can read query reducer", func() { - reducer := ctx.condition.Reducer - So(reducer.Type, ShouldEqual, "avg") - }) + require.Equal(t, "5m", ctx.condition.Query.From) + require.Equal(t, "now", ctx.condition.Query.To) + require.Equal(t, int64(1), ctx.condition.Query.DatasourceID) - Convey("Can read evaluator", func() { - evaluator, ok := ctx.condition.Evaluator.(*thresholdEvaluator) - So(ok, ShouldBeTrue) - So(evaluator.Type, ShouldEqual, "gt") - }) - }) + t.Run("Can read query reducer", func(t *testing.T) { + reducer := ctx.condition.Reducer + require.Equal(t, "avg", reducer.Type) + }) - Convey("should fire when avg is above 100", func() { - points := newTimeSeriesPointsFromArgs(120, 0) - ctx.series = plugins.DataTimeSeriesSlice{plugins.DataTimeSeries{Name: "test1", Points: points}} - cr, err := ctx.exec() + t.Run("Can read evaluator", func(t *testing.T) { + evaluator, ok := ctx.condition.Evaluator.(*thresholdEvaluator) + require.True(t, ok) + require.Equal(t, "gt", evaluator.Type) + }) + }) - So(err, ShouldBeNil) - So(cr.Firing, ShouldBeTrue) - }) + t.Run("should fire when avg is above 100", func(t *testing.T) { + ctx := setup() + points := newTimeSeriesPointsFromArgs(120, 0) + ctx.series = plugins.DataTimeSeriesSlice{plugins.DataTimeSeries{Name: "test1", Points: points}} + cr, err := ctx.exec(t) - Convey("should fire when avg is above 100 on dataframe", func() { - ctx.frame = data.NewFrame("", - data.NewField("time", nil, []time.Time{time.Now(), time.Now()}), - data.NewField("val", nil, []int64{120, 150}), - ) - cr, err := ctx.exec() + require.Nil(t, err) + require.True(t, cr.Firing) + }) - So(err, ShouldBeNil) - So(cr.Firing, ShouldBeTrue) - }) + t.Run("should fire when avg is above 100 on dataframe", func(t *testing.T) { + ctx := setup() + ctx.frame = data.NewFrame("", + data.NewField("time", nil, []time.Time{time.Now(), time.Now()}), + data.NewField("val", nil, []int64{120, 150}), + ) + cr, err := ctx.exec(t) - Convey("Should not fire when avg is below 100", func() { - points := newTimeSeriesPointsFromArgs(90, 0) - ctx.series = plugins.DataTimeSeriesSlice{plugins.DataTimeSeries{Name: "test1", Points: points}} - cr, err := ctx.exec() + require.Nil(t, err) + require.True(t, cr.Firing) + }) - So(err, ShouldBeNil) - So(cr.Firing, ShouldBeFalse) - }) + t.Run("Should not fire when avg is below 100", func(t *testing.T) { + ctx := setup() + points := newTimeSeriesPointsFromArgs(90, 0) + ctx.series = plugins.DataTimeSeriesSlice{plugins.DataTimeSeries{Name: "test1", Points: points}} + cr, err := ctx.exec(t) - Convey("Should not fire when avg is below 100 on dataframe", func() { - ctx.frame = data.NewFrame("", - data.NewField("time", nil, []time.Time{time.Now(), time.Now()}), - data.NewField("val", nil, []int64{12, 47}), - ) - cr, err := ctx.exec() + require.Nil(t, err) + require.False(t, cr.Firing) + }) - So(err, ShouldBeNil) - So(cr.Firing, ShouldBeFalse) - }) + t.Run("Should not fire when avg is below 100 on dataframe", func(t *testing.T) { + ctx := setup() + ctx.frame = data.NewFrame("", + data.NewField("time", nil, []time.Time{time.Now(), time.Now()}), + data.NewField("val", nil, []int64{12, 47}), + ) + cr, err := ctx.exec(t) - Convey("Should fire if only first series matches", func() { - ctx.series = plugins.DataTimeSeriesSlice{ - plugins.DataTimeSeries{Name: "test1", Points: newTimeSeriesPointsFromArgs(120, 0)}, - plugins.DataTimeSeries{Name: "test2", Points: newTimeSeriesPointsFromArgs(0, 0)}, - } - cr, err := ctx.exec() + require.Nil(t, err) + require.False(t, cr.Firing) + }) - So(err, ShouldBeNil) - So(cr.Firing, ShouldBeTrue) - }) + t.Run("Should fire if only first series matches", func(t *testing.T) { + ctx := setup() + ctx.series = plugins.DataTimeSeriesSlice{ + plugins.DataTimeSeries{Name: "test1", Points: newTimeSeriesPointsFromArgs(120, 0)}, + plugins.DataTimeSeries{Name: "test2", Points: newTimeSeriesPointsFromArgs(0, 0)}, + } + cr, err := ctx.exec(t) - Convey("No series", func() { - Convey("Should set NoDataFound when condition is gt", func() { - ctx.series = plugins.DataTimeSeriesSlice{} - cr, err := ctx.exec() + require.Nil(t, err) + require.True(t, cr.Firing) + }) - So(err, ShouldBeNil) - So(cr.Firing, ShouldBeFalse) - So(cr.NoDataFound, ShouldBeTrue) - }) + t.Run("No series", func(t *testing.T) { + ctx := setup() + t.Run("Should set NoDataFound when condition is gt", func(t *testing.T) { + ctx.series = plugins.DataTimeSeriesSlice{} + cr, err := ctx.exec(t) - Convey("Should be firing when condition is no_value", func() { - ctx.evaluator = `{"type": "no_value", "params": []}` - ctx.series = plugins.DataTimeSeriesSlice{} - cr, err := ctx.exec() + require.Nil(t, err) + require.False(t, cr.Firing) + require.True(t, cr.NoDataFound) + }) - So(err, ShouldBeNil) - So(cr.Firing, ShouldBeTrue) - }) - }) + t.Run("Should be firing when condition is no_value", func(t *testing.T) { + ctx.evaluator = `{"type": "no_value", "params": []}` + ctx.series = plugins.DataTimeSeriesSlice{} + cr, err := ctx.exec(t) - Convey("Empty series", func() { - Convey("Should set Firing if eval match", func() { - ctx.evaluator = `{"type": "no_value", "params": []}` - ctx.series = plugins.DataTimeSeriesSlice{ - plugins.DataTimeSeries{Name: "test1", Points: newTimeSeriesPointsFromArgs()}, - } - cr, err := ctx.exec() + require.Nil(t, err) + require.True(t, cr.Firing) + }) + }) - So(err, ShouldBeNil) - So(cr.Firing, ShouldBeTrue) - }) + t.Run("Empty series", func(t *testing.T) { + ctx := setup() + t.Run("Should set Firing if eval match", func(t *testing.T) { + ctx.evaluator = `{"type": "no_value", "params": []}` + ctx.series = plugins.DataTimeSeriesSlice{ + plugins.DataTimeSeries{Name: "test1", Points: newTimeSeriesPointsFromArgs()}, + } + cr, err := ctx.exec(t) - Convey("Should set NoDataFound both series are empty", func() { - ctx.series = plugins.DataTimeSeriesSlice{ - plugins.DataTimeSeries{Name: "test1", Points: newTimeSeriesPointsFromArgs()}, - plugins.DataTimeSeries{Name: "test2", Points: newTimeSeriesPointsFromArgs()}, - } - cr, err := ctx.exec() + require.Nil(t, err) + require.True(t, cr.Firing) + }) - So(err, ShouldBeNil) - So(cr.NoDataFound, ShouldBeTrue) - }) + t.Run("Should set NoDataFound both series are empty", func(t *testing.T) { + ctx.series = plugins.DataTimeSeriesSlice{ + plugins.DataTimeSeries{Name: "test1", Points: newTimeSeriesPointsFromArgs()}, + plugins.DataTimeSeries{Name: "test2", Points: newTimeSeriesPointsFromArgs()}, + } + cr, err := ctx.exec(t) - Convey("Should set NoDataFound both series contains null", func() { - ctx.series = plugins.DataTimeSeriesSlice{ - plugins.DataTimeSeries{Name: "test1", Points: plugins.DataTimeSeriesPoints{plugins.DataTimePoint{null.FloatFromPtr(nil), null.FloatFrom(0)}}}, - plugins.DataTimeSeries{Name: "test2", Points: plugins.DataTimeSeriesPoints{plugins.DataTimePoint{null.FloatFromPtr(nil), null.FloatFrom(0)}}}, - } - cr, err := ctx.exec() + require.Nil(t, err) + require.True(t, cr.NoDataFound) + }) - So(err, ShouldBeNil) - So(cr.NoDataFound, ShouldBeTrue) - }) + t.Run("Should set NoDataFound both series contains null", func(t *testing.T) { + ctx.series = plugins.DataTimeSeriesSlice{ + plugins.DataTimeSeries{Name: "test1", Points: plugins.DataTimeSeriesPoints{plugins.DataTimePoint{null.FloatFromPtr(nil), null.FloatFrom(0)}}}, + plugins.DataTimeSeries{Name: "test2", Points: plugins.DataTimeSeriesPoints{plugins.DataTimePoint{null.FloatFromPtr(nil), null.FloatFrom(0)}}}, + } + cr, err := ctx.exec(t) - Convey("Should not set NoDataFound if one series is empty", func() { - ctx.series = plugins.DataTimeSeriesSlice{ - plugins.DataTimeSeries{Name: "test1", Points: newTimeSeriesPointsFromArgs()}, - plugins.DataTimeSeries{Name: "test2", Points: newTimeSeriesPointsFromArgs(120, 0)}, - } - cr, err := ctx.exec() + require.Nil(t, err) + require.True(t, cr.NoDataFound) + }) - So(err, ShouldBeNil) - So(cr.NoDataFound, ShouldBeFalse) - }) - }) + t.Run("Should not set NoDataFound if one series is empty", func(t *testing.T) { + ctx.series = plugins.DataTimeSeriesSlice{ + plugins.DataTimeSeries{Name: "test1", Points: newTimeSeriesPointsFromArgs()}, + plugins.DataTimeSeries{Name: "test2", Points: newTimeSeriesPointsFromArgs(120, 0)}, + } + cr, err := ctx.exec(t) + + require.Nil(t, err) + require.False(t, cr.NoDataFound) }) }) } @@ -187,10 +205,8 @@ type queryConditionTestContext struct { condition *QueryCondition } -type queryConditionScenarioFunc func(c *queryConditionTestContext) - //nolint: staticcheck // plugins.DataPlugin deprecated -func (ctx *queryConditionTestContext) exec() (*alerting.ConditionResult, error) { +func (ctx *queryConditionTestContext) exec(t *testing.T) (*alerting.ConditionResult, error) { jsonModel, err := simplejson.NewJson([]byte(`{ "type": "query", "query": { @@ -201,10 +217,10 @@ func (ctx *queryConditionTestContext) exec() (*alerting.ConditionResult, error) "reducer":` + ctx.reducer + `, "evaluator":` + ctx.evaluator + ` }`)) - So(err, ShouldBeNil) + require.Nil(t, err) condition, err := newQueryCondition(jsonModel, 0) - So(err, ShouldBeNil) + require.Nil(t, err) ctx.condition = condition @@ -239,24 +255,6 @@ func (rh fakeReqHandler) HandleRequest(context.Context, *models.DataSource, plug return rh.response, nil } -func queryConditionScenario(desc string, fn queryConditionScenarioFunc) { - Convey(desc, func() { - bus.AddHandlerCtx("test", func(ctx context.Context, query *models.GetDataSourceQuery) error { - query.Result = &models.DataSource{Id: 1, Type: "graphite"} - return nil - }) - - ctx := &queryConditionTestContext{} - ctx.result = &alerting.EvalContext{ - Ctx: context.Background(), - Rule: &alerting.Rule{}, - RequestValidator: &validations.OSSPluginRequestValidator{}, - } - - fn(ctx) - }) -} - func TestFrameToSeriesSlice(t *testing.T) { tests := []struct { name string diff --git a/pkg/services/alerting/conditions/reducer_test.go b/pkg/services/alerting/conditions/reducer_test.go index baa46925785..f60c78d2db1 100644 --- a/pkg/services/alerting/conditions/reducer_test.go +++ b/pkg/services/alerting/conditions/reducer_test.go @@ -4,183 +4,103 @@ import ( "math" "testing" - . "github.com/smartystreets/goconvey/convey" - "github.com/grafana/grafana/pkg/components/null" "github.com/grafana/grafana/pkg/plugins" + "github.com/stretchr/testify/require" ) func TestSimpleReducer(t *testing.T) { - Convey("Test simple reducer by calculating", t, func() { - Convey("sum", func() { - result := testReducer("sum", 1, 2, 3) - So(result, ShouldEqual, float64(6)) - }) + t.Run("sum", func(t *testing.T) { + result := testReducer("sum", 1, 2, 3) + require.Equal(t, float64(6), result) + }) - Convey("min", func() { - result := testReducer("min", 3, 2, 1) - So(result, ShouldEqual, float64(1)) - }) + t.Run("min", func(t *testing.T) { + result := testReducer("min", 3, 2, 1) + require.Equal(t, float64(1), result) + }) - Convey("max", func() { - result := testReducer("max", 1, 2, 3) - So(result, ShouldEqual, float64(3)) - }) + t.Run("max", func(t *testing.T) { + result := testReducer("max", 1, 2, 3) + require.Equal(t, float64(3), result) + }) - Convey("count", func() { - result := testReducer("count", 1, 2, 3000) - So(result, ShouldEqual, float64(3)) - }) + t.Run("count", func(t *testing.T) { + result := testReducer("count", 1, 2, 3000) + require.Equal(t, float64(3), result) + }) - Convey("last", func() { - result := testReducer("last", 1, 2, 3000) - So(result, ShouldEqual, float64(3000)) - }) + t.Run("last", func(t *testing.T) { + result := testReducer("last", 1, 2, 3000) + require.Equal(t, float64(3000), result) + }) - Convey("median odd amount of numbers", func() { - result := testReducer("median", 1, 2, 3000) - So(result, ShouldEqual, float64(2)) - }) + t.Run("median odd amount of numbers", func(t *testing.T) { + result := testReducer("median", 1, 2, 3000) + require.Equal(t, float64(2), result) + }) - Convey("median even amount of numbers", func() { - result := testReducer("median", 1, 2, 4, 3000) - So(result, ShouldEqual, float64(3)) - }) + t.Run("median even amount of numbers", func(t *testing.T) { + result := testReducer("median", 1, 2, 4, 3000) + require.Equal(t, float64(3), result) + }) - Convey("median with one values", func() { - result := testReducer("median", 1) - So(result, ShouldEqual, float64(1)) - }) + t.Run("median with one values", func(t *testing.T) { + result := testReducer("median", 1) + require.Equal(t, float64(1), result) + }) - Convey("median should ignore null values", func() { - reducer := newSimpleReducer("median") + t.Run("median should ignore null values", func(t *testing.T) { + reducer := newSimpleReducer("median") + series := plugins.DataTimeSeries{ + Name: "test time series", + } + + series.Points = append(series.Points, plugins.DataTimePoint{null.FloatFromPtr(nil), null.FloatFrom(1)}) + series.Points = append(series.Points, plugins.DataTimePoint{null.FloatFromPtr(nil), null.FloatFrom(2)}) + series.Points = append(series.Points, plugins.DataTimePoint{null.FloatFromPtr(nil), null.FloatFrom(3)}) + series.Points = append(series.Points, plugins.DataTimePoint{null.FloatFrom(float64(1)), null.FloatFrom(4)}) + series.Points = append(series.Points, plugins.DataTimePoint{null.FloatFrom(float64(2)), null.FloatFrom(5)}) + series.Points = append(series.Points, plugins.DataTimePoint{null.FloatFrom(float64(3)), null.FloatFrom(6)}) + + result := reducer.Reduce(series) + require.Equal(t, true, result.Valid) + require.Equal(t, float64(2), result.Float64) + }) + + t.Run("avg", func(t *testing.T) { + result := testReducer("avg", 1, 2, 3) + require.Equal(t, float64(2), result) + }) + + t.Run("avg with only nulls", func(t *testing.T) { + reducer := newSimpleReducer("avg") + series := plugins.DataTimeSeries{ + Name: "test time series", + } + + series.Points = append(series.Points, plugins.DataTimePoint{null.FloatFromPtr(nil), null.FloatFrom(1)}) + require.Equal(t, false, reducer.Reduce(series).Valid) + }) + + t.Run("count_non_null", func(t *testing.T) { + t.Run("with null values and real values", func(t *testing.T) { + reducer := newSimpleReducer("count_non_null") series := plugins.DataTimeSeries{ Name: "test time series", } series.Points = append(series.Points, plugins.DataTimePoint{null.FloatFromPtr(nil), null.FloatFrom(1)}) series.Points = append(series.Points, plugins.DataTimePoint{null.FloatFromPtr(nil), null.FloatFrom(2)}) - series.Points = append(series.Points, plugins.DataTimePoint{null.FloatFromPtr(nil), null.FloatFrom(3)}) - series.Points = append(series.Points, plugins.DataTimePoint{null.FloatFrom(float64(1)), null.FloatFrom(4)}) - series.Points = append(series.Points, plugins.DataTimePoint{null.FloatFrom(float64(2)), null.FloatFrom(5)}) - series.Points = append(series.Points, plugins.DataTimePoint{null.FloatFrom(float64(3)), null.FloatFrom(6)}) - - result := reducer.Reduce(series) - So(result.Valid, ShouldEqual, true) - So(result.Float64, ShouldEqual, float64(2)) - }) - - Convey("avg", func() { - result := testReducer("avg", 1, 2, 3) - So(result, ShouldEqual, float64(2)) - }) - - Convey("avg with only nulls", func() { - reducer := newSimpleReducer("avg") - series := plugins.DataTimeSeries{ - Name: "test time series", - } - - series.Points = append(series.Points, plugins.DataTimePoint{null.FloatFromPtr(nil), null.FloatFrom(1)}) - So(reducer.Reduce(series).Valid, ShouldEqual, false) - }) - - Convey("count_non_null", func() { - Convey("with null values and real values", func() { - reducer := newSimpleReducer("count_non_null") - series := plugins.DataTimeSeries{ - Name: "test time series", - } - - series.Points = append(series.Points, plugins.DataTimePoint{null.FloatFromPtr(nil), null.FloatFrom(1)}) - series.Points = append(series.Points, plugins.DataTimePoint{null.FloatFromPtr(nil), null.FloatFrom(2)}) - series.Points = append(series.Points, plugins.DataTimePoint{null.FloatFrom(3), null.FloatFrom(3)}) - series.Points = append(series.Points, plugins.DataTimePoint{null.FloatFrom(3), null.FloatFrom(4)}) - - So(reducer.Reduce(series).Valid, ShouldEqual, true) - So(reducer.Reduce(series).Float64, ShouldEqual, 2) - }) - - Convey("with null values", func() { - reducer := newSimpleReducer("count_non_null") - series := plugins.DataTimeSeries{ - Name: "test time series", - } - - series.Points = append(series.Points, plugins.DataTimePoint{null.FloatFromPtr(nil), null.FloatFrom(1)}) - series.Points = append(series.Points, plugins.DataTimePoint{null.FloatFromPtr(nil), null.FloatFrom(2)}) - - So(reducer.Reduce(series).Valid, ShouldEqual, false) - }) - }) - - Convey("avg of number values and null values should ignore nulls", func() { - reducer := newSimpleReducer("avg") - series := plugins.DataTimeSeries{ - Name: "test time series", - } - - series.Points = append(series.Points, plugins.DataTimePoint{null.FloatFrom(3), null.FloatFrom(1)}) - series.Points = append(series.Points, plugins.DataTimePoint{null.FloatFromPtr(nil), null.FloatFrom(2)}) - series.Points = append(series.Points, plugins.DataTimePoint{null.FloatFromPtr(nil), null.FloatFrom(3)}) + series.Points = append(series.Points, plugins.DataTimePoint{null.FloatFrom(3), null.FloatFrom(3)}) series.Points = append(series.Points, plugins.DataTimePoint{null.FloatFrom(3), null.FloatFrom(4)}) - So(reducer.Reduce(series).Float64, ShouldEqual, float64(3)) + require.Equal(t, true, reducer.Reduce(series).Valid) + require.Equal(t, 2.0, reducer.Reduce(series).Float64) }) - // diff function Test Suite - Convey("diff of one positive point", func() { - result := testReducer("diff", 30) - So(result, ShouldEqual, float64(0)) - }) - - Convey("diff of one negative point", func() { - result := testReducer("diff", -30) - So(result, ShouldEqual, float64(0)) - }) - - Convey("diff of two positive points[1]", func() { - result := testReducer("diff", 30, 40) - So(result, ShouldEqual, float64(10)) - }) - - Convey("diff of two positive points[2]", func() { - result := testReducer("diff", 30, 20) - So(result, ShouldEqual, float64(-10)) - }) - - Convey("diff of two negative points[1]", func() { - result := testReducer("diff", -30, -40) - So(result, ShouldEqual, float64(-10)) - }) - - Convey("diff of two negative points[2]", func() { - result := testReducer("diff", -30, -10) - So(result, ShouldEqual, float64(20)) - }) - - Convey("diff of one positive and one negative point", func() { - result := testReducer("diff", 30, -40) - So(result, ShouldEqual, float64(-70)) - }) - - Convey("diff of one negative and one positive point", func() { - result := testReducer("diff", -30, 40) - So(result, ShouldEqual, float64(70)) - }) - - Convey("diff of three positive points", func() { - result := testReducer("diff", 30, 40, 50) - So(result, ShouldEqual, float64(20)) - }) - - Convey("diff of three negative points", func() { - result := testReducer("diff", -30, -40, -50) - So(result, ShouldEqual, float64(-20)) - }) - - Convey("diff with only nulls", func() { - reducer := newSimpleReducer("diff") + t.Run("with null values", func(t *testing.T) { + reducer := newSimpleReducer("count_non_null") series := plugins.DataTimeSeries{ Name: "test time series", } @@ -188,212 +108,289 @@ func TestSimpleReducer(t *testing.T) { series.Points = append(series.Points, plugins.DataTimePoint{null.FloatFromPtr(nil), null.FloatFrom(1)}) series.Points = append(series.Points, plugins.DataTimePoint{null.FloatFromPtr(nil), null.FloatFrom(2)}) - So(reducer.Reduce(series).Valid, ShouldEqual, false) + require.Equal(t, false, reducer.Reduce(series).Valid) }) + }) - // diff_abs function Test Suite - Convey("diff_abs of one positive point", func() { - result := testReducer("diff_abs", 30) - So(result, ShouldEqual, float64(0)) - }) + t.Run("avg of number values and null values should ignore nulls", func(t *testing.T) { + reducer := newSimpleReducer("avg") + series := plugins.DataTimeSeries{ + Name: "test time series", + } - Convey("diff_abs of one negative point", func() { - result := testReducer("diff_abs", -30) - So(result, ShouldEqual, float64(0)) - }) + series.Points = append(series.Points, plugins.DataTimePoint{null.FloatFrom(3), null.FloatFrom(1)}) + series.Points = append(series.Points, plugins.DataTimePoint{null.FloatFromPtr(nil), null.FloatFrom(2)}) + series.Points = append(series.Points, plugins.DataTimePoint{null.FloatFromPtr(nil), null.FloatFrom(3)}) + series.Points = append(series.Points, plugins.DataTimePoint{null.FloatFrom(3), null.FloatFrom(4)}) - Convey("diff_abs of two positive points[1]", func() { - result := testReducer("diff_abs", 30, 40) - So(result, ShouldEqual, float64(10)) - }) + require.Equal(t, float64(3), reducer.Reduce(series).Float64) + }) - Convey("diff_abs of two positive points[2]", func() { - result := testReducer("diff_abs", 30, 20) - So(result, ShouldEqual, float64(10)) - }) + // diff function Test Suite + t.Run("diff of one positive point", func(t *testing.T) { + result := testReducer("diff", 30) + require.Equal(t, float64(0), result) + }) - Convey("diff_abs of two negative points[1]", func() { - result := testReducer("diff_abs", -30, -40) - So(result, ShouldEqual, float64(10)) - }) + t.Run("diff of one negative point", func(t *testing.T) { + result := testReducer("diff", -30) + require.Equal(t, float64(0), result) + }) - Convey("diff_abs of two negative points[2]", func() { - result := testReducer("diff_abs", -30, -10) - So(result, ShouldEqual, float64(20)) - }) + t.Run("diff of two positive points[1]", func(t *testing.T) { + result := testReducer("diff", 30, 40) + require.Equal(t, float64(10), result) + }) - Convey("diff_abs of one positive and one negative point", func() { - result := testReducer("diff_abs", 30, -40) - So(result, ShouldEqual, float64(70)) - }) + t.Run("diff of two positive points[2]", func(t *testing.T) { + result := testReducer("diff", 30, 20) + require.Equal(t, float64(-10), result) + }) - Convey("diff_abs of one negative and one positive point", func() { - result := testReducer("diff_abs", -30, 40) - So(result, ShouldEqual, float64(70)) - }) + t.Run("diff of two negative points[1]", func(t *testing.T) { + result := testReducer("diff", -30, -40) + require.Equal(t, float64(-10), result) + }) - Convey("diff_abs of three positive points", func() { - result := testReducer("diff_abs", 30, 40, 50) - So(result, ShouldEqual, float64(20)) - }) + t.Run("diff of two negative points[2]", func(t *testing.T) { + result := testReducer("diff", -30, -10) + require.Equal(t, float64(20), result) + }) - Convey("diff_abs of three negative points", func() { - result := testReducer("diff_abs", -30, -40, -50) - So(result, ShouldEqual, float64(20)) - }) + t.Run("diff of one positive and one negative point", func(t *testing.T) { + result := testReducer("diff", 30, -40) + require.Equal(t, float64(-70), result) + }) - Convey("diff_abs with only nulls", func() { - reducer := newSimpleReducer("diff_abs") - series := plugins.DataTimeSeries{ - Name: "test time series", - } + t.Run("diff of one negative and one positive point", func(t *testing.T) { + result := testReducer("diff", -30, 40) + require.Equal(t, float64(70), result) + }) - series.Points = append(series.Points, plugins.DataTimePoint{null.FloatFromPtr(nil), null.FloatFrom(1)}) - series.Points = append(series.Points, plugins.DataTimePoint{null.FloatFromPtr(nil), null.FloatFrom(2)}) + t.Run("diff of three positive points", func(t *testing.T) { + result := testReducer("diff", 30, 40, 50) + require.Equal(t, float64(20), result) + }) - So(reducer.Reduce(series).Valid, ShouldEqual, false) - }) + t.Run("diff of three negative points", func(t *testing.T) { + result := testReducer("diff", -30, -40, -50) + require.Equal(t, float64(-20), result) + }) - // percent_diff function Test Suite - Convey("percent_diff of one positive point", func() { - result := testReducer("percent_diff", 30) - So(result, ShouldEqual, float64(0)) - }) + t.Run("diff with only nulls", func(t *testing.T) { + reducer := newSimpleReducer("diff") + series := plugins.DataTimeSeries{ + Name: "test time series", + } - Convey("percent_diff of one negative point", func() { - result := testReducer("percent_diff", -30) - So(result, ShouldEqual, float64(0)) - }) + series.Points = append(series.Points, plugins.DataTimePoint{null.FloatFromPtr(nil), null.FloatFrom(1)}) + series.Points = append(series.Points, plugins.DataTimePoint{null.FloatFromPtr(nil), null.FloatFrom(2)}) - Convey("percent_diff of two positive points[1]", func() { - result := testReducer("percent_diff", 30, 40) - So(result, ShouldEqual, float64(33.33333333333333)) - }) + require.Equal(t, false, reducer.Reduce(series).Valid) + }) - Convey("percent_diff of two positive points[2]", func() { - result := testReducer("percent_diff", 30, 20) - So(result, ShouldEqual, float64(-33.33333333333333)) - }) + // diff_abs function Test Suite + t.Run("diff_abs of one positive point", func(t *testing.T) { + result := testReducer("diff_abs", 30) + require.Equal(t, float64(0), result) + }) - Convey("percent_diff of two negative points[1]", func() { - result := testReducer("percent_diff", -30, -40) - So(result, ShouldEqual, float64(-33.33333333333333)) - }) + t.Run("diff_abs of one negative point", func(t *testing.T) { + result := testReducer("diff_abs", -30) + require.Equal(t, float64(0), result) + }) - Convey("percent_diff of two negative points[2]", func() { - result := testReducer("percent_diff", -30, -10) - So(result, ShouldEqual, float64(66.66666666666666)) - }) + t.Run("diff_abs of two positive points[1]", func(t *testing.T) { + result := testReducer("diff_abs", 30, 40) + require.Equal(t, float64(10), result) + }) - Convey("percent_diff of one positive and one negative point", func() { - result := testReducer("percent_diff", 30, -40) - So(result, ShouldEqual, float64(-233.33333333333334)) - }) + t.Run("diff_abs of two positive points[2]", func(t *testing.T) { + result := testReducer("diff_abs", 30, 20) + require.Equal(t, float64(10), result) + }) - Convey("percent_diff of one negative and one positive point", func() { - result := testReducer("percent_diff", -30, 40) - So(result, ShouldEqual, float64(233.33333333333334)) - }) + t.Run("diff_abs of two negative points[1]", func(t *testing.T) { + result := testReducer("diff_abs", -30, -40) + require.Equal(t, float64(10), result) + }) - Convey("percent_diff of three positive points", func() { - result := testReducer("percent_diff", 30, 40, 50) - So(result, ShouldEqual, float64(66.66666666666666)) - }) + t.Run("diff_abs of two negative points[2]", func(t *testing.T) { + result := testReducer("diff_abs", -30, -10) + require.Equal(t, float64(20), result) + }) - Convey("percent_diff of three negative points", func() { - result := testReducer("percent_diff", -30, -40, -50) - So(result, ShouldEqual, float64(-66.66666666666666)) - }) + t.Run("diff_abs of one positive and one negative point", func(t *testing.T) { + result := testReducer("diff_abs", 30, -40) + require.Equal(t, float64(70), result) + }) - Convey("percent_diff with only nulls", func() { - reducer := newSimpleReducer("percent_diff") - series := plugins.DataTimeSeries{ - Name: "test time series", - } + t.Run("diff_abs of one negative and one positive point", func(t *testing.T) { + result := testReducer("diff_abs", -30, 40) + require.Equal(t, float64(70), result) + }) - series.Points = append(series.Points, plugins.DataTimePoint{null.FloatFromPtr(nil), null.FloatFrom(1)}) - series.Points = append(series.Points, plugins.DataTimePoint{null.FloatFromPtr(nil), null.FloatFrom(2)}) + t.Run("diff_abs of three positive points", func(t *testing.T) { + result := testReducer("diff_abs", 30, 40, 50) + require.Equal(t, float64(20), result) + }) - So(reducer.Reduce(series).Valid, ShouldEqual, false) - }) + t.Run("diff_abs of three negative points", func(t *testing.T) { + result := testReducer("diff_abs", -30, -40, -50) + require.Equal(t, float64(20), result) + }) - // percent_diff_abs function Test Suite - Convey("percent_diff_abs_abs of one positive point", func() { - result := testReducer("percent_diff_abs", 30) - So(result, ShouldEqual, float64(0)) - }) + t.Run("diff_abs with only nulls", func(t *testing.T) { + reducer := newSimpleReducer("diff_abs") + series := plugins.DataTimeSeries{ + Name: "test time series", + } - Convey("percent_diff_abs of one negative point", func() { - result := testReducer("percent_diff_abs", -30) - So(result, ShouldEqual, float64(0)) - }) + series.Points = append(series.Points, plugins.DataTimePoint{null.FloatFromPtr(nil), null.FloatFrom(1)}) + series.Points = append(series.Points, plugins.DataTimePoint{null.FloatFromPtr(nil), null.FloatFrom(2)}) - Convey("percent_diff_abs of two positive points[1]", func() { - result := testReducer("percent_diff_abs", 30, 40) - So(result, ShouldEqual, float64(33.33333333333333)) - }) + require.Equal(t, false, reducer.Reduce(series).Valid) + }) - Convey("percent_diff_abs of two positive points[2]", func() { - result := testReducer("percent_diff_abs", 30, 20) - So(result, ShouldEqual, float64(33.33333333333333)) - }) + // percent_diff function Test Suite + t.Run("percent_diff of one positive point", func(t *testing.T) { + result := testReducer("percent_diff", 30) + require.Equal(t, float64(0), result) + }) - Convey("percent_diff_abs of two negative points[1]", func() { - result := testReducer("percent_diff_abs", -30, -40) - So(result, ShouldEqual, float64(33.33333333333333)) - }) + t.Run("percent_diff of one negative point", func(t *testing.T) { + result := testReducer("percent_diff", -30) + require.Equal(t, float64(0), result) + }) - Convey("percent_diff_abs of two negative points[2]", func() { - result := testReducer("percent_diff_abs", -30, -10) - So(result, ShouldEqual, float64(66.66666666666666)) - }) + t.Run("percent_diff of two positive points[1]", func(t *testing.T) { + result := testReducer("percent_diff", 30, 40) + require.Equal(t, float64(33.33333333333333), result) + }) - Convey("percent_diff_abs of one positive and one negative point", func() { - result := testReducer("percent_diff_abs", 30, -40) - So(result, ShouldEqual, float64(233.33333333333334)) - }) + t.Run("percent_diff of two positive points[2]", func(t *testing.T) { + result := testReducer("percent_diff", 30, 20) + require.Equal(t, float64(-33.33333333333333), result) + }) - Convey("percent_diff_abs of one negative and one positive point", func() { - result := testReducer("percent_diff_abs", -30, 40) - So(result, ShouldEqual, float64(233.33333333333334)) - }) + t.Run("percent_diff of two negative points[1]", func(t *testing.T) { + result := testReducer("percent_diff", -30, -40) + require.Equal(t, float64(-33.33333333333333), result) + }) - Convey("percent_diff_abs of three positive points", func() { - result := testReducer("percent_diff_abs", 30, 40, 50) - So(result, ShouldEqual, float64(66.66666666666666)) - }) + t.Run("percent_diff of two negative points[2]", func(t *testing.T) { + result := testReducer("percent_diff", -30, -10) + require.Equal(t, float64(66.66666666666666), result) + }) - Convey("percent_diff_abs of three negative points", func() { - result := testReducer("percent_diff_abs", -30, -40, -50) - So(result, ShouldEqual, float64(66.66666666666666)) - }) + t.Run("percent_diff of one positive and one negative point", func(t *testing.T) { + result := testReducer("percent_diff", 30, -40) + require.Equal(t, float64(-233.33333333333334), result) + }) - Convey("percent_diff_abs with only nulls", func() { - reducer := newSimpleReducer("percent_diff_abs") - series := plugins.DataTimeSeries{ - Name: "test time series", - } + t.Run("percent_diff of one negative and one positive point", func(t *testing.T) { + result := testReducer("percent_diff", -30, 40) + require.Equal(t, float64(233.33333333333334), result) + }) - series.Points = append(series.Points, plugins.DataTimePoint{null.FloatFromPtr(nil), null.FloatFrom(1)}) - series.Points = append(series.Points, plugins.DataTimePoint{null.FloatFromPtr(nil), null.FloatFrom(2)}) + t.Run("percent_diff of three positive points", func(t *testing.T) { + result := testReducer("percent_diff", 30, 40, 50) + require.Equal(t, float64(66.66666666666666), result) + }) - So(reducer.Reduce(series).Valid, ShouldEqual, false) - }) + t.Run("percent_diff of three negative points", func(t *testing.T) { + result := testReducer("percent_diff", -30, -40, -50) + require.Equal(t, float64(-66.66666666666666), result) + }) - Convey("min should work with NaNs", func() { - result := testReducer("min", math.NaN(), math.NaN(), math.NaN()) - So(result, ShouldEqual, float64(0)) - }) + t.Run("percent_diff with only nulls", func(t *testing.T) { + reducer := newSimpleReducer("percent_diff") + series := plugins.DataTimeSeries{ + Name: "test time series", + } - Convey("isValid should treat NaN as invalid", func() { - result := isValid(null.FloatFrom(math.NaN())) - So(result, ShouldBeFalse) - }) + series.Points = append(series.Points, plugins.DataTimePoint{null.FloatFromPtr(nil), null.FloatFrom(1)}) + series.Points = append(series.Points, plugins.DataTimePoint{null.FloatFromPtr(nil), null.FloatFrom(2)}) - Convey("isValid should treat invalid null.Float as invalid", func() { - result := isValid(null.FloatFromPtr(nil)) - So(result, ShouldBeFalse) - }) + require.Equal(t, false, reducer.Reduce(series).Valid) + }) + + // percent_diff_abs function Test Suite + t.Run("percent_diff_abs_abs of one positive point", func(t *testing.T) { + result := testReducer("percent_diff_abs", 30) + require.Equal(t, float64(0), result) + }) + + t.Run("percent_diff_abs of one negative point", func(t *testing.T) { + result := testReducer("percent_diff_abs", -30) + require.Equal(t, float64(0), result) + }) + + t.Run("percent_diff_abs of two positive points[1]", func(t *testing.T) { + result := testReducer("percent_diff_abs", 30, 40) + require.Equal(t, float64(33.33333333333333), result) + }) + + t.Run("percent_diff_abs of two positive points[2]", func(t *testing.T) { + result := testReducer("percent_diff_abs", 30, 20) + require.Equal(t, float64(33.33333333333333), result) + }) + + t.Run("percent_diff_abs of two negative points[1]", func(t *testing.T) { + result := testReducer("percent_diff_abs", -30, -40) + require.Equal(t, float64(33.33333333333333), result) + }) + + t.Run("percent_diff_abs of two negative points[2]", func(t *testing.T) { + result := testReducer("percent_diff_abs", -30, -10) + require.Equal(t, float64(66.66666666666666), result) + }) + + t.Run("percent_diff_abs of one positive and one negative point", func(t *testing.T) { + result := testReducer("percent_diff_abs", 30, -40) + require.Equal(t, float64(233.33333333333334), result) + }) + + t.Run("percent_diff_abs of one negative and one positive point", func(t *testing.T) { + result := testReducer("percent_diff_abs", -30, 40) + require.Equal(t, float64(233.33333333333334), result) + }) + + t.Run("percent_diff_abs of three positive points", func(t *testing.T) { + result := testReducer("percent_diff_abs", 30, 40, 50) + require.Equal(t, float64(66.66666666666666), result) + }) + + t.Run("percent_diff_abs of three negative points", func(t *testing.T) { + result := testReducer("percent_diff_abs", -30, -40, -50) + require.Equal(t, float64(66.66666666666666), result) + }) + + t.Run("percent_diff_abs with only nulls", func(t *testing.T) { + reducer := newSimpleReducer("percent_diff_abs") + series := plugins.DataTimeSeries{ + Name: "test time series", + } + + series.Points = append(series.Points, plugins.DataTimePoint{null.FloatFromPtr(nil), null.FloatFrom(1)}) + series.Points = append(series.Points, plugins.DataTimePoint{null.FloatFromPtr(nil), null.FloatFrom(2)}) + + require.Equal(t, false, reducer.Reduce(series).Valid) + }) + + t.Run("min should work with NaNs", func(t *testing.T) { + result := testReducer("min", math.NaN(), math.NaN(), math.NaN()) + require.Equal(t, float64(0), result) + }) + + t.Run("isValid should treat NaN as invalid", func(t *testing.T) { + result := isValid(null.FloatFrom(math.NaN())) + require.False(t, result) + }) + + t.Run("isValid should treat invalid null.Float as invalid", func(t *testing.T) { + result := isValid(null.FloatFromPtr(nil)) + require.False(t, result) }) } From fadf72dd3400ae8d8dc2a848d3574940b9694844 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hugo=20H=C3=A4ggmark?= Date: Wed, 27 Oct 2021 07:11:24 +0200 Subject: [PATCH 48/49] DashboardLinks: Fix time in links not being updated (#40934) --- packages/grafana-ui/src/utils/useForceUpdate.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/grafana-ui/src/utils/useForceUpdate.ts b/packages/grafana-ui/src/utils/useForceUpdate.ts index e1e17230f44..6f9a08b8b9d 100644 --- a/packages/grafana-ui/src/utils/useForceUpdate.ts +++ b/packages/grafana-ui/src/utils/useForceUpdate.ts @@ -2,6 +2,6 @@ import { useState } from 'react'; /** @internal */ export function useForceUpdate() { - const [value, setValue] = useState(0); // integer state - return () => setValue(value + 1); // update the state to force render + const [_, setValue] = useState(0); // integer state + return () => setValue((prevState) => prevState + 1); // update the state to force render } From 858d654d1c2191aedb46a7b1e1deee4179a2e9fb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Torkel=20=C3=96degaard?= Date: Wed, 27 Oct 2021 09:42:47 +0200 Subject: [PATCH 49/49] Barchart: Fixes barchart switching from palette to thresholds color mode (#40954) --- public/app/plugins/panel/barchart/module.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/public/app/plugins/panel/barchart/module.tsx b/public/app/plugins/panel/barchart/module.tsx index 2981d5cc630..b36bd54d19a 100755 --- a/public/app/plugins/panel/barchart/module.tsx +++ b/public/app/plugins/panel/barchart/module.tsx @@ -19,6 +19,7 @@ export const plugin = new PanelPlugin(BarC [FieldConfigProperty.Color]: { settings: { byValueSupport: true, + preferThresholdsMode: false, }, defaultValue: { mode: FieldColorModeId.PaletteClassic,