feat(metrics): add global tags to sentry_sdk tags as well #102795
Conversation
🔍 Existing Issues For ReviewYour pull request is modifying functions with the following pre-existing issues: 📄 File: src/sentry/runner/commands/run.py
|
github-actions
bot
added
the
Scope: Backend
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
src/sentry/metrics/middleware.py
Outdated
|
|
||
| def _add_global_tags(_all_threads: bool = False, **tags: TagValue) -> list[Tags]: | ||
| def _add_global_tags( | ||
| _all_threads: bool = False, set_sentry_tags: bool = False, **tags: TagValue |
There was a problem hiding this comment.
| _all_threads: bool = False, set_sentry_tags: bool = False, **tags: TagValue | |
| _all_threads: bool = False, _set_sentry_tags: bool = False, **tags: TagValue |
there is a convention to name all parameters with underscores so they don't collide with actual tags
bmckerry
force-pushed
the
ben/global-tags-sdk
branch
from
16aa284 to
dc65c6d
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #102795 +/- ##
===========================================
- Coverage 80.58% 79.78% -0.81%
===========================================
Files 8938 8962 +24
Lines 391188 405154 +13966
Branches 24884 24884
===========================================
+ Hits 315234 323244 +8010
- Misses 75584 81540 +5956
Partials 370 370 |
| See docstring of `add_global_tags` for how those two methods interact. | ||
| """ | ||
| stack = _add_global_tags(_all_threads=_all_threads, **tags) | ||
| stack = _add_global_tags(all_threads=all_threads, set_sentry_tags=set_sentry_tags, tags=tags) |
There was a problem hiding this comment.
Bug: Sentry Tags Escape Global Context Scope
The global_tags context manager doesn't revert sentry_sdk tags when set_sentry_tags=True is used. When exiting the context, only the metrics tag stack is cleaned up (line 110), but tags set via sentry_sdk.set_tags() (line 69) remain permanently set. This creates inconsistent behavior where metric tags are scoped but sentry tags leak beyond the context, violating the documented promise to "revert all tag changes upon exit."
There was a problem hiding this comment.
can't be bothered to figure out how to revert sentry tags. it's difficult to do cross-thread and APIs keep getting deprecated
This PR adds the option to also configure tags set in the metrics backend via
add_global_tagsto also be set in the sentry_sdk.