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.

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

feat(metric-meta): Normalize invalid metric names #2769

Merged
merged 1 commit into from
Nov 27, 2023

Conversation

Dav1dde
Copy link
Member

@Dav1dde Dav1dde commented Nov 27, 2023

Sentry currently generates metrics with dashes in their names. Generally we want to normalize invalid metric names instead of dropping them. The Python SDK already has this logic, we're now also implementing it in relay.
Consecutive invalid characters will be replaced with a single underscore. Metric names still need to start with a valid character.

@Dav1dde Dav1dde requested a review from a team November 27, 2023 13:46
@Dav1dde Dav1dde force-pushed the fix/mri-dash-in-name branch from de7478f to 6c50403 Compare November 27, 2023 13:48
@mitsuhiko
Copy link
Member

We should normalize _ to - or the other way round. Where would we do that? cc @jan-auer

@Dav1dde Dav1dde changed the title fix(metric-meta): Allow dashes in metric names fix(metric-meta): Normalize dashes to underscores in metric names Nov 27, 2023
@Dav1dde Dav1dde force-pushed the fix/mri-dash-in-name branch from 6c50403 to 2163520 Compare November 27, 2023 14:47
@Dav1dde Dav1dde changed the title fix(metric-meta): Normalize dashes to underscores in metric names feat(metric-meta): Normalize dashes to underscores in metric names Nov 27, 2023
@Dav1dde Dav1dde force-pushed the fix/mri-dash-in-name branch from 2163520 to 539ca5d Compare November 27, 2023 14:52
@Dav1dde Dav1dde changed the title feat(metric-meta): Normalize dashes to underscores in metric names feat(metric-meta): Normalize invalid metric names Nov 27, 2023
@Dav1dde Dav1dde force-pushed the fix/mri-dash-in-name branch from 539ca5d to 7e721be Compare November 27, 2023 15:25
Consecutive invalid characters will be replaced with a single
underscore.
Metric names still need to start with a valid character.
@Dav1dde Dav1dde force-pushed the fix/mri-dash-in-name branch from 7e721be to ea5fbc0 Compare November 27, 2023 16:10
@Dav1dde Dav1dde enabled auto-merge (squash) November 27, 2023 16:11
@Dav1dde Dav1dde merged commit b5a2c23 into master Nov 27, 2023
21 checks passed
@Dav1dde Dav1dde deleted the fix/mri-dash-in-name branch November 27, 2023 16:31
jan-auer added a commit that referenced this pull request Nov 29, 2023
* master: (27 commits)
  ref(metric-meta): Add metric for total incoming metric meta (#2784)
  feat(server): Return global config status for downstream requests (#2765)
  ref(processor): Create event processor sub-module with related code (#2779)
  fix(metrics): Temporarily restore previous configuration keys for bucket splitting (#2780)
  feat(metrics): Add source context to code locations (#2781)
  ref(processor): Split off profile processor code into separate sub-module (#2778)
  ref(processor): Split off replay processing code into separate sub-module (#2776)
  ref(metrics): Partition and split metrics buckets just before sending (#2682)
  feat(spans): Allow well-known path segments in resource URLs (#2770)
  ref(processor): Move user and client reports processing into separate submodule (#2772)
  ref(crons): Include message_type in kafka message (#2723)
  ref(spans): Split tag mapping in specific configs (#2773)
  release: 23.11.2
  feat(metric-meta): Normalize invalid metric names (#2769)
  feat(spans): Extract main_thread tag for spans (#2761)
  Add DE Deployments (#2746)
  ref(processor): Move sessions related code into separate sub-module (#2768)
  ref(metric-meta): Capture envelope payload in a sentry issue (#2767)
  release: 0.8.38
  ref(normalization): Restore span processing to transactionprocessor (#2764)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants