Skip to content

improv(metrics): emit warning when default dimensions are overwritten #4287

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

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

uttam282005
Copy link
Contributor

Summary

Changes

This PR restores and improves the warning mechanism for overwritten default dimensions in the Metrics utility:

Key Updates:

  • Introduced a static utility method suppressSetDefaultDimensionsWarnings() to temporarily suppress warnings during internal setDefaultDimensions() calls.
  • Fixes a bug where calling captureColdStartMetric() would emit duplicate warnings for the service dimension.
  • Refines warning behavior to only trigger when the overwritten service dimension is different from the internal default (i.e., truly user-defined).

Affected Code:

  • metrics.ts (suppression logic added)
  • dimension.test.ts (added tests covering the changes)
  • captureColdStartMetric updated to wrap internal setDefaultDimensions call
  • Constructor logic updated to avoid redundant warnings when values match

Issue number: closes #4134


By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Disclaimer: We value your time and bandwidth. As such, any pull requests created on non-triaged issues might not be successful.

@pull-request-size pull-request-size bot added the size/L PRs between 100-499 LOC label Aug 7, 2025
@boring-cyborg boring-cyborg bot added metrics This item relates to the Metrics Utility tests PRs that add or change tests labels Aug 7, 2025
Copy link
Contributor

@dreamorosi dreamorosi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll review this PR tomorrow, I am not 100% sure about wanting to add an internal flag but I need time to look at the code and come up with a proposal.

Please leave the PR open and, if possible, avoid making changes until we have time to review it.

Thank you!

@uttam282005
Copy link
Contributor Author

sure.

@dreamorosi dreamorosi changed the title fix(metrics): Emit warning when default dimensions are overwritten and suppress warnings in case of internal calls to setDefaultDimension. improv(metrics): emit warning when default dimensions are overwritten Aug 8, 2025
Copy link

sonarqubecloud bot commented Aug 8, 2025

@dreamorosi dreamorosi self-requested a review August 8, 2025 16:12
Copy link
Contributor

@dreamorosi dreamorosi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I need a bit more time - I've started looking at the PR and have made a couple commits to remove the static flag, but it needs some more work.

I'll continue by Monday end of day - please leave the PR open and if possible, avoid making changes to the code.

@dreamorosi dreamorosi added the do-not-merge This item should not be merged label Aug 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
do-not-merge This item should not be merged metrics This item relates to the Metrics Utility size/L PRs between 100-499 LOC tests PRs that add or change tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Maintenance: Add warning for overwriting dimensions in setDefaultDimensions method
2 participants