CloudWatch Logs: Support fetching fields in monaco editor #78244
Conversation
| @@ -39,6 +39,9 @@ export interface CodeEditorProps { | |||
| */ | |||
| onEditorDidMount?: (editor: MonacoEditor, monaco: Monaco) => void; | |||
|
|
|||
| /** Callback before the edior has unmounted */ | |||
| onEditorWillUnmount?: () => void; | |||
There was a problem hiding this comment.
I needed to be able to call dispose on the CompletionItemProvider on unmount (otherwise we end up with two set on the language when another CodeEditor is opened). There may be a better way to do this, but this seemed cleanest to me.
| @@ -56,6 +71,7 @@ export class LogsCompletionItemProvider extends CompletionItemProvider { | |||
| insertText: `${command} $0`, | |||
| insertTextRules: monaco.languages.CompletionItemInsertTextRule.InsertAsSnippet, | |||
| command: TRIGGER_SUGGEST, | |||
| kind: monaco.languages.CompletionItemKind.Method, | |||
There was a problem hiding this comment.
Changing these was mostly to make it so that they weren't all using the Field color coding. I can change them back if we want.
iwysiu
requested review from
fridgepoet,
idastambuk,
joshhunt and
JoaoSilvaGrafana
and removed request for
a team
| @@ -27,6 +27,10 @@ class UnthemedCodeEditor extends PureComponent<Props> { | |||
| if (this.completionCancel) { | |||
| this.completionCancel.dispose(); | |||
| } | |||
|
|
|||
| if (this.props.onEditorWillUnmount) { | |||
There was a problem hiding this comment.
You can also use a shorthand here: this.props.onEditorWillUnmount?.();
|
Looks good, great job with this! I played around with it and the only "bug" I noticed is when we have two or more logs editors open in one panel. In this case the field suggestions are the same for both editors, even if log groups are different (or missing in one of them, for example). It seems like multiple editors in the same window use a singleton instance of CompletionProvider that wouldn't be so simple to separate between each editor. I guess that means that this was a problem even with the old editor, maybe? Meaning it wouldn't be a showstopper for merging this. |
Unfortunately it probably wasn't. The old editor was able to send the query fields along with the actual request for suggestions, unlike monaco which needs them to be set on the completion item provider, so it probably wouldn't have this issue. Hmm, how do we work around this 😞 |
|
@idastambuk Fixed the issue with multiple queries! Now it reregisters on |
Tested it and looks good now! |
What is this feature?
Updates the monaco completion provider for cloudwatch logs to also get field suggestions.
Editor now fetching fields:
Why do we need this feature?
It makes it easier to write cloudwatch logs queries. Also, the old editor had this feature so we we should have it for the monaco editor.
Who is this feature for?
Users who want to write cloudwatch logs queries in grafana.
Which issue(s) does this PR fix?:
Fixes #77223
Special notes for your reviewer:
The conditions I thought of for testing were:
I didn't write tests for the new
LogsQueryFieldlogic because I wasn't sure how to, but if y'all have a suggestion I'm open to it.Please check that: