chore(replay): add a log sampling filter and sample replay ingest logs #79879
Conversation
github-actions
bot
added
the
Scope: Backend
| @@ -24,7 +25,18 @@ | |||
| from sentry.utils import json, metrics | |||
| from sentry.utils.outcomes import Outcome, track_outcome | |||
|
|
|||
| MOBILE_EVENT_SAMPLE_RATE = 0.5 | |||
There was a problem hiding this comment.
Sample rates defined here, can make them options if we want. Values right now are placeholders
Codecov ReportAll modified and coverable lines are covered by tests ✅ ✅ All tests successful. No failed tests found. Additional details and impacted files@@ Coverage Diff @@
## master #79879 +/- ##
==========================================
- Coverage 78.45% 78.34% -0.12%
==========================================
Files 7144 7144
Lines 316041 316057 +16
Branches 43546 43547 +1
==========================================
- Hits 247951 247614 -337
- Misses 61764 62100 +336
- Partials 6326 6343 +17 |
|
|
||
| def __init__(self, prob_mapping: dict[str, float]): | ||
| super().__init__() | ||
| self.prob_mapping = prob_mapping |
There was a problem hiding this comment.
Don't abbreviate. probability_map or probability_mapping.
| logger.addFilter( | ||
| SamplingFilter( | ||
| { | ||
| "mobile_event": MOBILE_EVENT_SAMPLE_RATE, |
There was a problem hiding this comment.
Why is this different from the other consumer?
There was a problem hiding this comment.
Are you suggesting we add rrweb_event_count here too? It's not currently logged in this file
There was a problem hiding this comment.
No just asking the question. If we're not logging it we don't need to include it.
There was a problem hiding this comment.
We're only logging mobile_event in this one
| "mobile_event": MOBILE_EVENT_SAMPLE_RATE, | ||
| "rrweb_event_count": RRWEB_EVENT_COUNT_SAMPLE_RATE, |
There was a problem hiding this comment.
Are either of these log messages emitted in our consumer?
There was a problem hiding this comment.
Do you mean a different file?
There was a problem hiding this comment.
What does "mobile_event" represent? The log message?
There was a problem hiding this comment.
Just realized I was looking at the event_type in extras. The msg is sentry.replays.slow_click. I'll have to rethink this
There was a problem hiding this comment.
Do we want different rates for these, or could we sample all slow click logs at the same rate for now?
There was a problem hiding this comment.
Can you access the extra field within your sampler class?
There was a problem hiding this comment.
I can. The filter will become specific to our usecase though, so I'd move it into the init file
| @@ -24,7 +25,18 @@ | |||
| from sentry.utils import json, metrics | |||
| from sentry.utils.outcomes import Outcome, track_outcome | |||
|
|
|||
| MOBILE_EVENT_SAMPLE_RATE = 0.5 | |||
|
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 "A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀 |
|
@cmanallen @aliu39 need any help w/ this? :D |
|
@JoshFerge Andrew and I are on PTO until next week. You're welcome to help out! I'll take a peak at this tomorrow or Wednesday and review it for you (if you make any changes)! |
|
IIRC here was a reason Colton blocked this that we forgot to document here. Last we talked we were thinking of trying a whole different approach |
|
okay! i was thinking we could just sample 1% of logs. is there any reason that doesn't work? |
|
cc @cmanallen
@JoshFerge That seems fine to me but wanted to loop in Colton on this. FF is launching tomorrow, I'll follow up after and try to close this by the end of the week. |
|
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 "A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀 |
|
Sorry this got lost.. since this is tracked in the ticket, I'll close, and we can revisit after holidays |
Closes