New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
CloudWatch: Add account support to variable queries #63822
Conversation
Backend code coverage report for PR #63822 |
Frontend code coverage report for PR #63822
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice!
What behavior do you expect when I choose a Region in which I have no accounts? I'm just wondering because when I test it out I see the Account field just disappear and am not sure if it should be active but with an empty dropdown, for example.
public/app/plugins/datasource/cloudwatch/components/VariableQueryEditor/VariableQueryEditor.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch on the region thing! Updated the resource api. Also thanks for the screenshot
public/app/plugins/datasource/cloudwatch/components/VariableQueryEditor/VariableQueryEditor.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, but I was testing the variable editor and when I select 'All' in the dropdown, nothing happens when I click the Run query button, so I dont see the change in query results as I expected. Is this intended behaviour?
public/app/plugins/datasource/cloudwatch/components/VariableQueryEditor/VariableQueryEditor.tsx
Show resolved
Hide resolved
label="Account" | ||
value={query.accountId ?? null} | ||
onChange={(accountId?: string) => onQueryChange({ ...parsedQuery, accountId })} | ||
options={accountState?.value.length ? [ALL_ACCOUNTS_OPTION, ...accountState?.value] : []} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
options={accountState?.value.length ? [ALL_ACCOUNTS_OPTION, ...accountState?.value] : []} | |
options={[ALL_ACCOUNTS_OPTION, ...accountState?.value]} |
There's a condition above that checks for accountState.value.length, I dont think we need it here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, but I was testing the variable editor and when I select 'All' in the dropdown, nothing happens when I click the Run query button, so I dont see the change in query results as I expected. Is this intended behaviour?
The form auto-reruns the query, so if you select a log group query, look at the results, then select all
for the account it should change
public/app/plugins/datasource/cloudwatch/components/VariableQueryEditor/VariableQueryEditor.tsx
Show resolved
Hide resolved
label="Account" | ||
value={query.accountId ?? null} | ||
onChange={(accountId?: string) => onQueryChange({ ...parsedQuery, accountId })} | ||
options={accountState?.value.length ? [ALL_ACCOUNTS_OPTION, ...accountState?.value] : []} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
Co-authored-by: Erik Sundell <erik.sundell87@gmail.com>
What is this feature?
Supports cross-account variable queries by adding the AccountId field to variable queries for log groups, metrics, dimension keys, and dimension values.
Why do we need this feature?
Cross account querying should also be supported by variable queries.
Which issue(s) does this PR fix?:
Fixes #61450
Special notes for your reviewer:
I went through all the variable query types and I believe that these are the only ones that support accountId