Fix dynamic JobId dimension population for AWS/EMR-Serverless in CloudWatch plugin (#104062) #105506
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Serverless in CloudWatch plugin (grafana#104062) This commit resolves issue grafana#104062 by enabling dynamic population of JobId dimensions based on ApplicationId for AWS/EMRServerless metrics in the Grafana CloudWatch plugin. It also fixes a TypeScript error in FilterItem.tsx and a Prometheus metric registration panic. **Changes:** - **Backend**: - Added `handleGetDimensionValues` in `pkg/tsdb/cloudwatch/metric_find_query.go` to filter JobId by ApplicationId for AWS/EMRServerless using `ListMetrics`. - Added test in `pkg/tsdb/cloudwatch/metric_find_query_test.go` for JobId retrieval. - Removed `datasourceResponseGauge` in `pkg/infra/httpclient/httpclientprovider/datasource_metrics_middleware.go` to fix Prometheus metric conflict panic. - **Frontend**: - Modified `public/plugins/datasource/cloudwatch/MetricsQueryEditor/MetricsQueryEditor.tsx` to pass `applicationId` to `MetricStatEditor`. - Updated `public/plugins/datasource/cloudwatch/shared/MetricStatEditor.tsx` to pass `applicationId` to `Dimensions`. - Updated `public/plugins/datasource/cloudwatch/Dimensions/Dimensions.tsx` to pass `applicationId` to `FilterItem`. - Modified `public/plugins/datasource/cloudwatch/Dimensions/FilterItem.tsx` to include `applicationId` in `getDimensionValues` params for JobId queries, using type assertion due to missing `applicationId` in `datasource.ts`. **Scope:** - Limited to AWS/EMRServerless JobId queries and fixing startup panic; no impact on other functionality. **Notes:** - Type assertion in `FilterItem.tsx` is temporary; apply `datasource.ts` to remove it. - Metric fix is a workaround; consider upstream Grafana issue for proper resolution. Fixes: grafana#104062
Victorthedev
requested review from
kevinwcyu,
njvrzm,
andresmgot,
oshirohugo,
s4kh,
IfSentient and
mustafasencer
and removed request for
a team
|
|
Victorthedev
requested review from
dprokop and
mdvictor
and removed request for
a team
Proximyst
requested review from
a team,
iwysiu and
idastambuk
and removed request for
imatwawana,
JohnnyK-Grafana,
irenerl24,
a team,
joshhunt,
tskarhed,
aangelisc,
mgyongyosi,
colin-stuart,
ashharrison90,
gillesdemey,
konrad147 and
soniaAguilarPeiron
|
This pull request has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in 2 weeks if no further activity occurs. Please feel free to give a status update or ping for review. Thank you for your contributions! |
iwysiu
reviewed
Jul 21, 2025
There was a problem hiding this comment.
HI @Victorthedev , thanks for the pr! I was reviewing this, and did you test this? It doesn't seem like the applicationId is getting passed from the frontend to the backend at any point.
Also, could you clean up the pr a bit? it looks like the backend changes don't do anything and there are a bunch of comments and newlines that are removed that we would like to keep.
| return result, nil | ||
| } | ||
|
|
||
| func (e *cloudWatchExecutor) handleGetDimensionValues(ctx context.Context, pluginCtx backend.PluginContext, parameters url.Values) ([]suggestData, error) { |
There was a problem hiding this comment.
Where is this function getting called?
| }; | ||
|
|
||
| if (namespace === 'AWS/EMRServerless' && filter.key === 'JobId' && applicationId) { | ||
| (params as any).applicationId = applicationId; // Type assertion to bypass TS error |
There was a problem hiding this comment.
You could add applicationId to the GetDimensionValuesRequest in resource/types.ts and declare
const params: GetDimensionValuesRequest = {...
above to avoid using the any
| } from './types'; | ||
| import { CloudWatchVariableSupport } from './variables'; | ||
|
|
||
| export interface DimensionValuesRequest { |
There was a problem hiding this comment.
There is already a GetDimensionValuesRequest in resource/types and I don't think you're using these anywhere
kevinwcyu
removed
area/alerting
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This commit resolves issue #104062 by enabling dynamic population of JobId dimensions based on ApplicationId for AWS/EMRServerless metrics in the Grafana CloudWatch plugin. It also fixes a TypeScript error in FilterItem.tsx and a Prometheus metric registration panic.
Changes:
handleGetDimensionValuesinpkg/tsdb/cloudwatch/metric_find_query.goto filter JobId by ApplicationId for AWS/EMRServerless usingListMetrics.pkg/tsdb/cloudwatch/metric_find_query_test.gofor JobId retrieval.datasourceResponseGaugeinpkg/infra/httpclient/httpclientprovider/datasource_metrics_middleware.goto fix Prometheus metric conflict panic (see Duplicate Prometheus Metric Registration Panic for plugins_datasource_response_size #105507).public/plugins/datasource/cloudwatch/MetricsQueryEditor/MetricsQueryEditor.tsxto passapplicationIdtoMetricStatEditor.public/plugins/datasource/cloudwatch/shared/MetricStatEditor.tsxto passapplicationIdtoDimensions.public/plugins/datasource/cloudwatch/Dimensions/Dimensions.tsxto passapplicationIdtoFilterItem.public/plugins/datasource/cloudwatch/Dimensions/FilterItem.tsxto includeapplicationIdingetDimensionValuesparams for JobId queries, using type assertion due to missingapplicationIdindatasource.ts.Scope:
Notes:
FilterItem.tsxis temporary; applydatasource.tsto remove it.Fixes: #104062
What is this feature?
[Add a brief description of what the feature or update does.]
Why do we need this feature?
[Add a description of the problem the feature is trying to solve.]
Who is this feature for?
[Add information on what kind of user the feature is for.]
Which issue(s) does this PR fix?:
Fixes #
Special notes for your reviewer:
Please check that: