Skip to content
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

fix(aws-lambda): Avoid overwriting root span name #15000

Merged
merged 2 commits into from
Jan 17, 2025

Conversation

Lms24
Copy link
Member

@Lms24 Lms24 commented Jan 14, 2025

Came across this by chance: Updating event.transaction in an event processor without checking for the type also always overwrites the root span name. To fix this, this PR calls scope.updateTransactionName which updates the error location "transaction" name on the scope. This is only applied to error events during event processing, so the intention is clearer than adding an event processor.

This was mentioned in #13391 but admittedly I must have misread the issue because I'm 99% sure the event processor was unintended behaviour from our end.

@Lms24 Lms24 self-assigned this Jan 14, 2025
Copy link
Member

@andreiborza andreiborza left a comment

Choose a reason for hiding this comment

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

thanks for fixing this!

@Lms24 Lms24 force-pushed the lms/fix-aws-lambda-scope-transactionname branch from c50718e to b930e03 Compare January 17, 2025 11:09
@Lms24 Lms24 enabled auto-merge (squash) January 17, 2025 11:10
Copy link
Contributor

size-limit report 📦

⚠️ Warning: Base artifact is not the latest one, because the latest workflow run is not done yet. This may lead to incorrect results. Try to re-run all tests to get up to date results.

Path Size % Change Change
@sentry/browser 22.86 KB - -
@sentry/browser - with treeshaking flags 21.53 KB - -
@sentry/browser (incl. Tracing) 35.56 KB - -
@sentry/browser (incl. Tracing, Replay) 72.37 KB - -
@sentry/browser (incl. Tracing, Replay) - with treeshaking flags 62.86 KB - -
@sentry/browser (incl. Tracing, Replay with Canvas) 76.62 KB - -
@sentry/browser (incl. Tracing, Replay, Feedback) 88.63 KB - -
@sentry/browser (incl. Feedback) 39.08 KB - -
@sentry/browser (incl. sendFeedback) 27.5 KB - -
@sentry/browser (incl. FeedbackAsync) 32.27 KB - -
@sentry/react 25.56 KB - -
@sentry/react (incl. Tracing) 38.36 KB - -
@sentry/vue 26.95 KB - -
@sentry/vue (incl. Tracing) 37.31 KB - -
@sentry/svelte 22.99 KB - -
CDN Bundle 24.27 KB - -
CDN Bundle (incl. Tracing) 35.9 KB -0.05% -15 B 🔽
CDN Bundle (incl. Tracing, Replay) 70.53 KB -0.03% -17 B 🔽
CDN Bundle (incl. Tracing, Replay, Feedback) 75.67 KB -0.03% -16 B 🔽
CDN Bundle - uncompressed 70.81 KB - -
CDN Bundle (incl. Tracing) - uncompressed 106.43 KB +0.01% +6 B 🔺
CDN Bundle (incl. Tracing, Replay) - uncompressed 217.25 KB +0.01% +6 B 🔺
CDN Bundle (incl. Tracing, Replay, Feedback) - uncompressed 229.79 KB +0.01% +6 B 🔺
@sentry/nextjs (client) 38.47 KB - -
@sentry/sveltekit (client) 36.11 KB - -
@sentry/node 161.32 KB -0.01% -1 B 🔽
@sentry/node - without tracing 97.11 KB - -
@sentry/aws-serverless 127 KB - -

View base workflow run

Copy link

codecov bot commented Jan 17, 2025

❌ 1 Tests Failed:

Tests completed Failed Passed Skipped
692 1 691 299
View the full list of 1 ❄️ flaky tests
tracing/request/fetch/test.ts should create spans for fetch requests

Flake rate in main: 11.76% (Passed 45 times, Failed 6 times)

Stack Traces | 10.1s run time
test.ts:7:11 should create spans for fetch requests

To view more test analytics, go to the Test Analytics Dashboard
📢 Thoughts on this report? Let us know!

@Lms24 Lms24 merged commit 3e14f6b into develop Jan 17, 2025
155 checks passed
@Lms24 Lms24 deleted the lms/fix-aws-lambda-scope-transactionname branch January 17, 2025 11:31
Lms24 added a commit that referenced this pull request Jan 17, 2025
Calls `scope.updateTransactionName`
which updates the error location "transaction" name on the scope. This
is only applied to error events during event processing, so the
intention is clearer than adding an event processor.
Lms24 added a commit that referenced this pull request Jan 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants