feat(metrics): Extract session metrics from session aggregates [INGEST-678] [INGEST-253] #1140
Conversation
This reverts commit 2c8871d.
jjbayer
changed the title
| fn abnormal_count(&self) -> u32; | ||
| fn crashed_count(&self) -> u32; | ||
| fn errors(&self) -> Option<SessionError>; | ||
| fn final_duration(&self) -> Option<(f64, SessionStatus)>; |
There was a problem hiding this comment.
This is now an interface tailored specifically for extract_session_metrics (especially errors and final_duration), which has little to do with the protocol. But I still think it makes sense to have only a single function for metrics extraction, such that changes apply to both types of sessions.
| true | ||
| } | ||
|
|
||
| fn process_session_aggregates( |
There was a problem hiding this comment.
It might make sense to create a common interface for Session and SessionAggregate (similar to but different from SessionLike) instead, so that we only have to define process_session once.
There was a problem hiding this comment.
this is fine as-is. i agree with your remarks about code duplication (make them jira tickets for now). before merging, can you open a PR on the sentry side for the sessions test harness that does the same thing?
I would like to have the python and rust impl look the same, roughly, i am not sure if that can be achieved
Extract session metrics from session aggregates, just like we do for individual session updates.
Also in this PR: Apply the same validation (timestamp, attributes) to aggregates as to individual session updates.