Elasticsearch: Handle multiple annotation structures

[gabor]

since the elastic annotation handling was migrated from Angular to react in #49529 , we have a bug in how they are handled. this PR fixes the problem.

in Grafana versions up to 9.0.9 the annotation editor for elastic was angular based, and this is how it worked:
you entered a lucene query, and it stored in the annotation-structure like this (there are other fields there too, but they are not important here, i'm ignoring them):

{
    "query":"level:error"
}

this is stored in the dashboard-json, and then the datasource's annotationQuery method reads it at:

grafana/public/app/plugins/datasource/elasticsearch/datasource.ts

Line 246 in 720fe9d

const queryString = annotation.query;

NOTE: an unrelated thing that will be important here is: we are doing custom annotation-handling here. Grafana is able to do a generic annotation-handling-thing (for other datasources), and in such a case it creates the JSON like this:

{
    "target": {
        "query": "level:error"
    }
}

(it basically stores a whole Query-object at annotation.target.

with Grafana 9.1.0, we switched for elastic-annotations to React, and wrote custom code to handle this. we tried to keep the JSON shape the same, so we still write to the top-level "query" field:

grafana/public/app/plugins/datasource/elasticsearch/components/QueryEditor/AnnotationQueryEditor.tsx

Lines 26 to 29 in b33ce06

	onAnnotationChange({
	...annotation,
	query,
	});

this worked well, but there's a complication. with this being in React, some other generic Grafana code gets activated, that tries to "migrat" from the OLD toplevel-query-format to the NEW inside-target-format:

grafana/public/app/features/annotations/standardAnnotationSupport.ts

Lines 24 to 37 in b33ce06

	prepareAnnotation: (json: any) => {
	if (isString(json?.query)) {
	const { query, ...rest } = json;
	return {
	...rest,
	target: {
	refId: 'annotation_query',
	query,
	},
	mappings: {},
	};
	}
	return json as AnnotationQuery;
	},

this always happens whenever an annotation-query is opened for editing. and when this happens, the shape of our JSON changes, query becomes target.query. and when this happens, the datasource.annotationQuery function is not able to do it's work, because it expect the value at query, not at target.query.

also, note that the elastic AnnotationQueryEditor, when editing an existing annotation, READS from target.query, but WRITES to query:

grafana/public/app/plugins/datasource/elasticsearch/components/QueryEditor/AnnotationQueryEditor.tsx

Lines 24 to 30 in b33ce06

	value={annotation.target?.query}
	onChange={(query) => {
	onAnnotationChange({
	...annotation,
	query,
	});
	}}

(this works because when an annotation is opened for editing, that generic grafana migration code happens, and the annotation received by the react component is already in the NEW format)

but this causes problems too. imagine that we have this JSON in dashboard-json, that we open for editing:

{
    "query": "level:error"
}

now, in the react annotation-editor we will see level:error in the text-field. we change it to level:info, and save the JSON.
but, the JSON stored to dashboard-json will be this:

{
    "query": "level:info",
   "target": {
        "query":"level:error"
   }
}

(the automatic grafana migration thing happens when the form is opened for editing, not when the content is saved at the end)

so, to handle all this, this pull request does two changes:

1. we change datasource.annotationQuery to look for both places (query for old annotations, and target.query for new annotations).. when both exist, we take the old-format one, because of the bug i described just above.. i think it's better to prefer the old value in this scenario.
2. we change the elastic AnnotationQueryEditor to write to target.query

this should cover all the possible cases.

how to test:
(technically we should start with a locally built grafana 9.0.9, but i was not able to build it locally anymore, it complained about some yarn-thing, so i used grafana-in-docker instead)

1. use grafana 9.0.9 with something like make devenv=grafana grafana_version=9.0.9
2. create an annotation. if you are using devenv-elastic, an easy-to-use one is:
   • query: counter:42.
   • set the "text" field to "counter"
3. verify that you see a single line in the dashboard panel, and when you mouseover to the value of the annotation, it says 42.
4. verify that the structure of the json is top-level-query, not in-target-query
5. copy the whole dashboard-json out into a file on your computer.
6. now switch to this PR on your computer
7. we need to import the dashboard into your computer, but the UID of the elastic-datasource might be different, so do this:
   • find out the UID of your elastic datasource on your computer. for example, go to edit-datasource-config, and the UID is the last part of the URL.
   • choose import dashboard JSON, and afterwards modify the JSON by correcting the datasource-UID
   • at this point the dashboard should work correctly.
8. refresh the dashboard. verify that you see a single line in the dashboard panel. (so that new code works well with old-format)
9. switch to grafana main-branch
10. edit the dashboard-annotation, set the lucene query to counter:43
11. save the dashboard
12. check the dashboard-json, verify that it has a top-level counter:43 and an inside-target counter:42.
13. switch to this PR
14. refresh the dashboard
15. verify that you see a single line in the dashboard panel, and when you mouseover to the value of the annotation, it says 43. (so that the new code shows .querywhen both .query and .target.query exists.
16. edit the annotation, set the lucene query to counter:44
17. save the dashboard
18. verify in the dashboard-json that .target.query exists with counter:44, and no .query exists.
19. refresh the dashboard
20. verify that you see a single line in the dashboard panel, and when you mouseover to the value of the annotation, it says 44. (so that the new code is able to read .target.query too)
21. delete the existing annotation, save dashboard, add a new annotation, for example set the query to counter:45. save dashboard, and verify in the JSON that .target.query is set to counter:45, and .query does not exist. refresh dashboard, verify the annotation says 45 (so a newly created annotation works too)
22. done 😄

(fixes #61107)

[github-actions]

Backend code coverage report for PR #66762
No changes

[github-actions]

Frontend code coverage report for PR #66762

Plugin	Main	PR	Difference
elasticsearch	78.5%	78.43%	-.07%

[gabor]

using this refId-value is inspired by

grafana/public/app/features/annotations/standardAnnotationSupport.ts

Line 32 in 98c695c

refId: 'annotation_query',

[grafanabot]

Hello @gabor!
Backport pull requests need to be either:

• Pull requests which address bugs,
• Urgent fixes which need product approval, in order to get merged,
• Docs changes.

Please, if the current pull request addresses a bug fix, label it with the type/bug label.
If it already has the product approval, please add the product-approved label. For docs changes, please add the type/docs label.
If the pull request modifies CI behaviour, please add the type/ci label.
If none of the above applies, please consider removing the backport label and target the next major/minor release.
Thanks!

[ivanahuckova]

LGTM! Thank you for the extensive how to test this. Helped a ltop with making sure it works as expeted.