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(perf): Fix transaction setter on scope to use containing_transaction #1366

Merged
merged 3 commits into from Mar 14, 2022

Conversation

sl0thentr0py
Copy link
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

@lobsterkatie
Copy link
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