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

ref(seer grouping): Add platform to did_call_seer metric #80965

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

lobsterkatie
Copy link
Member

This adds the event's platform as a tag on the did_call_seer metric. In order to streamline things a bit, it also adds a new helper, record_did_call_seer_metric, to make the actual datadog call. (This saves a lot of repetition of sample rate, tags, etc.)

Other than the new tag, the only behavior change is that we've been (in theory) also recording did_call_seer during backfill, in cases where the killswitch is enabled. This is a mistake, though a harmless one as we haven't actually had to engage the killswitches in the last few months, so all our current DD data should be legit. Regardless, this fixes that, and only calls the new helper when we have an event (which we only do during ingest, not backfill).

Note: Leaving this in draft because it won't typecheck until some other PRs have gone through, and in order to wait for new did_call_seer calls to be merged, at which point I'll rebase and include them.

@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Nov 19, 2024
Copy link

codecov bot commented Nov 19, 2024

Codecov Report

Attention: Patch coverage is 78.57143% with 3 lines in your changes missing coverage. Please review.

✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
src/sentry/seer/similarity/utils.py 62.50% 0 Missing and 3 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master   #80965      +/-   ##
==========================================
- Coverage   78.48%   78.47%   -0.01%     
==========================================
  Files        7210     7208       -2     
  Lines      319545   319520      -25     
  Branches    43966    43966              
==========================================
- Hits       250799   250753      -46     
- Misses      62360    62372      +12     
- Partials     6386     6395       +9     

metrics.incr(
"grouping.similarity.did_call_seer",
sample_rate=options.get("seer.similarity.metrics_sample_rate"),
tags={"call_made": called, "blocker": blocker, "platform": event.platform},
Copy link
Member

Choose a reason for hiding this comment

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

since event.platform can be None, should we make it unknown if it's None?

@getsantry
Copy link
Contributor

getsantry bot commented Dec 11, 2024

This pull request has gone three weeks without activity. In another week, I will close it.

But! If you comment or otherwise update it, I will reset the clock, and if you add the label WIP, I will leave it alone unless WIP is removed ... forever!


"A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀

@getsantry getsantry bot added Stale and removed Stale labels Dec 11, 2024
@getsantry getsantry bot added the Stale label Jan 3, 2025
@getsantry
Copy link
Contributor

getsantry bot commented Jan 3, 2025

This issue has gone three weeks without activity. In another week, I will close it.

But! If you comment or otherwise update it, I will reset the clock, and if you remove the label Waiting for: Community, I will leave it alone ... forever!


"A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀

@getsantry getsantry bot removed the Stale label Jan 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Scope: Backend Automatically applied to PRs that change backend components
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants