Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,10 @@

## Unreleased

### Changes

- Remove the undocumented `maxTransactionDurationExceeded` span attribute; use the `deadline_exceeded` span status to filter timed-out transactions instead ([#6820](https://github.com/getsentry/sentry-react-native/pull/6820))

### Fixes

- Fix stale `turbo_module.*` tags on Android native crashes ([#6823](https://github.com/getsentry/sentry-react-native/pull/6823))
Expand Down
2 changes: 0 additions & 2 deletions packages/core/src/js/tracing/onSpanEndUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -92,8 +92,6 @@ export const adjustTransactionDuration = (client: Client, span: Span, maxDuratio

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');
Comment on lines 92 to -96

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Won't this be a break change with users alert filters?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

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.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thank you for your investigation!
It still could affect self hosted but id say so far so good for a release

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

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 ๐Ÿ™‡

}
});
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ describe('adjustTransactionDuration', () => {
span.end(startTimestamp + 120);

expect(spanToJSON(span).status).toBe('deadline_exceeded');
expect(spanToJSON(span).data).toMatchObject({ maxTransactionDurationExceeded: 'true' });
});

it('does not mark span as deadline_exceeded when duration is within maxDurationMs', () => {
Expand All @@ -53,7 +52,6 @@ describe('adjustTransactionDuration', () => {
span.end(startTimestamp + 30);

expect(spanToJSON(span).status).not.toBe('deadline_exceeded');
expect(spanToJSON(span).data).not.toMatchObject({ maxTransactionDurationExceeded: 'true' });
});

it('does not mark span as deadline_exceeded when duration equals maxDurationMs exactly', () => {
Expand All @@ -68,7 +66,6 @@ describe('adjustTransactionDuration', () => {
span.end(startTimestamp + 60);

expect(spanToJSON(span).status).not.toBe('deadline_exceeded');
expect(spanToJSON(span).data).not.toMatchObject({ maxTransactionDurationExceeded: 'true' });
});

it('marks span as deadline_exceeded when duration is negative', () => {
Expand All @@ -83,7 +80,6 @@ describe('adjustTransactionDuration', () => {
span.end(startTimestamp - 10);

expect(spanToJSON(span).status).toBe('deadline_exceeded');
expect(spanToJSON(span).data).toMatchObject({ maxTransactionDurationExceeded: 'true' });
});

it('correctly handles maxDurationMs in milliseconds not seconds', () => {
Expand All @@ -99,7 +95,6 @@ describe('adjustTransactionDuration', () => {
span.end(startTimestamp + 601);

expect(spanToJSON(span).status).toBe('deadline_exceeded');
expect(spanToJSON(span).data).toMatchObject({ maxTransactionDurationExceeded: 'true' });
});

it('does not mark span when duration is 599 seconds with 600_000ms max', () => {
Expand All @@ -115,7 +110,6 @@ describe('adjustTransactionDuration', () => {
span.end(startTimestamp + 599);

expect(spanToJSON(span).status).not.toBe('deadline_exceeded');
expect(spanToJSON(span).data).not.toMatchObject({ maxTransactionDurationExceeded: 'true' });
});

it('does not affect spans from other transactions', () => {
Expand Down Expand Up @@ -153,6 +147,5 @@ describe('adjustTransactionDuration', () => {
span.end(startTimestamp + 1);

expect(spanToJSON(span).status).toBe('deadline_exceeded');
expect(spanToJSON(span).data).toMatchObject({ maxTransactionDurationExceeded: 'true' });
});
});
Loading