Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/sentry/consumers/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -672,7 +672,7 @@ def create_with_partitions(self, commit, partitions):
# Update the min_partition global tag based on current partition assignment
if partitions:
min_partition = min(p.index for p in partitions)
add_global_tags(min_partition=str(min_partition))
add_global_tags(tags={"min_partition": str(min_partition)})

return self.inner.create_with_partitions(commit, partitions)

Expand Down
33 changes: 23 additions & 10 deletions src/sentry/metrics/middleware.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,10 @@
from contextlib import contextmanager
from threading import local

import sentry_sdk
from django.conf import settings

from sentry.metrics.base import MetricsBackend, MutableTags, Tags, TagValue
from sentry.metrics.base import MetricsBackend, MutableTags, Tags

_BAD_TAGS = frozenset(["event", "project", "group"])
_NOT_BAD_TAGS = frozenset(["use_case_id", "consumer_member_id", "broker_id", "kafka_slice_id"])
Expand Down Expand Up @@ -50,45 +51,57 @@ def _filter_tags(key: str, tags: MutableTags) -> MutableTags:
_GLOBAL_TAGS: list[Tags] = []


def _add_global_tags(_all_threads: bool = False, **tags: TagValue) -> list[Tags]:
if _all_threads:
def _add_global_tags(
all_threads: bool = False, set_sentry_tags: bool = False, tags: Tags | None = None
) -> list[Tags]:
if all_threads:
stack = _GLOBAL_TAGS
else:
if not hasattr(_THREAD_LOCAL_TAGS, "stack"):
stack = _THREAD_LOCAL_TAGS.stack = []
else:
stack = _THREAD_LOCAL_TAGS.stack

if tags is None:
tags = {}
stack.append(tags)
if set_sentry_tags:
sentry_sdk.set_tags(tags)
return stack


def add_global_tags(_all_threads: bool = False, **tags: TagValue) -> None:
def add_global_tags(
all_threads: bool = False, set_sentry_tags: bool = False, tags: Tags | None = None
) -> None:
"""
Set multiple metric tags onto the global or thread-local stack which then
apply to all metrics.
If `set_sentry_tags` is True, also sets the given tags in sentry_sdk.

When used in combination with the `global_tags` context manager,
`add_global_tags` is reverted in any wrapping invocaation of `global_tags`.
`add_global_tags` is reverted in any wrapping invocation of `global_tags`.
However, tags set in the current sentry_sdk instance will remain set there.
For example::

with global_tags(tag_a=123):
add_global_tags(tag_b=123)
with global_tags(tags={"tag_a": 123}):
add_global_tags(tags={"tag_b": 123})

# tag_b is no longer visible
"""
_add_global_tags(_all_threads=_all_threads, **tags)
_add_global_tags(all_threads=all_threads, set_sentry_tags=set_sentry_tags, tags=tags)


@contextmanager
def global_tags(_all_threads: bool = False, **tags: TagValue) -> Generator[None]:
def global_tags(
all_threads: bool = False, set_sentry_tags: bool = False, tags: Tags | None = None
) -> Generator[None]:
"""
The context manager version of `add_global_tags` that reverts all tag
changes upon exit.

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)
Copy link
Contributor

Choose a reason for hiding this comment

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

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."

Fix in Cursor Fix in Web

Copy link
Member

Choose a reason for hiding this comment

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

can't be bothered to figure out how to revert sentry tags. it's difficult to do cross-thread and APIs keep getting deprecated

old_len = len(stack) - 1

try:
Expand Down
7 changes: 6 additions & 1 deletion src/sentry/runner/commands/run.py
Original file line number Diff line number Diff line change
Expand Up @@ -593,7 +593,12 @@ def basic_consumer(
logging.getLogger("arroyo").setLevel(log_level.upper())

add_global_tags(
kafka_topic=topic, consumer_group=options["group_id"], kafka_slice_id=kafka_slice_id
set_sentry_tags=True,
tags={
"kafka_topic": topic,
"consumer_group": options["group_id"],
"kafka_slice_id": kafka_slice_id,
},
)
processor = get_stream_processor(
consumer_name,
Expand Down
4 changes: 1 addition & 3 deletions src/sentry/sentry_metrics/configuration.py
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,4 @@ def initialize_main_process_state(config: MetricsIngestConfiguration) -> None:

from sentry.utils.metrics import add_global_tags

global_tag_map = {"pipeline": config.internal_metrics_tag or ""}

add_global_tags(_all_threads=True, **global_tag_map)
add_global_tags(all_threads=True, tags={"pipeline": config.internal_metrics_tag or ""})
2 changes: 1 addition & 1 deletion src/sentry/tasks/store.py
Original file line number Diff line number Diff line change
Expand Up @@ -533,7 +533,7 @@ def _do_save_event(
event_data=data,
)

with metrics.global_tags(event_type=event_type):
with metrics.global_tags(tags={"event_type": event_type}):
if event_id is None and data is not None:
event_id = data["event_id"]

Expand Down
2 changes: 1 addition & 1 deletion src/sentry/utils/arroyo.py
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ def _initialize_arroyo_subprocess(initializer: Callable[[], None] | None, tags:
from sentry.metrics.middleware import add_global_tags

# Inherit global tags from the parent process
add_global_tags(_all_threads=True, **tags)
add_global_tags(all_threads=True, tags=tags)


def initialize_arroyo_main() -> None:
Expand Down
4 changes: 2 additions & 2 deletions tests/sentry/metrics/test_middleware.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,9 @@ def test_filter_tags_prod() -> None:
def test_global() -> None:
assert get_current_global_tags() == {}

with global_tags(tag_a=123):
with global_tags(tags={"tag_a": 123}):
assert get_current_global_tags() == {"tag_a": 123}
add_global_tags(tag_b=123)
add_global_tags(tags={"tag_b": 123})

assert get_current_global_tags() == {"tag_a": 123, "tag_b": 123}

Expand Down
2 changes: 1 addition & 1 deletion tests/sentry/sentry_metrics/test_parallel_indexer.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
def reset_global_metrics_state():
# running a MetricsConsumerStrategyFactory has a side-effect of mutating
# global metrics tags
with global_tags(_all_threads=True):
with global_tags(all_threads=True):
yield


Expand Down
Loading