Repository navigation
Conversation
…ribute The `adjustTransactionDuration` handler set a `maxTransactionDurationExceeded` span-data flag alongside the `deadline_exceeded` status. The flag is a leftover from the old `@sentry/tracing` `maxTransactionDuration` option, which upstream replaced with `finalTimeout` in v7 (getsentry/sentry-javascript#5044). Nothing consumes it: no references in sentry, relay, or current sentry-javascript, and `@sentry/core` already records the modern equivalent (`sentry.idle_span_finish_reason: "finalTimeout"`) when the idle span times out. Drop the dead attribute. The `deadline_exceeded` status and both duration guards are unchanged; tests still assert the status. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Semver Impact of This PR⚪ None (no version bump detected) 📋 Changelog PreviewThis is how your changes will appear in the changelog.
🤖 This preview updates automatically when you update the PR. |
|
|
||
| if (isOutdatedTransaction) { | ||
| span.setStatus({ code: SPAN_STATUS_ERROR, message: 'deadline_exceeded' }); | ||
| // TODO: check where was used, might be possible to delete | ||
| span.setAttribute('maxTransactionDurationExceeded', 'true'); |
There was a problem hiding this comment.
Won't this be a break change with users alert filters?
There was a problem hiding this comment.
Thank you for calling this out @lucas-zimerman 🙇 I've rechecked the data with HEX: Across configured queries (metric/issue alerts, saved searches, Discover saved queries, and dashboard widgets) there are 0 references of maxTransactionDurationExceeded, vs 171 orgs using the deadline_exceededstatus filter.
if we decide to keep it as a minor, we should add a field on the changelog about it.
Makes sense 👍 Added a changelog entry. Also updated the PR description adding the full timeline of the attribute.
There was a problem hiding this comment.
Thank you for your investigation!
It still could affect self hosted but id say so far so good for a release
There was a problem hiding this comment.
It still could affect self hosted but id say so far so good for a release
True 👍 I'd advocate on shipping the removal now and crossing this off but we could also keep it for the v9 bump. I'll leave the final approval to @alwx 🙇
|
if we decide to keep it as a minor, we should add a field on the changelog about it. |
2 participants
📢 Type of change
📜 Description
adjustTransactionDuration(onSpanEndUtils.ts) set amaxTransactionDurationExceededspan-data flag alongside thedeadline_exceededstatus. This removes the flag (and the stale// TODO: check where was used, might be possible to deletenext to it). Thedeadline_exceededstatus and both duration guards (diff < 0, recorded-duration-exceeds-finalTimeout) are unchanged.💡 Motivation and Context
The flag is a leftover from the old
@sentry/tracingmaxTransactionDurationoption, which upstream replaced withfinalTimeoutin v7 (getsentry/sentry-javascript#5044). It only survives in archived@sentry/tracingmirrors.Timeline
maxTransactionDurationExceededadded to the RN SDK (#1230), mirroring the old@sentry/tracingadjustTransactionDuration(which set it as a tag).maxTransactionDurationoption withfinalTimeout(the diff deletestransaction.setTag('maxTransactionDurationExceeded', 'true')).deadline_exceededstatus.The
deadline_exceededstatus is set on the exact same transactions (the sameif (isOutdatedTransaction)branch), and@sentry/core's idle span also emits it onfinalTimeout, so it remains the signal to filter timed-out transactions on.💚 How did you test it?
0references ofmaxTransactionDurationExceeded, vs171orgs using thedeadline_exceededstatus filter📝 Checklist
sendDefaultPIIis enabled.🔮 Next steps