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-stats): Add metric_stats generic metrics namespace #66955

Merged
merged 5 commits into from Mar 15, 2024

Conversation

Dav1dde
Copy link
Member

@Dav1dde Dav1dde commented Mar 14, 2024

Adds the metrics_stats namespace and some initial strings.

All strings added are currently unused.

Depends on a librelay release after: getsentry/relay#3267
Epic: getsentry/relay#3147

@Dav1dde Dav1dde requested a review from a team as a code owner March 14, 2024 13:53
@Dav1dde Dav1dde requested a review from a team March 14, 2024 13:53
@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Mar 14, 2024
Copy link

codecov bot commented Mar 14, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 84.27%. Comparing base (b291992) to head (a5cc382).
Report is 58 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #66955      +/-   ##
==========================================
- Coverage   84.31%   84.27%   -0.04%     
==========================================
  Files        5309     5306       -3     
  Lines      237401   237255     -146     
  Branches    41059    41045      -14     
==========================================
- Hits       200172   199954     -218     
- Misses      37011    37083      +72     
  Partials      218      218              
Files Coverage Δ
src/sentry/sentry_metrics/indexer/strings.py 100.00% <100.00%> (ø)
src/sentry/sentry_metrics/use_case_id_registry.py 96.42% <100.00%> (+0.13%) ⬆️

... and 57 files with indirect coverage changes

Copy link
Member

@jjbayer jjbayer left a comment

Choose a reason for hiding this comment

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

See question about counter vs. gauge.

src/sentry/sentry_metrics/indexer/strings.py Outdated Show resolved Hide resolved
@@ -191,6 +191,12 @@
"os.version": PREFIX + 271,
# Performance Score
"sentry.score_profile_version": PREFIX + 272,
# Metric stats
"mri": PREFIX + 273,
Copy link
Member

Choose a reason for hiding this comment

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

We just had an incident (INC-680) where adding new strings here broke existing strings in prod. Can you double check that none of these tags are in the indexer currently? Otherwise they will overwrite the old values and mess up the data.

Copy link
Member Author

Choose a reason for hiding this comment

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

I did checked all of them (see PR description)

Copy link
Member

@iker-barriocanal iker-barriocanal left a comment

Choose a reason for hiding this comment

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

Does the PR require any follow-ups before the use case is used?

@Dav1dde Dav1dde merged commit b000cfd into master Mar 15, 2024
51 checks passed
@Dav1dde Dav1dde deleted the dav1d/feat/metric-stats-namespace branch March 15, 2024 12:52
JonasBa pushed a commit that referenced this pull request Mar 17, 2024
Adds the `metrics_stats` namespace and some initial strings.
@github-actions github-actions bot locked and limited conversation to collaborators Mar 30, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Scope: Backend Automatically applied to PRs that change backend components
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants