CloudWatch: Fix - make sure dimensions are propagated to alert query editor

[conorevans]

Signed-off-by: Conor Evans coevans@tcd.ie

Which issue(s) does this PR fix?:

Fixes #58280

Special notes for your reviewer:

I'm not sure if Cloudwatch can return strings as the value for Dimensions. From what I can see, they're always arrays with a single string. However, I've kept the existing code in place and just modified it to account for that latter case.

Before:

grafana_latest_bug.mov

After:

grafana_local_fixed.mov

[sunker]

Thanks a lot for reporting this issue and proposing a solution @conorevans!

I wonder if the problem can be solved with a different approach though. The alerting engine doesn't support template variables, so before the alerting UI is loaded, it calls interpolateVariablesInQuery in the data source file. It looks like through a pretty complicated chain of calls, expandVariableToArray is being called to interpolate any template variables in the dimension values prop. This method should not convert the dimension value to an array in case the value itself was not a template variable. Would you like to try and come up with a fix using that approach?

Thanks again!

[conorevans]

Hi @sunker thanks for the review and pointing me in a helpful direction! Yeah there's quite the chaining happening here, makes for some gnarly debugging 😅 (esp as I'm no TS whiz).

I've implemented your feedback and it looks cleaner + set properly closer to source than my original tack. Please feel free to suggest any changes or apply them yourself. Cheers

[sunker]

Thanks! Added a few comments. Also, would you be able to add a test for the interpolateVariablesInQueries method? There's already a pattern for testing this method here in the datasource test file. If you could add a scenario with something like should not change structure of dimension prop if its not a template variable, that would be great!

[conorevans]

@sunker implemented that batch of feedback! I tried to add a test via a few different permutations but I could not find a satisfactory way of accessing the dimensions of the interpolated queries. Do you mind taking a look? I'm happy for you to push to my PR

[sunker]

@sunker implemented that batch of feedback! I tried to add a test via a few different permutations but I could not find a satisfactory way of accessing the dimensions of the interpolated queries. Do you mind taking a look? I'm happy for you to push to my PR

Test added!

[conorevans]

@sunker implemented that batch of feedback! I tried to add a test via a few different permutations but I could not find a satisfactory way of accessing the dimensions of the interpolated queries. Do you mind taking a look? I'm happy for you to push to my PR

Test added!

Thank you 😄

[conorevans]

Sorry didn't realise that requesting re-review from @sunker would remove request for review from @iwysiu and @kevinwcyu

[sunker]

@conorevans I'll look into the failing tests as soon as I can!

[sunker]

Sorry @conorevans, dropped the ball on this PR for a while.

The path that I suggested in this PR, making changes to datasource.interpolateVariablesInQuery, had side effects that there are not easy fixes for. I rebased and removed all commits except for the first one, which includes you initial fix. Also added a unit test.

Thanks a lot for contributing and again sorry for the delay!

[conorevans]

All okay @sunker ! Thanks for your efforts