-
-
Notifications
You must be signed in to change notification settings - Fork 4.5k
fix(Replay): Include Timestamp Attribute in Trace Item #104030
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
Conversation
| "replay_id": replay_id, | ||
| "segment_id": segment_id, | ||
| "project_id": project.id, | ||
| "timestamp": int(timestamp.timestamp()), | ||
| "category": category, | ||
| } | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: The create_eap_replay_breadcrumb() helper is missing the sentry.timestamp attribute in its generated attributes dictionary, causing queries to return NULL for timestamp aggregations.
Severity: CRITICAL | Confidence: High
🔍 Detailed Analysis
The create_eap_replay_breadcrumb() helper in src/sentry/testutils/cases.py no longer populates the sentry.timestamp attribute in the attributes dictionary. This omission occurs because the line "timestamp": int(timestamp.timestamp()) was removed without a replacement that adds sentry.timestamp to attributes_proto. Consequently, queries in organization_replay_details.py that depend on min(sentry.timestamp) and max(sentry.timestamp) will return NULL for started_at and finished_at. This will cause related test assertions to fail and the replay details UI to display incorrect NULL values.
💡 Suggested Fix
In create_eap_replay_breadcrumb(), add attributes_proto["sentry.timestamp"] = AnyValue(int_value=int(timestamp.timestamp() * 1e9)) to correctly populate the sentry.timestamp attribute, similar to OurLogTestCase.create_ourlog().
🤖 Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: src/sentry/testutils/cases.py#L3860-L3865
Potential issue: The `create_eap_replay_breadcrumb()` helper in
`src/sentry/testutils/cases.py` no longer populates the `sentry.timestamp` attribute in
the `attributes` dictionary. This omission occurs because the line `"timestamp":
int(timestamp.timestamp())` was removed without a replacement that adds
`sentry.timestamp` to `attributes_proto`. Consequently, queries in
`organization_replay_details.py` that depend on `min(sentry.timestamp)` and
`max(sentry.timestamp)` will return `NULL` for `started_at` and `finished_at`. This will
cause related test assertions to fail and the replay details UI to display incorrect
`NULL` values.
Did we get this right? 👍 / 👎 to inform future reviews.
Reference ID: 3644401
This PR is in regards to the following issue: https://sentry.sentry.io/issues/7061220977/?alert_rule_id=14394965&alert_type=issue¬ification_uuid=c59f28da-3fac-4c5f-8e8c-f54412d24dc5&project=11276&referrer=slack
Upon further exploration, it seems like the EAP replay query is unable to find any timestamp data:
This is likely because the timestamp column is not placed as an attribute when we create the trace item.
Relates to: https://linear.app/getsentry/issue/REPLAY-824/create-query-function-for-replay-details