Skip to content

Conversation

Lms24
Copy link
Member

@Lms24 Lms24 commented Sep 1, 2025

When using beforeStartSpan in browserTracingIntgration, we document that users should return a new options object rather than mutating the passed in object in-place. However, nothing keeps users from setting the name in-place like so:'

browserTracingIntegration({
  beforeStartSpan: opts => {
    opts.name = 'changed';
    return opts;
  },
}),

In such a case, we previously didn't catch the mutation and wouldn't adjust the span source attribute to 'custom' accordingly. This PR ensures we take care of this case, too. (found while writing a test for #17500)

@Lms24 Lms24 self-assigned this Sep 1, 2025
@Lms24 Lms24 requested review from s1gr1d, a team and mydea and removed request for a team September 1, 2025 12:45
@Lms24 Lms24 changed the title fix(browser): ensure name and source are set correctly when updating span name in-place in beforeStartSpan fix(browser): Ensure name and source are set correctly when updating span name in-place in beforeStartSpan Sep 1, 2025
@Lms24 Lms24 changed the title fix(browser): Ensure name and source are set correctly when updating span name in-place in beforeStartSpan fix(browser): Ensure source are set correctly when updating span name in-place in beforeStartSpan Sep 1, 2025
@Lms24 Lms24 changed the title fix(browser): Ensure source are set correctly when updating span name in-place in beforeStartSpan fix(browser): Ensure source is set correctly when updating span name in-place in beforeStartSpan Sep 1, 2025
startBrowserTracingPageLoadSpan(client, {
name: 'test span',
attributes: {
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url',
Copy link
Member Author

Choose a reason for hiding this comment

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

this test previously didn't properly test the source adjustment because source by default is already custom.

@Lms24 Lms24 merged commit 1df660b into develop Sep 2, 2025
144 checks passed
@Lms24 Lms24 deleted the lms/fix-browser-beforeStartSpan-name-in-place branch September 2, 2025 08:59
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.

2 participants