Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[spanmetrics] Sanitize name in mapped dimensions #3171

Merged
merged 5 commits into from Nov 23, 2023
Merged

[spanmetrics] Sanitize name in mapped dimensions #3171

merged 5 commits into from Nov 23, 2023

Conversation

mapno
Copy link
Member

@mapno mapno commented Nov 23, 2023 •
edited

What this PR does:

If not sanitized, Prometheus incompatible labels can be sent.

Which issue(s) this PR fixes:
Fixes #

Checklist

  • Tests updated
  • Documentation added
  • CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]

@mapno mapno enabled auto-merge (squash) November 23, 2023 10:52
kvrhdn
kvrhdn approved these changes Nov 23, 2023
View reviewed changes
Copy link
Member

@kvrhdn kvrhdn left a comment

Choose a reason for hiding this comment

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

LGTM!

Since we are calling the sanitize function at multiple points, I wonder if we should move this into the registry? The registry could do it more centralized and we don't risk forgetting about it in the processors.

@mapno mapno merged commit a0a0ce2 into grafana:main Nov 23, 2023
14 checks passed
@mapno mapno deleted the sanitize-dimensions-mapping-name branch November 23, 2023 17:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@electron0zero electron0zero electron0zero left review comments

@kvrhdn kvrhdn kvrhdn approved these changes

@joe-elliott joe-elliott Awaiting requested review from joe-elliott joe-elliott is a code owner

@annanay25 annanay25 Awaiting requested review from annanay25 annanay25 is a code owner

@mdisibio mdisibio Awaiting requested review from mdisibio mdisibio is a code owner

@zalegrala zalegrala Awaiting requested review from zalegrala zalegrala is a code owner

@ie-pham ie-pham Awaiting requested review from ie-pham ie-pham is a code owner

@stoewer stoewer Awaiting requested review from stoewer stoewer is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@mapno @kvrhdn @electron0zero