Skip to content

Conversation

michellewzhang
Copy link
Member

@michellewzhang michellewzhang commented Aug 28, 2025

🚨 for actual test changes: use this view https://github.com/getsentry/sentry/pull/98482/files/9500752d1c341f3a051599c7e5bd40deb8143f18

closes https://linear.app/getsentry/issue/REPLAY-621/create-log-parsing-test-suite-with-real-replay-event-data

  • use real replay events to test as_log_message. serves as an example for reliable test data that can be referenced in the future. all types in EventType are tested
  • validates that get_timestamp_unit returns the correct timestamp based on the real replay data
  • fixed a bug where feedback breadcrumbs were incorrectly categorized as s when they should be ms
  • validates that the log message timestamp is always in MS

def get_timestamp_unit(event_type: EventType) -> Literal["s", "ms"]:
"""
Returns the time unit of event["timestamp"] for a replay event.
This is not guaranteed to match event.data.payload.timestamp.
We do not allow wildcard or default cases. Please be explicit when adding new types.
Beware that EventType.UNKNOWN returns "ms" but there's no way to know the actual unit.
"""
match event_type:
case (
EventType.CLS
| EventType.LCP
| EventType.FEEDBACK
| EventType.MEMORY
| EventType.MUTATIONS
| EventType.NAVIGATION_SPAN
| EventType.RESOURCE_FETCH
| EventType.RESOURCE_IMAGE
| EventType.RESOURCE_SCRIPT
| EventType.RESOURCE_XHR
| EventType.UI_BLUR
| EventType.UI_FOCUS
):
return "s"
case (
EventType.CANVAS
| EventType.CONSOLE
| EventType.CLICK
| EventType.DEAD_CLICK
| EventType.RAGE_CLICK
| EventType.SLOW_CLICK
| EventType.HYDRATION_ERROR
| EventType.NAVIGATION
| EventType.OPTIONS
| EventType.UNKNOWN
):
return "ms"

Copy link

linear bot commented Aug 28, 2025

@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Aug 28, 2025

This comment was marked as outdated.

Copy link
Member

@cmanallen cmanallen left a comment

Choose a reason for hiding this comment

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

This file should be a use case: src/sentry/replays/lib/summarize.py.

A library should be generic and totally unrelated to replay. This is coupled to replay so its an implementation detail of the product. You don't have to address on this PR. But eventually it should be moved.

@michellewzhang
Copy link
Member Author

michellewzhang commented Sep 2, 2025

This file should be a use case: src/sentry/replays/lib/summarize.py.

A library should be generic and totally unrelated to replay. This is coupled to replay so its an implementation detail of the product. You don't have to address on this PR. But eventually it should be moved.

@cmanallen moved! (both that file & the test file)

Copy link
Member

@cmanallen cmanallen left a comment

Choose a reason for hiding this comment

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

thank you!

cursor[bot]

This comment was marked as outdated.

@michellewzhang michellewzhang merged commit 56c9ced into master Sep 2, 2025
64 checks passed
@michellewzhang michellewzhang deleted the mz/fix-tests branch September 2, 2025 21:24
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.

3 participants