public/grafana
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Dashboard: Schema V2 - Fix annotations losing queries (#102951)

Browse Source 
* Fix annotations losing queries when saving v1 with dynamic dashboards enabled

* Add extra options property to the DashboardAnnotationQuerySpec to catch all field datasource-specific properties

* Add options to the v2Alpha1 schema and run make gen-apps

* Add unit test and clean up console.logs

Co-authored-by: Ivan Ortega Alba <ivanortegaalba@gmail.com>
This commit is contained in:
Alexa V
2025-04-09 18:51:51 +02:00
committed by GitHub
co-authored by Ivan Ortega Alba
parent d9dc93c4a6
commit 2f60d54648
13 changed files with 576 additions and 12 deletions
Download Patch File Download Diff File Expand all files Collapse all files
apps/dashboard/kinds/v2alpha1/dashboard_spec.cue
+1
View File
@@ -394,6 +394,7 @@ AnnotationQuerySpec: {
name: string
builtIn?: bool | *false
filter?: AnnotationPanelFilter
options?: [string]: _ //Catch-all field for datasource-specific properties
}
AnnotationQueryKind: {
apps/dashboard/pkg/apis/dashboard/v2alpha1/dashboard_spec_gen.go
+2
View File
@@ -32,6 +32,8 @@ type DashboardAnnotationQuerySpec struct {
Name string `json:"name"`
BuiltIn *bool `json:"builtIn,omitempty"`
Filter *DashboardAnnotationPanelFilter `json:"filter,omitempty"`
// Catch-all field for datasource-specific properties
Options map[string]interface{} `json:"options,omitempty"`
}
// NewDashboardAnnotationQuerySpec creates a new DashboardAnnotationQuerySpec object.
apps/dashboard/pkg/apis/dashboard/v2alpha1/zz_generated.openapi.go
+15
View File
@@ -633,6 +633,21 @@ func schema_pkg_apis_dashboard_v2alpha1_DashboardAnnotationQuerySpec(ref common.
Ref: ref("github.com/grafana/grafana/apps/dashboard/pkg/apis/dashboard/v2alpha1.DashboardAnnotationPanelFilter"),
},
},
"options": {
SchemaProps: spec.SchemaProps{
Description: "Catch-all field for datasource-specific properties",
Type: []string{"object"},
AdditionalProperties: &spec.SchemaOrBool{
Allows: true,
Schema: &spec.Schema{
SchemaProps: spec.SchemaProps{
Type: []string{"object"},
Format: "",
},
},
},
},
},
},
Required: []string{"enable", "hide", "iconColor", "name"},
},
packages/grafana-schema/src/schema/dashboard/v2alpha0/dashboard.schema.cue
+2 -1
View File
@@ -236,7 +236,7 @@ DynamicConfigValue: {
}
// Matcher is a predicate configuration. Based on the config a set of field(s) or values is filtered in order to apply override / transformation.
// It comes with in id ( to resolve implementation from registry) and a configuration that’s specific to a particular matcher type.
// It comes with in id ( to resolve implementation from registry) and a configuration that's specific to a particular matcher type.
MatcherConfig: {
// The matcher id. This is used to find the matcher implementation from registry.
id: string | *""
@@ -394,6 +394,7 @@ AnnotationQuerySpec: {
name: string
builtIn?: bool | *false
filter?: AnnotationPanelFilter
options?: [string]: _ // Catch-all field for datasource-specific properties
}
AnnotationQueryKind: {
packages/grafana-schema/src/schema/dashboard/v2alpha0/types.gen.ts
+3 -1
View File
@@ -70,6 +70,8 @@ export interface AnnotationQuerySpec {
name: string;
builtIn?: boolean;
filter?: AnnotationPanelFilter;
// Catch-all field for datasource-specific properties
options?: Record<string, any>;
}
export const defaultAnnotationQuerySpec = (): AnnotationQuerySpec => ({
@@ -245,7 +247,7 @@ export const defaultDataTransformerConfig = (): DataTransformerConfig => ({
});
// Matcher is a predicate configuration. Based on the config a set of field(s) or values is filtered in order to apply override / transformation.
// It comes with in id ( to resolve implementation from registry) and a configuration that’s specific to a particular matcher type.
// It comes with in id ( to resolve implementation from registry) and a configuration that's specific to a particular matcher type.
export interface MatcherConfig {
// The matcher id. This is used to find the matcher implementation from registry.
id: string;
packages/grafana-schema/src/schema/dashboard/v2alpha1/types.spec.gen.ts
+2
View File
@@ -19,6 +19,8 @@ export interface AnnotationQuerySpec {
name: string;
builtIn?: boolean;
filter?: AnnotationPanelFilter;
// Catch-all field for datasource-specific properties
options?: Record<string, any>;
}
export const defaultAnnotationQuerySpec = (): AnnotationQuerySpec => ({
pkg/tests/apis/openapi_snapshots/dashboard.grafana.app-v2alpha1.json
+7
View File
@@ -1273,6 +1273,13 @@
"type": "string",
"default": ""
},
"options": {
"description": "Catch-all field for datasource-specific properties",
"type": "object",
"additionalProperties": {
"type": "object"
}
},
"query": {
"$ref": "#/components/schemas/com.github.grafana.grafana.apps.dashboard.pkg.apis.dashboard.v2alpha1.DashboardDataQueryKind"
}
public/app/features/annotations/components/StandardAnnotationQueryEditor.test.tsx
+280
View File
@@ -49,6 +49,7 @@ describe('StandardAnnotationQueryEditor', () => {
expect.anything()
);
});
it('should keep and pass the initial query if the defaultQuery is not defined', () => {
const { props } = setup({
annotation: { name: 'initialAnn', target: { refId: 'initialAnnotationRef' } } as AnnotationQuery,
@@ -66,4 +67,283 @@ describe('StandardAnnotationQueryEditor', () => {
expect.anything()
);
});
it('v2 dashboard - should preserve options field when changing target', () => {
// Setup with annotation that has options
const mockOnChange = jest.fn();
const { props } = setup({
annotation: {
name: 'annotationWithOptions',
target: { refId: 'refId1' },
options: {
expr: 'rate(http_requests_total[5m])',
queryType: 'range',
},
enable: true,
iconColor: 'red',
hide: false,
} as unknown as AnnotationQuery,
onChange: mockOnChange,
datasource: {
annotations: {
QueryEditor: jest.fn(() => <div>Editor</div>),
prepareAnnotation: (annotation: AnnotationQuery) => annotation,
},
} as unknown as DataSourceApi,
});
// Get the onQueryChange function from the component instance
const componentInstance = (props.datasource.annotations?.QueryEditor as jest.Mock).mock.calls[0][0];
// Simulate changing the target
componentInstance.onChange({ refId: 'refId2', newField: 'value' });
// Check that options are preserved
expect(mockOnChange).toHaveBeenCalledWith(
expect.objectContaining({
target: { refId: 'refId2', newField: 'value' },
options: {
expr: 'rate(http_requests_total[5m])',
queryType: 'range',
},
})
);
});
it('should preserve options field when using onAnnotationChange', () => {
// Setup with annotation that has options
const mockOnChange = jest.fn();
const { props } = setup({
annotation: {
name: 'annotationWithOptions',
options: {
expr: 'rate(http_requests_total[5m])',
queryType: 'range',
},
enable: true,
iconColor: 'blue',
hide: false,
} as unknown as AnnotationQuery,
onChange: mockOnChange,
datasource: {
annotations: {
QueryEditor: jest.fn(() => <div>Editor</div>),
prepareAnnotation: (annotation: AnnotationQuery) => annotation,
},
} as unknown as DataSourceApi,
});
// Get the onAnnotationChange function from the component instance
const componentInstance = (props.datasource.annotations?.QueryEditor as jest.Mock).mock.calls[0][0];
// Simulate annotation change from child component
componentInstance.onAnnotationChange({
name: 'newName',
iconColor: 'red',
});
// Check that options are preserved
expect(mockOnChange).toHaveBeenCalledWith(
expect.objectContaining({
name: 'newName',
iconColor: 'red',
options: {
expr: 'rate(http_requests_total[5m])',
queryType: 'range',
},
})
);
});
it('should handle v2 dashboard annotations with query.spec', () => {
const { props } = setup({
annotation: {
name: 'v2annotation',
query: {
kind: 'prometheus',
spec: {
expr: 'rate(http_requests_total[5m])',
refId: 'A',
},
},
} as unknown as AnnotationQuery,
datasource: {
annotations: {
QueryEditor: jest.fn(() => <div>Editor</div>),
prepareAnnotation: (annotation: AnnotationQuery) => annotation,
},
} as unknown as DataSourceApi,
});
// Check that query.spec is used as target for QueryEditor
expect(props.datasource?.annotations?.QueryEditor).toHaveBeenCalledWith(
expect.objectContaining({
query: expect.objectContaining({
expr: 'rate(http_requests_total[5m])',
refId: 'A',
}),
}),
expect.anything()
);
});
it('should propagate options to root level for v2 dashboards', () => {
const { props } = setup({
annotation: {
name: 'v2annotationWithOptions',
query: {
kind: 'prometheus',
spec: {
refId: 'A',
},
},
options: {
expr: 'rate(http_requests_total[5m])',
legendFormat: '{{method}} {{endpoint}}',
},
} as unknown as AnnotationQuery,
datasource: {
annotations: {
QueryEditor: jest.fn(() => <div>Editor</div>),
prepareAnnotation: (annotation: AnnotationQuery) => annotation,
},
} as unknown as DataSourceApi,
});
// Check that options are propagated to root level for the editor
expect(props.datasource?.annotations?.QueryEditor).toHaveBeenCalledWith(
expect.objectContaining({
annotation: expect.objectContaining({
name: 'v2annotationWithOptions',
query: expect.anything(),
options: expect.anything(),
expr: 'rate(http_requests_total[5m])',
legendFormat: '{{method}} {{endpoint}}',
}),
}),
expect.anything()
);
});
it('should handle v1 dashboard with a prop name query but that is not a v2 spec', () => {
const { props } = setup({
annotation: {
name: 'v1WithQueryNoSpec',
target: { refId: 'AnnoTarget' },
query: 'abcdefg', // v1 dashboard might have a prop called query, but is not v2, it does not have spec
datasource: {
type: 'prometheus',
uid: 'abc123',
},
// v1 dashboards don't have options field
enable: true,
iconColor: 'red',
hide: false,
} as unknown as AnnotationQuery,
datasource: {
annotations: {
QueryEditor: jest.fn(() => <div>Editor</div>),
prepareAnnotation: (annotation: AnnotationQuery) => annotation,
},
} as unknown as DataSourceApi,
});
// The QueryEditor is called with the annotation object, we should check that
// it contains the correct annotation object with our query
expect(props.datasource?.annotations?.QueryEditor).toHaveBeenCalledWith(
expect.objectContaining({
annotation: expect.objectContaining({
name: 'v1WithQueryNoSpec',
target: { refId: 'AnnoTarget' },
// v1 dashboard might have a prop called query, but is not v2, it does not have spec
query: 'abcdefg',
}),
// The query will be the target object after processing
query: expect.objectContaining({
refId: 'AnnoTarget',
}),
}),
expect.anything()
);
});
it('should work with v1 dashboards that have target field', () => {
const { props } = setup({
annotation: {
name: 'v1WithTarget',
target: {
refId: 'A',
expr: 'up',
},
datasource: {
type: 'prometheus',
uid: 'abc123',
},
enable: true,
iconColor: 'green',
hide: false,
} as unknown as AnnotationQuery,
datasource: {
annotations: {
QueryEditor: jest.fn(() => <div>Editor</div>),
prepareAnnotation: (annotation: AnnotationQuery) => annotation,
},
} as unknown as DataSourceApi,
});
// Should use existing target for v1 dashboards
expect(props.datasource?.annotations?.QueryEditor).toHaveBeenCalledWith(
expect.objectContaining({
query: expect.objectContaining({
refId: 'A',
expr: 'up',
}),
}),
expect.anything()
);
});
it('should handle both onQueryChange and onChange handlers from datasource plugins', () => {
const mockOnChange = jest.fn();
const { props } = setup({
annotation: {
name: 'annotation',
target: { refId: 'A' },
iconColor: 'yellow',
hide: false,
} as unknown as AnnotationQuery,
onChange: mockOnChange,
datasource: {
annotations: {
QueryEditor: jest.fn(() => <div>Editor</div>),
prepareAnnotation: (annotation: AnnotationQuery) => annotation,
},
} as unknown as DataSourceApi,
});
const componentInstance = (props.datasource.annotations?.QueryEditor as jest.Mock).mock.calls[0][0];
// Test onQueryChange handler
componentInstance.onChange({ refId: 'B', expr: 'new_expr' });
expect(mockOnChange).toHaveBeenCalledWith(
expect.objectContaining({
target: { refId: 'B', expr: 'new_expr' },
})
);
mockOnChange.mockClear();
// Test onAnnotationChange handler
componentInstance.onAnnotationChange({
name: 'updated',
iconColor: 'blue',
enable: false,
});
expect(mockOnChange).toHaveBeenCalledWith(
expect.objectContaining({
name: 'updated',
iconColor: 'blue',
enable: false,
})
);
});
});
public/app/features/annotations/components/StandardAnnotationQueryEditor.tsx
+30 -4
View File
@@ -103,6 +103,8 @@ export default class StandardAnnotationQueryEditor extends PureComponent<Props,
this.props.onChange({
...this.props.annotation,
target,
// Keep options from the original annotation if they exist
...(this.props.annotation.options ? { options: this.props.annotation.options } : {}),
});
};
@@ -206,7 +208,12 @@ export default class StandardAnnotationQueryEditor extends PureComponent<Props,
}
onAnnotationChange = (annotation: AnnotationQuery) => {
this.props.onChange(annotation);
// Also preserve any options field that might exist when migrating from V2 to V1
this.props.onChange({
...annotation,
// Keep options from the original annotation if they exist
...(this.props.annotation.options ? { options: this.props.annotation.options } : {}),
});
};
render() {
@@ -225,11 +232,30 @@ export default class StandardAnnotationQueryEditor extends PureComponent<Props,
);
}
const query = {
// For v2 dashboards, target is not available, only query
let target = annotation.target;
// For v2 dashboards, use query.spec
if (annotation.query && annotation.query.spec) {
target = {
...annotation.query.spec,
};
}
let query = {
...datasource.annotations?.getDefaultQuery?.(),
...(annotation.target ?? { refId: 'Anno' }),
...(target ?? { refId: 'Anno' }),
};
// Create annotation object that respects annotations API
let editorAnnotation = annotation;
// For v2 dashboards: propagate options to root level for datasource compatibility
if (annotation.query && annotation.options) {
editorAnnotation = { ...annotation };
Object.assign(editorAnnotation, annotation.options);
}
return (
<>
<DataSourcePluginContextProvider instanceSettings={datasourceInstanceSettings}>
@@ -241,7 +267,7 @@ export default class StandardAnnotationQueryEditor extends PureComponent<Props,
onRunQuery={this.onRunQuery}
data={response?.panelData}
range={getTimeSrv().timeRange()}
annotation={annotation}
annotation={editorAnnotation}
onAnnotationChange={this.onAnnotationChange}
/>
</DataSourcePluginContextProvider>
public/app/features/dashboard-scene/serialization/transformSaveModelSchemaV2ToScene.test.ts
+116
View File
@@ -33,6 +33,7 @@ import {
import { DashboardWithAccessInfo } from 'app/features/dashboard/api/types';
import { MIXED_DATASOURCE_NAME } from 'app/plugins/datasource/mixed/MixedDataSource';
import { DashboardAnnotationsDataLayer } from '../scene/DashboardAnnotationsDataLayer';
import { DashboardDataLayerSet } from '../scene/DashboardDataLayerSet';
import { DefaultGridLayoutManager } from '../scene/layout-default/DefaultGridLayoutManager';
import { AutoGridItem } from '../scene/layout-responsive-grid/ResponsiveGridItem';
@@ -665,4 +666,119 @@ describe('transformSaveModelSchemaV2ToScene', () => {
});
});
});
describe('annotations', () => {
it('should transform annotation with options field', () => {
// Create a dashboard with an annotation that has options
const dashboardWithAnnotationOptions: DashboardWithAccessInfo<DashboardV2Spec> = {
kind: 'DashboardWithAccessInfo',
apiVersion: 'v2alpha1',
metadata: {
name: 'test-dashboard',
namespace: 'default',
creationTimestamp: new Date().toISOString(),
labels: {},
annotations: {},
generation: 1,
resourceVersion: '1',
},
spec: {
title: 'Dashboard with annotation options',
editable: true,
preload: false,
liveNow: false,
cursorSync: 'Off',
links: [],
tags: [],
timeSettings: {
from: 'now-6h',
to: 'now',
timezone: 'browser',
hideTimepicker: false,
autoRefresh: '5s',
autoRefreshIntervals: ['5s', '10s', '30s'],
fiscalYearStartMonth: 0,
weekStart: 'monday',
},
variables: [],
elements: {},
layout: {
kind: 'GridLayout',
spec: { items: [] },
},
annotations: [
{
kind: 'AnnotationQuery',
spec: {
name: 'Annotation with options',
builtIn: false,
enable: true,
hide: false,
iconColor: 'purple',
datasource: {
type: 'prometheus',
uid: 'abc123',
},
options: {
expr: 'rate(http_requests_total[5m])',
queryType: 'range',
legendFormat: '{{method}} {{endpoint}}',
useValueAsTime: true,
step: '1m',
},
},
},
],
},
access: {
canSave: true,
canEdit: true,
canDelete: true,
canAdmin: true,
canStar: true,
canShare: true,
annotationsPermissions: {
dashboard: {
canAdd: true,
canEdit: true,
canDelete: true,
},
organization: {
canAdd: true,
canEdit: true,
canDelete: true,
},
},
},
};
const scene = transformSaveModelSchemaV2ToScene(dashboardWithAnnotationOptions);
// Get the annotation layers
const dataLayerSet = scene.state.$data as DashboardDataLayerSet;
expect(dataLayerSet).toBeDefined();
expect(dataLayerSet.state.annotationLayers.length).toBe(1);
const annotationLayer = dataLayerSet.state.annotationLayers[0] as DashboardAnnotationsDataLayer;
// Verify that the options have been merged into the query object
expect(annotationLayer.state.query).toMatchObject({
name: 'Annotation with options',
expr: 'rate(http_requests_total[5m])',
queryType: 'range',
legendFormat: '{{method}} {{endpoint}}',
useValueAsTime: true,
step: '1m',
});
// Verify the original options object is also preserved
expect(annotationLayer.state.query.options).toMatchObject({
expr: 'rate(http_requests_total[5m])',
queryType: 'range',
legendFormat: '{{method}} {{endpoint}}',
useValueAsTime: true,
step: '1m',
});
});
});
});
public/app/features/dashboard-scene/serialization/transformSaveModelSchemaV2ToScene.ts
+10 -1
View File
@@ -88,10 +88,19 @@ export function transformSaveModelSchemaV2ToScene(dto: DashboardWithAccessInfo<D
const { spec: dashboard, metadata } = dto;
const annotationLayers = dashboard.annotations.map((annotation) => {
let annoQuerySpec = annotation.spec;
// some annotations will contain in the options properties that need to be
// added to the root level annotation spec
if (annoQuerySpec?.options) {
annoQuerySpec = {
...annoQuerySpec,
...annoQuerySpec.options,
};
}
return new DashboardAnnotationsDataLayer({
key: uniqueId('annotations-'),
query: {
...annotation.spec,
...annoQuerySpec,
builtIn: annotation.spec.builtIn ? 1 : 0,
},
name: annotation.spec.name,
public/app/features/dashboard-scene/serialization/transformSceneToSaveModelSchemaV2.test.ts
+60
View File
@@ -559,6 +559,66 @@ describe('transformSceneToSaveModelSchemaV2', () => {
expect(queryRunner.state.datasource?.type).toBeUndefined(); // No queryRunner datasource
});
});
it('should test annotation with options field', () => {
// Create a scene with an annotation layer that has options
const annotationWithOptions = new DashboardAnnotationsDataLayer({
key: 'layerWithOptions',
query: {
datasource: {
type: 'prometheus',
uid: 'abc123',
},
name: 'annotation-with-options',
enable: true,
iconColor: 'red',
options: {
expr: 'rate(http_requests_total[5m])',
queryType: 'range',
legendFormat: '{{method}} {{endpoint}}',
useValueAsTime: true,
},
// Some other properties that aren't in the annotation spec
// and should be moved to options
customProp1: 'value1',
customProp2: 'value2',
},
name: 'layerWithOptions',
isEnabled: true,
isHidden: false,
});
const scene = setupDashboardScene({
$data: new DashboardDataLayerSet({
annotationLayers: [annotationWithOptions],
}),
body: new DefaultGridLayoutManager({
grid: new SceneGridLayout({ children: [] }),
}),
});
const result = transformSceneToSaveModelSchemaV2(scene);
// Verify the annotation options are properly serialized
expect(result.annotations.length).toBe(1);
expect(result.annotations[0].spec.options).toBeDefined();
expect(result.annotations[0].spec.options).toEqual({
expr: 'rate(http_requests_total[5m])',
queryType: 'range',
legendFormat: '{{method}} {{endpoint}}',
useValueAsTime: true,
customProp1: 'value1',
customProp2: 'value2',
});
// Ensure these properties are not at the root level
expect(result).not.toHaveProperty('annotations[0].spec.expr');
expect(result).not.toHaveProperty('annotations[0].spec.queryType');
expect(result).not.toHaveProperty('annotations[0].spec.legendFormat');
expect(result).not.toHaveProperty('annotations[0].spec.useValueAsTime');
expect(result).not.toHaveProperty('annotations[0].spec.customProp1');
expect(result).not.toHaveProperty('annotations[0].spec.customProp2');
});
});
describe('getElementDatasource', () => {
public/app/features/dashboard-scene/serialization/transformSceneToSaveModelSchemaV2.ts
+48 -5
View File
@@ -414,14 +414,57 @@ function getAnnotations(state: DashboardSceneState): AnnotationQueryKind[] {
},
};
// Check if DataQueryKind exists
const queryKind = getAnnotationQueryKind(layer.state.query);
if (layer.state.query.query?.kind === queryKind) {
// Transform v1 dashboard (using target) to v2 structure
if (layer.state.query.target) {
// Handle built-in annotations
if (layer.state.query.builtIn) {
result.spec.query = {
kind: 'grafana', // built-in annotations are always of type grafana
spec: {
...layer.state.query.target,
},
};
} else {
result.spec.query = {
kind: getAnnotationQueryKind(layer.state.query),
spec: {
...layer.state.query.target,
},
};
}
}
// For annotations without query.query defined (e.g., grafana annotations without tags)
else if (layer.state.query.query?.kind) {
result.spec.query = {
kind: queryKind,
spec: layer.state.query.query.spec,
kind: layer.state.query.query.kind,
spec: {
...layer.state.query.query.spec,
},
};
}
// Collect datasource-specific properties not in standard annotation spec
let otherProps = omit(
layer.state.query,
'type',
'target',
'builtIn',
'name',
'datasource',
'iconColor',
'enable',
'hide',
'filter',
'query'
);
// Store extra properties in the options field instead of directly in the spec
if (Object.keys(otherProps).length > 0) {
// Extract options property and get the rest of the properties
const { options, ...restProps } = otherProps;
// Merge options with the rest of the properties
result.spec.options = { ...options, ...restProps };
}
// If filter is an empty array, don't save it
if (layer.state.query.filter?.ids?.length) {