Skip to content

fix(perf): Fix transaction setter on scope to use containing_transaction#1366

Merged
sl0thentr0py merged 3 commits intomasterfrom
neel/WEBBACKEND-92-scope-set-tx-name
Mar 14, 2022
Merged

fix(perf): Fix transaction setter on scope to use containing_transaction#1366
sl0thentr0py merged 3 commits intomasterfrom
neel/WEBBACKEND-92-scope-set-tx-name

Conversation

@sl0thentr0py
Copy link
Copy Markdown
Member

@sl0thentr0py sl0thentr0py commented Mar 9, 2022

fixes #1155, not sure why we have this discrepancy between getter and setter behaviour in the first place

@sl0thentr0py sl0thentr0py requested a review from antonpirker March 9, 2022 13:53
@lobsterkatie
Copy link
Copy Markdown
Member

I think it's fine to change this, but the entire business of getter and setter handling two different types is a headache which I'd love to see us fix. (It would take a new major, since it'd be a breaking change, but it's never led to anything but confusion, for us and users, so highly worth it, IMHO.)

So while it's true that this would make what the issue creator is doing work, IMHO it's kind of an antipattern which we should remove from the docs even if we don't remove from the code. scope.transaction.name = whatever will do it just fine, and doesn't rely on the weird legacy behavior.

@sl0thentr0py sl0thentr0py enabled auto-merge (squash) March 14, 2022 09:10
@sl0thentr0py sl0thentr0py merged commit a6cec41 into master Mar 14, 2022
@sl0thentr0py sl0thentr0py deleted the neel/WEBBACKEND-92-scope-set-tx-name branch March 14, 2022 09:39
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.

Setting transaction name doesn't work from child span.

3 participants