-
-
Notifications
You must be signed in to change notification settings - Fork 359
Fixes the issue with unit mismatch in adjustTransactionDuration
#5740
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
Merged
Merged
Changes from all commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
a0aab85
Fixes the issue with unit mismatch in `adjustTransactionDuration`
alwx e09d707
Tests
alwx 4c09552
Changelog entry
alwx 6726b7a
Post review fixes
alwx bc4ef5b
Lint fix
alwx 501170d
Merge branch 'main' into fix/alwx/seconds-ms
alwx fede0f9
Merge branch 'main' into fix/alwx/seconds-ms
alwx File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
157 changes: 157 additions & 0 deletions
157
packages/core/test/tracing/adjustTransactionDuration.test.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,157 @@ | ||
| import { getClient, spanToJSON, startSpanManual } from '@sentry/core'; | ||
| import { adjustTransactionDuration } from '../../src/js/tracing/onSpanEndUtils'; | ||
| import { setupTestClient } from '../mocks/client'; | ||
|
|
||
| jest.mock('react-native', () => { | ||
| return { | ||
| AppState: { | ||
| isAvailable: true, | ||
| currentState: 'active', | ||
| addEventListener: jest.fn(() => ({ remove: jest.fn() })), | ||
| }, | ||
| Platform: { OS: 'ios' }, | ||
| NativeModules: { | ||
| RNSentry: {}, | ||
| }, | ||
| }; | ||
| }); | ||
|
|
||
| describe('adjustTransactionDuration', () => { | ||
| beforeEach(() => { | ||
| setupTestClient(); | ||
| }); | ||
|
|
||
| afterEach(() => { | ||
| jest.clearAllMocks(); | ||
| }); | ||
|
|
||
| it('marks span as deadline_exceeded when duration exceeds maxDurationMs', () => { | ||
| const client = getClient()!; | ||
| const span = startSpanManual({ name: 'Test Transaction', forceTransaction: true }, span => span); | ||
|
|
||
| const maxDurationMs = 60_000; // 60 seconds | ||
| adjustTransactionDuration(client, span, maxDurationMs); | ||
|
|
||
| // End the span 120 seconds after it started (exceeds 60s max) | ||
| const startTimestamp = spanToJSON(span).start_timestamp; | ||
| 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', () => { | ||
| const client = getClient()!; | ||
| const span = startSpanManual({ name: 'Test Transaction', forceTransaction: true }, span => span); | ||
|
|
||
| const maxDurationMs = 60_000; // 60 seconds | ||
| adjustTransactionDuration(client, span, maxDurationMs); | ||
|
|
||
| // End the span 30 seconds after it started (within 60s max) | ||
| const startTimestamp = spanToJSON(span).start_timestamp; | ||
| 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', () => { | ||
| const client = getClient()!; | ||
| const span = startSpanManual({ name: 'Test Transaction', forceTransaction: true }, span => span); | ||
|
|
||
| const maxDurationMs = 60_000; // 60 seconds | ||
| adjustTransactionDuration(client, span, maxDurationMs); | ||
|
|
||
| // End the span exactly 60 seconds after it started | ||
| const startTimestamp = spanToJSON(span).start_timestamp; | ||
| 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', () => { | ||
| const client = getClient()!; | ||
| const span = startSpanManual({ name: 'Test Transaction', forceTransaction: true }, span => span); | ||
|
|
||
| const maxDurationMs = 60_000; | ||
| adjustTransactionDuration(client, span, maxDurationMs); | ||
|
|
||
| // End the span before it started (negative duration) | ||
| const startTimestamp = spanToJSON(span).start_timestamp; | ||
| 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', () => { | ||
| const client = getClient()!; | ||
| const span = startSpanManual({ name: 'Test Transaction', forceTransaction: true }, span => span); | ||
|
|
||
| // maxDurationMs = 600_000 ms = 600 seconds = 10 minutes | ||
| // A span lasting 601 seconds should exceed this limit | ||
| const maxDurationMs = 600_000; | ||
| adjustTransactionDuration(client, span, maxDurationMs); | ||
|
|
||
| const startTimestamp = spanToJSON(span).start_timestamp; | ||
| 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', () => { | ||
| const client = getClient()!; | ||
| const span = startSpanManual({ name: 'Test Transaction', forceTransaction: true }, span => span); | ||
|
|
||
| // maxDurationMs = 600_000 ms = 600 seconds | ||
| // A span lasting 599 seconds should NOT exceed this limit | ||
| const maxDurationMs = 600_000; | ||
| adjustTransactionDuration(client, span, maxDurationMs); | ||
|
|
||
| const startTimestamp = spanToJSON(span).start_timestamp; | ||
| 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', () => { | ||
| const client = getClient()!; | ||
| const trackedSpan = startSpanManual({ name: 'Tracked Transaction', forceTransaction: true }, span => span); | ||
| const otherSpan = startSpanManual({ name: 'Other Transaction', forceTransaction: true }, span => span); | ||
|
|
||
| const maxDurationMs = 60_000; | ||
| adjustTransactionDuration(client, trackedSpan, maxDurationMs); | ||
|
|
||
| // End the other span with a duration that exceeds the limit | ||
| const otherStartTimestamp = spanToJSON(otherSpan).start_timestamp; | ||
| otherSpan.end(otherStartTimestamp + 120); | ||
|
|
||
| // Other span should not be affected by adjustTransactionDuration | ||
| expect(spanToJSON(otherSpan).status).not.toBe('deadline_exceeded'); | ||
|
|
||
| // End tracked span within limits | ||
| const trackedStartTimestamp = spanToJSON(trackedSpan).start_timestamp; | ||
| trackedSpan.end(trackedStartTimestamp + 30); | ||
|
|
||
| expect(spanToJSON(trackedSpan).status).not.toBe('deadline_exceeded'); | ||
| }); | ||
|
|
||
| it('handles very short maxDurationMs values', () => { | ||
| const client = getClient()!; | ||
| const span = startSpanManual({ name: 'Test Transaction', forceTransaction: true }, span => span); | ||
|
|
||
| // 100ms max duration | ||
| const maxDurationMs = 100; | ||
| adjustTransactionDuration(client, span, maxDurationMs); | ||
|
|
||
| // End after 1 second (1000ms > 100ms) | ||
| const startTimestamp = spanToJSON(span).start_timestamp; | ||
| span.end(startTimestamp + 1); | ||
|
|
||
| expect(spanToJSON(span).status).toBe('deadline_exceeded'); | ||
| expect(spanToJSON(span).data).toMatchObject({ maxTransactionDurationExceeded: 'true' }); | ||
| }); | ||
| }); |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
There is a lint issue here. A
yarn fixshould correct that.