From 25494965f7cfbbbbb8102c54c657dd744e8a5086 Mon Sep 17 00:00:00 2001 From: Armin Ronacher Date: Thu, 21 Sep 2023 09:51:05 +0200 Subject: [PATCH 01/11] feat(minimetrics): Switch the backend over to sentry-sdk minimetrics --- src/minimetrics/__init__.py | 4 - src/minimetrics/core.py | 377 ----------------------- src/minimetrics/transport.py | 130 -------- src/minimetrics/types.py | 83 ----- src/sentry/metrics/minimetrics.py | 137 ++++---- src/sentry/utils/sdk.py | 3 + tests/minimetrics/__init__.py | 0 tests/minimetrics/test_core.py | 174 ----------- tests/minimetrics/test_transport.py | 234 -------------- tests/sentry/metrics/test_minimetrics.py | 153 +++++++-- 10 files changed, 208 insertions(+), 1087 deletions(-) delete mode 100644 src/minimetrics/__init__.py delete mode 100644 src/minimetrics/core.py delete mode 100644 src/minimetrics/transport.py delete mode 100644 src/minimetrics/types.py delete mode 100644 tests/minimetrics/__init__.py delete mode 100644 tests/minimetrics/test_core.py delete mode 100644 tests/minimetrics/test_transport.py diff --git a/src/minimetrics/__init__.py b/src/minimetrics/__init__.py deleted file mode 100644 index 6d933cc538b8..000000000000 --- a/src/minimetrics/__init__.py +++ /dev/null @@ -1,4 +0,0 @@ -from .core import MiniMetricsClient -from .types import MetricTagsExternal - -__all__ = ["MiniMetricsClient", "MetricTagsExternal"] diff --git a/src/minimetrics/core.py b/src/minimetrics/core.py deleted file mode 100644 index 263d8c2651a1..000000000000 --- a/src/minimetrics/core.py +++ /dev/null @@ -1,377 +0,0 @@ -import os -import threading -import time -import zlib -from functools import wraps -from threading import Event, Lock, Thread -from typing import Any, Callable, Dict, Iterable, List, Optional, Set, Tuple, Union - -import sentry_sdk - -from minimetrics.transport import MetricEnvelopeTransport, RelayStatsdEncoder -from minimetrics.types import ( - BucketKey, - FlushableBuckets, - FlushedMetricValue, - Metric, - MetricTagsExternal, - MetricTagsInternal, - MetricType, - MetricUnit, - MetricValue, -) -from sentry import options -from sentry.utils import metrics - -thread_local = threading.local() - - -def in_minimetrics(): - try: - return thread_local.in_minimetrics - except AttributeError: - return False - - -def minimetrics_noop(func): - @wraps(func) - def new_func(*args, **kwargs): - try: - in_minimetrics = thread_local.in_minimetrics - except AttributeError: - in_minimetrics = False - thread_local.in_minimetrics = True - try: - if not in_minimetrics: - return func(*args, **kwargs) - finally: - thread_local.in_minimetrics = in_minimetrics - - return new_func - - -class CounterMetric(Metric[float]): - __slots__ = ("value",) - - def __init__(self, first: float) -> None: - self.value = first - - @property - def weight(self) -> int: - return 1 - - def add(self, value: float) -> None: - self.value += value - - def serialize_value(self) -> Iterable[FlushedMetricValue]: - return (self.value,) - - -class GaugeMetric(Metric[float]): - __slots__ = ( - "last", - "min", - "max", - "sum", - "count", - ) - - def __init__(self, first: float) -> None: - self.last = first - self.min = first - self.max = first - self.sum = first - self.count = 1 - - @property - def weight(self) -> int: - # Number of elements. - return 5 - - def add(self, value: float) -> None: - self.last = value - self.min = min(self.min, value) - self.max = max(self.max, value) - self.sum += value - self.count += 1 - - def serialize_value(self) -> Iterable[FlushedMetricValue]: - return ( - self.last, - self.min, - self.max, - self.sum, - self.count, - ) - - -class DistributionMetric(Metric[float]): - __slots__ = ("value",) - - def __init__(self, first: float) -> None: - self.value: List[float] = [first] - - @property - def weight(self) -> int: - return len(self.value) - - def add(self, value: float) -> None: - self.value.append(float(value)) - - def serialize_value(self) -> Iterable[FlushedMetricValue]: - return self.value - - -class SetMetric(Metric[Union[str, int]]): - __slots__ = ("value",) - - def __init__(self, first: Union[str, int]) -> None: - self.value: Set[Union[str, int]] = {first} - - @property - def weight(self) -> int: - return len(self.value) - - def add(self, value: Union[str, int]) -> None: - self.value.add(value) - - def serialize_value(self) -> Iterable[FlushedMetricValue]: - def _hash(x: Any) -> int: - if isinstance(x, str): - return zlib.crc32(x.encode("utf-8")) & 0xFFFFFFFF - return int(x) - - return (_hash(value) for value in self.value) - - -METRIC_TYPES: Dict[str, Callable[[Any], Metric[Any]]] = { - "c": CounterMetric, - "g": GaugeMetric, - "d": DistributionMetric, - "s": SetMetric, -} - - -class Aggregator: - ROLLUP_IN_SECONDS = 10.0 - MAX_WEIGHT = 100000 - DEFAULT_SAMPLE_RATE = 1.0 - - def __init__(self) -> None: - # Buckets holding the grouped metrics. The buckets are represented in two levels, in order to more efficiently - # perform locking. - self.buckets: Dict[int, Dict[BucketKey, Metric[Any]]] = {} - # Stores the total weight of the in-memory buckets. Weight is determined on a per metric type basis and - # represents how much weight is there to represent the metric (e.g., counter = 1, distribution = n). - self._buckets_total_weight: int = 0 - # Transport layer used to send metrics. - self._transport: MetricEnvelopeTransport = MetricEnvelopeTransport(RelayStatsdEncoder()) - # Lock protecting concurrent access to variables by the flusher and the calling threads that call add or stop. - self._lock: Lock = Lock() - # Signals whether the loop of the flusher is running. - self._running: bool = True - # Used to maintain synchronization between the flusher and external callers. - self._flush_event: Event = Event() - # Use to signal whether we want to flush the buckets in the next loop iteration, irrespectively of the cutoff. - self._force_flush: bool = False - - # Thread handling the flushing loop. - self._flusher: Optional[Thread] = None - self._flusher_pid: Optional[int] = None - self._ensure_thread() - - def _ensure_thread(self): - """For forking processes we might need to restart this thread. - This ensures that our process actually has that thread running. - """ - pid = os.getpid() - if self._flusher_pid == pid: - return - with self._lock: - self._flusher_pid = pid - self._flusher = Thread(target=self._flush_loop) - self._flusher.daemon = True - self._flusher.start() - - def _flush_loop(self) -> None: - thread_local.in_minimetrics = True - while self._running or self._force_flush: - self._flush() - self._flush_event.wait(5.0) - - def _flush(self): - flushable_buckets, _ = self._flushable_buckets() - if flushable_buckets: - # You should emit metrics to `metrics` only inside this method, since we know that if we received - # metrics the `sentry.utils.metrics` file was initialized. If we do it before, it will likely cause a - # circular dependency since the methods in the `sentry.utils.metrics` depend on the backend - # initialization, thus if you emit metrics when a backend is initialized Python will throw an error. - self._emit(flushable_buckets) - - def _flushable_buckets(self) -> Tuple[FlushableBuckets, bool]: - with self._lock: - force_flush = self._force_flush - cutoff = time.time() - self.ROLLUP_IN_SECONDS - flushable_buckets: Any = [] - weight_to_remove = 0 - - if force_flush: - flushable_buckets = self.buckets.items() - self.buckets = {} - self._buckets_total_weight = 0 - self._force_flush = False - else: - for buckets_timestamp, buckets in self.buckets.items(): - # If the timestamp of the bucket is newer that the rollup we want to skip it. - if buckets_timestamp > cutoff: - continue - - flushable_buckets.append((buckets_timestamp, buckets)) - - # We will clear the elements while holding the lock, in order to avoid requesting it downstream again. - for buckets_timestamp, buckets in flushable_buckets: - for _, metric in buckets.items(): - weight_to_remove += metric.weight - del self.buckets[buckets_timestamp] - - self._buckets_total_weight -= weight_to_remove - - return flushable_buckets, force_flush - - @minimetrics_noop - def add( - self, - ty: MetricType, - key: str, - value: MetricValue, - unit: MetricUnit, - tags: Optional[MetricTagsExternal], - timestamp: Optional[float], - ) -> None: - self._ensure_thread() - - if self._flusher is None: - return - - if timestamp is None: - timestamp = time.time() - - bucket_timestamp = int((timestamp // self.ROLLUP_IN_SECONDS) * self.ROLLUP_IN_SECONDS) - bucket_key = ( - ty, - key, - unit, - # We have to convert tags into our own internal format, since we don't support lists as - # tag values. - self._to_internal_metric_tags(tags), - ) - - with self._lock: - local_buckets = self.buckets.setdefault(bucket_timestamp, {}) - metric = local_buckets.get(bucket_key) - if metric is not None: - previous_weight = metric.weight - metric.add(value) - else: - metric = local_buckets[bucket_key] = METRIC_TYPES[ty](value) - previous_weight = 0 - - self._buckets_total_weight += metric.weight - previous_weight - - # Given the new weight we consider whether we want to force flush. - self.consider_force_flush() - - # We want to track how many times metrics are being added, so that we know the actual count of adds. - metrics.incr( - key="minimetrics.add", - amount=1, - tags={"metric_type": ty}, - sample_rate=self.DEFAULT_SAMPLE_RATE, - ) - - def stop(self): - if self._flusher is None: - return - - # Firstly we tell the flusher that we want to force flush. - with self._lock: - self._force_flush = True - self._running = False - - # Secondly we notify the flusher to move on and we wait for its completion. - self._flush_event.set() - self._flusher.join() - self._flusher = None - - def consider_force_flush(self): - # It's important to acquire a lock around this method, since it will touch shared data structures. - total_weight = len(self.buckets) + self._buckets_total_weight - if total_weight >= self.MAX_WEIGHT: - self._force_flush = True - self._flush_event.set() - - def _emit(self, flushable_buckets: FlushableBuckets) -> Any: - if options.get("delightful_metrics.enable_envelope_forwarding"): - try: - self._transport.send(flushable_buckets) - except Exception as e: - sentry_sdk.capture_exception(e) - - def _to_internal_metric_tags(self, tags: Optional[MetricTagsExternal]) -> MetricTagsInternal: - rv = [] - for key, value in (tags or {}).items(): - # If the value is a collection, we want to flatten it. - if isinstance(value, (list, tuple)): - for inner_value in value: - rv.append((key, inner_value)) - else: - rv.append((key, value)) - - # It's very important to sort the tags in order to obtain the same bucket key. - return tuple(sorted(rv)) - - -class MiniMetricsClient: - def __init__(self) -> None: - self.aggregator = Aggregator() - - def incr( - self, - key: str, - value: float, - unit: MetricUnit = "nanosecond", - tags: Optional[MetricTagsExternal] = None, - timestamp: Optional[float] = None, - ) -> None: - self.aggregator.add("c", key, value, unit, tags, timestamp) - - def timing( - self, - key: str, - value: float, - unit: MetricUnit = "second", - tags: Optional[MetricTagsExternal] = None, - timestamp: Optional[float] = None, - ) -> None: - self.aggregator.add("d", key, value, unit, tags, timestamp) - - def set( - self, - key: str, - value: Union[str, int], - unit: MetricUnit = "none", - tags: Optional[MetricTagsExternal] = None, - timestamp: Optional[float] = None, - ) -> None: - self.aggregator.add("s", key, value, unit, tags, timestamp) - - def gauge( - self, - key: str, - value: float, - unit: MetricUnit = "second", - tags: Optional[MetricTagsExternal] = None, - timestamp: Optional[float] = None, - ) -> None: - # For now, we emit gauges as counts. - self.aggregator.add("c", key, value, unit, tags, timestamp) diff --git a/src/minimetrics/transport.py b/src/minimetrics/transport.py deleted file mode 100644 index d5747eb60bbb..000000000000 --- a/src/minimetrics/transport.py +++ /dev/null @@ -1,130 +0,0 @@ -import re -from functools import partial -from io import BytesIO -from typing import Dict, Iterable, List, Tuple - -import sentry_sdk -from sentry_sdk.envelope import Envelope, Item - -from minimetrics.types import FlushableBuckets, FlushableMetric, MetricType -from sentry import options -from sentry.utils import metrics - - -class EncodingError(Exception): - """ - Raised when the encoding of a flushed metric encounters an error. - """ - - pass - - -sanitize_value = partial(re.compile(r"[^a-zA-Z0-9_/.]").sub, "") - - -class RelayStatsdEncoder: - def _encode(self, value: FlushableMetric, out: BytesIO): - _write = out.write - timestamp, (metric_type, metric_name, metric_unit, metric_tags), metric = value - metric_name = sanitize_value(metric_name) or "invalid-metric-name" - _write(f"{metric_name}@{metric_unit}".encode()) - - for serialized_value in metric.serialize_value(): - _write(b":") - _write(str(serialized_value).encode("utf-8")) - - _write(f"|{metric_type}".encode("ascii")) - - if metric_tags: - _write(b"|#") - first = True - for tag_key, tag_value in metric_tags: - tag_key = sanitize_value(tag_key) - if not tag_key: - continue - if first: - first = False - else: - _write(b",") - _write(tag_key.encode("utf-8")) - _write(b":") - _write(sanitize_value(tag_value).encode("utf-8")) - - _write(f"|T{timestamp}".encode("ascii")) - - def encode_multiple(self, values: Iterable[FlushableMetric]) -> bytes: - out = BytesIO() - _write = out.write - - for value in values: - self._encode(value, out) - _write(b"\n") - - return out.getvalue() - - -class MetricEnvelopeTransport: - def __init__(self, encoder: RelayStatsdEncoder): - self._encoder = encoder - - def send(self, flushable_buckets: FlushableBuckets): - client = sentry_sdk.Hub.current.client - if client is None: - return - - transport = client.transport - if transport is None: - return - - flushable_metrics: List[FlushableMetric] = [] - stats_by_type: Dict[MetricType, Tuple[int, int]] = {} - for buckets_timestamp, buckets in flushable_buckets: - for bucket_key, metric in buckets.items(): - flushable_metric: FlushableMetric = (buckets_timestamp, bucket_key, metric) - flushable_metrics.append(flushable_metric) - (prev_buckets_count, prev_buckets_weight) = stats_by_type.get(bucket_key[0], (0, 0)) - stats_by_type[bucket_key[0]] = ( - prev_buckets_count + 1, - prev_buckets_weight + metric.weight, - ) - - for metric_type, (buckets_count, buckets_weight) in stats_by_type.items(): - # We want to emit a metric on how many buckets and weight there was for a metric type. - metrics.timing( - key="minimetrics.flushed_buckets", - value=buckets_count, - tags={"metric_type": metric_type}, - sample_rate=1.0, - ) - metrics.incr( - key="minimetrics.flushed_buckets_counter", - amount=buckets_count, - tags={"metric_type": metric_type}, - sample_rate=1.0, - ) - metrics.timing( - key="minimetrics.flushed_buckets_weight", - value=buckets_weight, - tags={"metric_type": metric_type}, - sample_rate=1.0, - ) - metrics.incr( - key="minimetrics.flushed_buckets_weight_counter", - amount=buckets_weight, - tags={"metric_type": metric_type}, - sample_rate=1.0, - ) - - if options.get("delightful_metrics.enable_envelope_serialization"): - encoded_metrics = self._encoder.encode_multiple(flushable_metrics) - metrics.timing( - key="minimetrics.encoded_metrics_size", value=len(encoded_metrics), sample_rate=1.0 - ) - metric_item = Item(payload=encoded_metrics, type="statsd") - envelope = Envelope( - headers=None, - items=[metric_item], - ) - - if options.get("delightful_metrics.enable_capture_envelope"): - transport.capture_envelope(envelope) diff --git a/src/minimetrics/types.py b/src/minimetrics/types.py deleted file mode 100644 index e081de444d5d..000000000000 --- a/src/minimetrics/types.py +++ /dev/null @@ -1,83 +0,0 @@ -from typing import ( - Any, - Dict, - Generic, - Iterable, - List, - Literal, - Mapping, - Sequence, - Tuple, - TypeVar, - Union, -) - -# Unit of the metrics. -MetricUnit = Literal[ - "none", - "nanosecond", - "microsecond", - "millisecond", - "second", - "minute", - "hour", - "day", - "week", - "bit", - "byte", - "kilobyte", - "kibibyte", - "mebibyte", - "gigabyte", - "terabyte", - "tebibyte", - "petabyte", - "pebibyte", - "exabyte", - "exbibyte", - "ratio", - "percent", -] - -# Type of the metric. -MetricType = Literal["d", "s", "g", "c"] - -# Value of the metric. -MetricValue = Union[int, float, str] - -# Tag key of a metric. -MetricTagKey = str - -# Internal representation of tags as a tuple of tuples (this is done in order to allow for the same key to exist -# multiple times). -MetricTagValueInternal = str -MetricTagsInternal = Tuple[Tuple[MetricTagKey, MetricTagValueInternal], ...] - -# External representation of tags as a dictionary. -MetricTagValueExternal = Union[str, List[str], Tuple[str, ...]] -MetricTagsExternal = Mapping[MetricTagKey, MetricTagValueExternal] - -# Value inside the generator for the metric value. -FlushedMetricValue = Union[int, float] - -BucketKey = Tuple[MetricType, str, MetricUnit, MetricTagsInternal] - -T = TypeVar("T") - - -class Metric(Generic[T]): - __slots__ = () - - @property - def weight(self) -> int: - raise NotImplementedError() - - def add(self, value: T) -> None: - raise NotImplementedError() - - def serialize_value(self) -> Iterable[FlushedMetricValue]: - raise NotImplementedError() - - -FlushableMetric = Tuple[int, BucketKey, Metric[Any]] -FlushableBuckets = Sequence[Tuple[int, Dict[BucketKey, Metric[Any]]]] diff --git a/src/sentry/metrics/minimetrics.py b/src/sentry/metrics/minimetrics.py index a69dc4323393..e46ace642033 100644 --- a/src/sentry/metrics/minimetrics.py +++ b/src/sentry/metrics/minimetrics.py @@ -1,71 +1,95 @@ import random -from typing import Any, Optional, Union +from functools import wraps +from typing import Any, Dict, Iterable, Optional, Tuple, Union import sentry_sdk -from minimetrics import MetricTagsExternal, MiniMetricsClient -from sentry.metrics.base import MetricsBackend, Tags - - -def _to_minimetrics_external_metric_tags(tags: Optional[Tags]) -> Optional[MetricTagsExternal]: - # We remove all `None` values, since then the types will be compatible. - casted_tags: Any = None - if tags is not None: - casted_tags = { - tag_key: str(tag_value) for tag_key, tag_value in tags.items() if tag_value is not None - } +try: + from sentry_sdk.metrics import Metric, MetricsAggregator - return casted_tags + have_minimetrics = True +except ImportError: + have_minimetrics = False - -# This is needed to pass data between the sdk patcher and the -# minimetrics backend. This is not super clean but it allows us to -# initialize these things in arbitrary order. -minimetrics_client: Optional[MiniMetricsClient] = None +from sentry import options +from sentry.metrics.base import MetricsBackend, Tags +from sentry.utils import metrics def patch_sentry_sdk(): - client = sentry_sdk.Hub.main.client - if client is None: + if not have_minimetrics: return - old_flush = client.flush - - def new_flush(*args, **kwargs): - client = minimetrics_client - if client is not None: - client.aggregator.consider_force_flush() - return old_flush(*args, **kwargs) - - client.flush = new_flush # type:ignore - - old_close = client.close - - def new_close(*args, **kwargs): - client = minimetrics_client - if client is not None: - client.aggregator.stop() - return old_close(*args, **kwargs) - - client.close = new_close # type:ignore - - old_data_category = sentry_sdk.envelope.Item.data_category.fget # type:ignore + real_add = MetricsAggregator.add + real_emit = MetricsAggregator._emit + + @wraps(real_add) + def tracked_add(self, ty, *args, **kwargs): + real_add(self, ty, *args, **kwargs) + metrics.incr( + key="minimetrics.add", + amount=1, + tags={"metric_type": ty}, + sample_rate=1.0, + ) + + @wraps(real_emit) + def patched_emit(self, flushable_buckets: Iterable[Tuple[int, Dict[Any, Metric]]]): + flushable_metrics = [] + stats_by_type = {} + for buckets_timestamp, buckets in flushable_buckets: + for bucket_key, metric in buckets.items(): + flushable_metric = (buckets_timestamp, bucket_key, metric) + flushable_metrics.append(flushable_metric) + (prev_buckets_count, prev_buckets_weight) = stats_by_type.get(bucket_key[0], (0, 0)) + stats_by_type[bucket_key[0]] = ( + prev_buckets_count + 1, + prev_buckets_weight + metric.weight, + ) + + for metric_type, (buckets_count, buckets_weight) in stats_by_type.items(): + metrics.timing( + key="minimetrics.flushed_buckets", + value=buckets_count, + tags={"metric_type": metric_type}, + sample_rate=1.0, + ) + metrics.incr( + key="minimetrics.flushed_buckets_counter", + amount=buckets_count, + tags={"metric_type": metric_type}, + sample_rate=1.0, + ) + metrics.timing( + key="minimetrics.flushed_buckets_weight", + value=buckets_weight, + tags={"metric_type": metric_type}, + sample_rate=1.0, + ) + metrics.incr( + key="minimetrics.flushed_buckets_weight_counter", + amount=buckets_weight, + tags={"metric_type": metric_type}, + sample_rate=1.0, + ) - @property # type:ignore - def data_category(self): - if self.headers.get("type") == "statsd": - return "statsd" - return old_data_category(self) + if options.get("delightful_metrics.enable_capture_envelope"): + envelope = real_emit(self, flushable_buckets) + metrics.timing( + key="minimetrics.encoded_metrics_size", + value=len(envelope.items[0].payload.get_bytes()), + sample_rate=1.0, + ) - sentry_sdk.envelope.Item.data_category = data_category # type:ignore + MetricsAggregator.add = tracked_add + MetricsAggregator._emit = patched_emit class MiniMetricsMetricsBackend(MetricsBackend): def __init__(self, prefix: Optional[str] = None): super().__init__(prefix=prefix) - global minimetrics_client - self.client = MiniMetricsClient() - minimetrics_client = self.client + if not have_minimetrics: + raise RuntimeError("Sentry SDK too old (no minimetrics)") @staticmethod def _keep_metric(sample_rate: float) -> bool: @@ -80,10 +104,10 @@ def incr( sample_rate: float = 1, ) -> None: if self._keep_metric(sample_rate): - self.client.incr( + sentry_sdk.metrics.incr( key=self._get_key(key), value=amount, - tags=_to_minimetrics_external_metric_tags(tags), + tags=tags, ) def timing( @@ -95,9 +119,7 @@ def timing( sample_rate: float = 1, ) -> None: if self._keep_metric(sample_rate): - self.client.timing( - key=self._get_key(key), value=value, tags=_to_minimetrics_external_metric_tags(tags) - ) + sentry_sdk.metrics.distribution(key=self._get_key(key), value=value, tags=tags) def gauge( self, @@ -108,6 +130,5 @@ def gauge( sample_rate: float = 1, ) -> None: if self._keep_metric(sample_rate): - self.client.gauge( - key=self._get_key(key), value=value, tags=_to_minimetrics_external_metric_tags(tags) - ) + # XXX: make this into a gauge later + sentry_sdk.metrics.incr(key=self._get_key(key), value=value, tags=tags) diff --git a/src/sentry/utils/sdk.py b/src/sentry/utils/sdk.py index c39369dd8382..e54ecefa9b0d 100644 --- a/src/sentry/utils/sdk.py +++ b/src/sentry/utils/sdk.py @@ -479,6 +479,9 @@ def flush( "schedule-digests", ] + # turn on minimetrics + sdk_options.setdefault("_experiments", {})["enable_metrics"] = True + sentry_sdk.init( # set back the sentry4sentry_dsn popped above since we need a default dsn on the client # for dynamic sampling context public_key population diff --git a/tests/minimetrics/__init__.py b/tests/minimetrics/__init__.py deleted file mode 100644 index e69de29bb2d1..000000000000 diff --git a/tests/minimetrics/test_core.py b/tests/minimetrics/test_core.py deleted file mode 100644 index a9215fdb1aec..000000000000 --- a/tests/minimetrics/test_core.py +++ /dev/null @@ -1,174 +0,0 @@ -from unittest.mock import patch - -from minimetrics.core import CounterMetric, DistributionMetric, MiniMetricsClient, SetMetric -from sentry.testutils.helpers.datetime import freeze_time -from sentry.testutils.pytest.fixtures import django_db_all - - -@django_db_all -def test_envelope_forwarding(): - client = MiniMetricsClient() - client.incr("button_clicked", 1.0) - client.aggregator.stop() - - assert len(client.aggregator.buckets) == 0 - - -@freeze_time("2023-09-06 10:00:00") -@patch("minimetrics.core.Aggregator._emit") -def test_client_incr(_emit): - tags = { - "browser": "Chrome", - "browser.version": "1.0", - "user.orgs": ["sentry", "google", "apple"], - "user.classes": ["1", "2", "3"], - } - client = MiniMetricsClient() - client.incr("button_clicked", 1.0, tags=tags) # type:ignore - client.aggregator.stop() - - assert len(client.aggregator.buckets) == 0 - emit_args = list(_emit.call_args.args[0]) - assert len(emit_args) == 1 - assert emit_args[0][0] == 1693994400 - keys = list(emit_args[0][1].keys()) - assert keys == [ - ( - "c", - "button_clicked", - "nanosecond", - ( - ("browser", "Chrome"), - ("browser.version", "1.0"), - ("user.classes", "1"), - ("user.classes", "2"), - ("user.classes", "3"), - ("user.orgs", "apple"), - ("user.orgs", "google"), - ("user.orgs", "sentry"), - ), - ) - ] - values = list(emit_args[0][1].values()) - assert isinstance(values[0], CounterMetric) - assert list(values[0].serialize_value()) == [1] - - -@freeze_time("2023-09-06 10:00:00") -@patch("minimetrics.core.Aggregator._emit") -def test_client_timing(_emit): - tags = { - "browser": "Chrome", - "browser.version": "1.0", - "user.orgs": ["sentry", "google", "apple"], - "user.classes": ["1", "2", "3"], - } - client = MiniMetricsClient() - client.timing("execution_time", 1.0, tags=tags) # type:ignore - client.aggregator.stop() - - assert len(client.aggregator.buckets) == 0 - emit_args = list(_emit.call_args.args[0]) - assert len(emit_args) == 1 - assert emit_args[0][0] == 1693994400 - keys = list(emit_args[0][1].keys()) - assert keys == [ - ( - "d", - "execution_time", - "second", - ( - ("browser", "Chrome"), - ("browser.version", "1.0"), - ("user.classes", "1"), - ("user.classes", "2"), - ("user.classes", "3"), - ("user.orgs", "apple"), - ("user.orgs", "google"), - ("user.orgs", "sentry"), - ), - ) - ] - values = list(emit_args[0][1].values()) - assert isinstance(values[0], DistributionMetric) - assert list(values[0].serialize_value()) == [1.0] - - -@freeze_time("2023-09-06 10:00:00") -@patch("minimetrics.core.Aggregator._emit") -def test_client_set(_emit): - tags = { - "browser": "Chrome", - "browser.version": "1.0", - "user.orgs": ["sentry", "google", "apple"], - "user.classes": ["1", "2", "3"], - } - client = MiniMetricsClient() - client.set("user", "riccardo", tags=tags) # type:ignore - client.aggregator.stop() - - assert len(client.aggregator.buckets) == 0 - emit_args = list(_emit.call_args.args[0]) - assert len(emit_args) == 1 - assert emit_args[0][0] == 1693994400 - keys = list(emit_args[0][1].keys()) - assert keys == [ - ( - "s", - "user", - "none", - ( - ("browser", "Chrome"), - ("browser.version", "1.0"), - ("user.classes", "1"), - ("user.classes", "2"), - ("user.classes", "3"), - ("user.orgs", "apple"), - ("user.orgs", "google"), - ("user.orgs", "sentry"), - ), - ) - ] - values = list(emit_args[0][1].values()) - assert isinstance(values[0], SetMetric) - assert list(values[0].serialize_value()) == [3455635177] - - -@freeze_time("2023-09-06 10:00:00") -@patch("minimetrics.core.Aggregator._emit") -def test_client_gauge_as_counter(_emit): - tags = { - "browser": "Chrome", - "browser.version": "1.0", - "user.orgs": ["sentry", "google", "apple"], - "user.classes": ["1", "2", "3"], - } - client = MiniMetricsClient() - client.gauge("frontend_time", 15.0, tags=tags) # type:ignore - client.aggregator.stop() - - assert len(client.aggregator.buckets) == 0 - emit_args = list(_emit.call_args.args[0]) - assert len(emit_args) == 1 - assert emit_args[0][0] == 1693994400 - keys = list(emit_args[0][1].keys()) - assert keys == [ - ( - "c", - "frontend_time", - "second", - ( - ("browser", "Chrome"), - ("browser.version", "1.0"), - ("user.classes", "1"), - ("user.classes", "2"), - ("user.classes", "3"), - ("user.orgs", "apple"), - ("user.orgs", "google"), - ("user.orgs", "sentry"), - ), - ) - ] - values = list(emit_args[0][1].values()) - assert isinstance(values[0], CounterMetric) - assert list(values[0].serialize_value()) == [15.0] diff --git a/tests/minimetrics/test_transport.py b/tests/minimetrics/test_transport.py deleted file mode 100644 index 6a991e198231..000000000000 --- a/tests/minimetrics/test_transport.py +++ /dev/null @@ -1,234 +0,0 @@ -import io -from typing import Any -from unittest.mock import patch - -from minimetrics.core import CounterMetric, DistributionMetric, GaugeMetric, SetMetric -from minimetrics.transport import MetricEnvelopeTransport, RelayStatsdEncoder -from minimetrics.types import BucketKey -from sentry.testutils.helpers import override_options -from sentry.testutils.pytest.fixtures import django_db_all - - -def encode_metric(value): - encoder = RelayStatsdEncoder() - out = io.BytesIO() - encoder._encode(value, out) - return out.getvalue().decode("utf-8") - - -def test_relay_encoder_with_counter(): - bucket_key: BucketKey = ( - "c", - "button_click", - "none", - ( - ("browser", "Chrome"), - ("browser.version", "1.0"), - ), - ) - metric = CounterMetric(first=2) - flushed_metric = (1693994400, bucket_key, metric) - - result = encode_metric(flushed_metric) - assert result == "button_click@none:2|c|#browser:Chrome,browser.version:1.0|T1693994400" - - -def test_relay_encoder_with_distribution(): - bucket_key: BucketKey = ( - "d", - "execution_time", - "second", - ( - ("browser", "Chrome"), - ("browser.version", "1.0"), - ), - ) - metric = DistributionMetric(first=1.0) - metric.add(0.5) - metric.add(3.0) - flushed_metric = (1693994400, bucket_key, metric) - - result = encode_metric(flushed_metric) - assert ( - result - == "execution_time@second:1.0:0.5:3.0|d|#browser:Chrome,browser.version:1.0|T1693994400" - ) - - -def test_relay_encoder_with_set(): - bucket_key: BucketKey = ( - "s", - "users", - "none", - ( - ("browser", "Chrome"), - ("browser.version", "1.0"), - ), - ) - metric = SetMetric(first=123) - metric.add(456) - metric.add("riccardo") - flushed_metric = (1693994400, bucket_key, metric) - - result = encode_metric(flushed_metric) - pieces = result.split("|") - - m = pieces[0].split(":") - assert m[0] == "users@none" - assert sorted(m[1:]) == sorted(["123", "456", "3455635177"]) - - assert pieces[1] == "s" - assert pieces[2] == "#browser:Chrome,browser.version:1.0" - assert pieces[3] == "T1693994400" - - -def test_relay_encoder_with_gauge(): - bucket_key: BucketKey = ( - "g", - "startup_time", - "second", - ( - ("browser", "Chrome"), - ("browser.version", "1.0"), - ), - ) - metric = GaugeMetric(first=10.0) - metric.add(5.0) - metric.add(7.0) - flushed_metric = (1693994400, bucket_key, metric) - - result = encode_metric(flushed_metric) - assert ( - result - == "startup_time@second:7.0:5.0:10.0:22.0:3|g|#browser:Chrome,browser.version:1.0|T1693994400" - ) - - -def test_relay_encoder_with_invalid_chars(): - bucket_key: BucketKey = ( - "c", - "büttòn_click", - "second", - ( - # Invalid tag key. - ("browser\nname", "Chrome"), - # Invalid tag value. - ("browser.version", "\t1.\n0ô"), - # Valid tag key and value. - ("platform", "Android"), - # Totally invalid tag key. - ("\nöś", "Windows"), - # Totally invalid tag value. - ("version", "\n\t"), - ), - ) - metric = CounterMetric(first=1) - flushed_metric = (1693994400, bucket_key, metric) - - result = encode_metric(flushed_metric) - assert ( - result - == "bttn_click@second:1|c|#browsername:Chrome,browser.version:1.0,platform:Android,version:|T1693994400" - ) - - bucket_key = ( - "c", - "üòë", - "second", - (), - ) - metric = CounterMetric(first=1) - flushed_metric = (1693994400, bucket_key, metric) - - assert encode_metric(flushed_metric) == "invalid-metric-name@second:1|c|T1693994400" - - -def test_relay_encoder_with_multiple_metrics(): - encoder = RelayStatsdEncoder() - - flushed_metric_1 = ( - 1693994400, - ( - "g", - "startup_time", - "second", - ( - ("browser", "Chrome"), - ("browser.version", "1.0"), - ), - ), - GaugeMetric(first=10.0), - ) - - flushed_metric_2 = ( - 1693994400, - ( - "c", - "button_click", - "none", - ( - ("browser", "Chrome"), - ("browser.version", "1.0"), - ), - ), - CounterMetric(first=1), - ) - - flushed_metric_3 = ( - 1693994400, - ( - "c", - # This name will be completely scraped, resulting in an invalid metric. - "öüâ", - "none", - ( - ("browser", "Chrome"), - ("browser.version", "1.0"), - ), - ), - CounterMetric(first=1), - ) - - metrics: Any = [flushed_metric_1, flushed_metric_2, flushed_metric_3] - result = encoder.encode_multiple(metrics).decode("utf-8") - - assert result == ( - "startup_time@second:10.0:10.0:10.0:10.0:1|g|#browser:Chrome,browser.version:1.0|T1693994400\n" - "button_click@none:1|c|#browser:Chrome,browser.version:1.0|T1693994400\n" - "invalid-metric-name@none:1|c|#browser:Chrome,browser.version:1.0|T1693994400\n" - ) - - -@override_options( - { - "delightful_metrics.enable_envelope_serialization": True, - "delightful_metrics.enable_capture_envelope": True, - } -) -@patch("minimetrics.transport.sentry_sdk") -@django_db_all -def test_send(sentry_sdk): - flushed_metric = ( - 1693994400, - { - ( - "c", - "button_click", - "none", - ( - ("browser", "Chrome"), - ("browser.version", "1.0"), - ), - ): CounterMetric(first=1), - }, - ) - - transport = MetricEnvelopeTransport(RelayStatsdEncoder()) - metrics: Any = [flushed_metric] - transport.send(metrics) - - args = sentry_sdk.Hub.current.client.transport.capture_envelope.call_args.args - assert len(args) == 1 - arg = args[0] - assert arg.items[0].type == "statsd" - assert arg.items[0].data_category == "statsd" diff --git a/tests/sentry/metrics/test_minimetrics.py b/tests/sentry/metrics/test_minimetrics.py index 74c37da8b452..104c4d3326be 100644 --- a/tests/sentry/metrics/test_minimetrics.py +++ b/tests/sentry/metrics/test_minimetrics.py @@ -1,38 +1,137 @@ -from unittest.mock import patch +import pytest +from sentry_sdk import Client, Hub, Transport -from sentry.metrics.minimetrics import MiniMetricsMetricsBackend -from sentry.testutils.cases import TestCase +from sentry.metrics.minimetrics import MiniMetricsMetricsBackend, have_minimetrics +from sentry.testutils.helpers import override_options -class MiniMetricsMetricsBackendTest(TestCase): - def setUp(self): - self.backend = MiniMetricsMetricsBackend(prefix="sentrytest.") +def parse_metrics(bytes: bytes): + rv = [] + for line in bytes.splitlines(): + pieces = line.decode("utf-8").split("|") + payload = pieces[0].split(":") + name = payload[0] + values = payload[1:] + ty = pieces[1] + ts = None + tags = {} + for piece in pieces[2:]: + if piece[0] == "#": + for pair in piece[1:].split(","): + k, v = pair.split(":", 1) + old = tags.get(k) + if old is not None: + if isinstance(old, list): + old.append(v) + else: + tags[k] = [old, v] + else: + tags[k] = v + elif piece[0] == "T": + ts = int(piece[1:]) + else: + raise ValueError(f"unknown piece {piece!r}") + rv.append((ts, name, ty, values, tags)) + rv.sort(key=lambda x: (x[0], x[1], tuple(sorted(tags.items())))) + return rv - @patch("minimetrics.core.Aggregator.ROLLUP_IN_SECONDS", 1.0) - def test_incr_called_with_no_tags(self): - self.backend.incr(key="foo") - self.backend.client.aggregator.stop() - assert len(self.backend.client.aggregator.buckets) == 0 +class DummyTransport(Transport): + def __init__(self, options): + self.captured = [] - @patch("minimetrics.core.Aggregator.ROLLUP_IN_SECONDS", 1.0) - def test_incr_called_with_tag_value_as_list(self): - # The minimetrics backend supports the list type. - self.backend.incr(key="foo", tags={"foo": ["bar", "baz"]}) # type:ignore - self.backend.client.aggregator.stop() + def capture_envelope(self, envelope): + self.captured.append(envelope) - assert len(self.backend.client.aggregator.buckets) == 0 + def get_metrics(self): + result = [] + for envelope in self.captured: + for item in envelope.items: + if item.headers.get("type") == "statsd": + result.extend(parse_metrics(item.payload.get_bytes())) + result.sort(key=lambda x: (x[0], x[1], x[2])) + return result - @patch("minimetrics.core.Aggregator.ROLLUP_IN_SECONDS", 1.0) - def test_incr_not_called_after_flusher_stopped(self): - self.backend.client.aggregator.stop() - self.backend.incr(key="foo") - assert len(self.backend.client.aggregator.buckets) == 0 +@pytest.fixture(scope="function") +def hub(): + hub = Hub( + Client( + dsn="http://foo@example.invalid/42", + transport=DummyTransport, + _experiments={ + "enable_metrics": True, + }, + ) + ) + with hub: + yield hub - @patch("minimetrics.core.Aggregator.ROLLUP_IN_SECONDS", 1.0) - def test_stop_called_twice(self): - self.backend.client.aggregator.stop() - self.backend.client.aggregator.stop() - assert len(self.backend.client.aggregator.buckets) == 0 +@pytest.fixture(scope="function") +def backend(): + return MiniMetricsMetricsBackend(prefix="sentrytest.") + + +@pytest.mark.skipif(not have_minimetrics, reason="no minimetrics") +@override_options( + { + "delightful_metrics.enable_capture_envelope": True, + } +) +def test_incr_called_with_no_tags(backend, hub): + backend.incr(key="foo", tags={"x": "y"}) + hub.client.flush() + + metrics = hub.client.transport.get_metrics() + + assert len(metrics) == 1 + assert metrics[0][1] == "sentrytest.foo@none" + assert metrics[0][2] == "c" + assert metrics[0][3] == ["1.0"] + assert metrics[0][4]["release"] != "" + assert metrics[0][4]["environment"] != "" + assert metrics[0][4]["x"] == "y" + + assert len(hub.client.metrics_aggregator.buckets) == 0 + + +@pytest.mark.skipif(not have_minimetrics, reason="no minimetrics") +@override_options( + { + "delightful_metrics.enable_capture_envelope": True, + } +) +def test_incr_called_with_tag_value_as_list(backend, hub): + # The minimetrics backend supports the list type. + backend.incr(key="foo", tags={"x": ["bar", "baz"]}) # type: ignore + hub.client.flush() + + metrics = hub.client.transport.get_metrics() + + assert len(metrics) == 1 + assert metrics[0][1] == "sentrytest.foo@none" + assert metrics[0][4]["x"] == ["bar", "baz"] + + assert len(hub.client.metrics_aggregator.buckets) == 0 + + +@pytest.mark.skipif(not have_minimetrics, reason="no minimetrics") +@override_options( + { + "delightful_metrics.enable_capture_envelope": True, + } +) +def test_gauge_as_counter(backend, hub): + # The minimetrics backend supports the list type. + backend.gauge(key="foo", value=42.0) + hub.client.flush() + + metrics = hub.client.transport.get_metrics() + + assert len(metrics) == 1 + assert metrics[0][1] == "sentrytest.foo@none" + assert metrics[0][2] == "c" + assert metrics[0][3] == ["42.0"] + + assert len(hub.client.metrics_aggregator.buckets) == 0 From b9b21e75e3cdec562f68cb4b1eed4e136b795e81 Mon Sep 17 00:00:00 2001 From: Armin Ronacher Date: Thu, 21 Sep 2023 10:18:53 +0200 Subject: [PATCH 02/11] fix types --- src/sentry/metrics/minimetrics.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/sentry/metrics/minimetrics.py b/src/sentry/metrics/minimetrics.py index e46ace642033..7dbe57e698e8 100644 --- a/src/sentry/metrics/minimetrics.py +++ b/src/sentry/metrics/minimetrics.py @@ -36,7 +36,7 @@ def tracked_add(self, ty, *args, **kwargs): @wraps(real_emit) def patched_emit(self, flushable_buckets: Iterable[Tuple[int, Dict[Any, Metric]]]): flushable_metrics = [] - stats_by_type = {} + stats_by_type: Any = {} for buckets_timestamp, buckets in flushable_buckets: for bucket_key, metric in buckets.items(): flushable_metric = (buckets_timestamp, bucket_key, metric) @@ -81,8 +81,8 @@ def patched_emit(self, flushable_buckets: Iterable[Tuple[int, Dict[Any, Metric]] sample_rate=1.0, ) - MetricsAggregator.add = tracked_add - MetricsAggregator._emit = patched_emit + MetricsAggregator.add = tracked_add # type: ignore + MetricsAggregator._emit = patched_emit # type: ignore class MiniMetricsMetricsBackend(MetricsBackend): From eacd3e2bb108018bbbe6e7c04ea61201b8134893 Mon Sep 17 00:00:00 2001 From: Armin Ronacher Date: Thu, 21 Sep 2023 10:30:54 +0200 Subject: [PATCH 03/11] ref: be explicit about the unit in case we change the default still --- src/sentry/metrics/minimetrics.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/sentry/metrics/minimetrics.py b/src/sentry/metrics/minimetrics.py index 7dbe57e698e8..4821155d77be 100644 --- a/src/sentry/metrics/minimetrics.py +++ b/src/sentry/metrics/minimetrics.py @@ -119,7 +119,9 @@ def timing( sample_rate: float = 1, ) -> None: if self._keep_metric(sample_rate): - sentry_sdk.metrics.distribution(key=self._get_key(key), value=value, tags=tags) + sentry_sdk.metrics.distribution( + key=self._get_key(key), value=value, tags=tags, unit="second" + ) def gauge( self, From e4cceda342899f2e1b207a0b068761450ffd56b8 Mon Sep 17 00:00:00 2001 From: Armin Ronacher Date: Thu, 21 Sep 2023 14:47:35 +0200 Subject: [PATCH 04/11] Some typing nonsense --- tests/sentry/metrics/test_minimetrics.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/sentry/metrics/test_minimetrics.py b/tests/sentry/metrics/test_minimetrics.py index 104c4d3326be..8a00ffdf79b6 100644 --- a/tests/sentry/metrics/test_minimetrics.py +++ b/tests/sentry/metrics/test_minimetrics.py @@ -1,3 +1,5 @@ +from typing import Any, Dict + import pytest from sentry_sdk import Client, Hub, Transport @@ -14,7 +16,7 @@ def parse_metrics(bytes: bytes): values = payload[1:] ty = pieces[1] ts = None - tags = {} + tags: Dict[str, Any] = {} for piece in pieces[2:]: if piece[0] == "#": for pair in piece[1:].split(","): @@ -104,7 +106,7 @@ def test_incr_called_with_no_tags(backend, hub): ) def test_incr_called_with_tag_value_as_list(backend, hub): # The minimetrics backend supports the list type. - backend.incr(key="foo", tags={"x": ["bar", "baz"]}) # type: ignore + backend.incr(key="foo", tags={"x": ["bar", "baz"]}) hub.client.flush() metrics = hub.client.transport.get_metrics() From 0ef900eab2a514b6c2a6b8941ec2f683139a5247 Mon Sep 17 00:00:00 2001 From: Armin Ronacher Date: Thu, 21 Sep 2023 14:48:20 +0200 Subject: [PATCH 05/11] Remove minimetrics from makefile --- Makefile | 1 - 1 file changed, 1 deletion(-) diff --git a/Makefile b/Makefile index f2b1edd3d664..5f3f35e3cc17 100644 --- a/Makefile +++ b/Makefile @@ -129,7 +129,6 @@ test-python-ci: create-db @echo "--> Running CI Python tests" pytest \ tests/integration \ - tests/minimetrics \ tests/relay_integration \ tests/sentry \ tests/sentry_plugins \ From 1cb1db2b36c8878f889e36176e9d82dc6bd53728 Mon Sep 17 00:00:00 2001 From: Armin Ronacher Date: Thu, 21 Sep 2023 14:49:19 +0200 Subject: [PATCH 06/11] Make mypy skip over minimetrics.py since the sdk is conditional --- src/sentry/metrics/minimetrics.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/sentry/metrics/minimetrics.py b/src/sentry/metrics/minimetrics.py index 4821155d77be..b60bfb7abaab 100644 --- a/src/sentry/metrics/minimetrics.py +++ b/src/sentry/metrics/minimetrics.py @@ -1,3 +1,5 @@ +# mypy: ignore-errors + import random from functools import wraps from typing import Any, Dict, Iterable, Optional, Tuple, Union From 97edbb4d140e931720dc0f1457a75bbd14989a61 Mon Sep 17 00:00:00 2001 From: Armin Ronacher Date: Thu, 21 Sep 2023 15:08:23 +0200 Subject: [PATCH 07/11] fix: disable check check for experiments --- tests/sentry/metrics/test_minimetrics.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/sentry/metrics/test_minimetrics.py b/tests/sentry/metrics/test_minimetrics.py index 8a00ffdf79b6..6a9093ea3e01 100644 --- a/tests/sentry/metrics/test_minimetrics.py +++ b/tests/sentry/metrics/test_minimetrics.py @@ -62,7 +62,7 @@ def hub(): dsn="http://foo@example.invalid/42", transport=DummyTransport, _experiments={ - "enable_metrics": True, + "enable_metrics": True, # type: ignore }, ) ) From a0bf73337bd4b62a110362c72092666bced99055 Mon Sep 17 00:00:00 2001 From: Armin Ronacher Date: Thu, 21 Sep 2023 15:11:33 +0200 Subject: [PATCH 08/11] Alternative to ignoring type check entirely --- src/sentry/metrics/minimetrics.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/sentry/metrics/minimetrics.py b/src/sentry/metrics/minimetrics.py index b60bfb7abaab..7b3e6e07089e 100644 --- a/src/sentry/metrics/minimetrics.py +++ b/src/sentry/metrics/minimetrics.py @@ -1,8 +1,6 @@ -# mypy: ignore-errors - import random from functools import wraps -from typing import Any, Dict, Iterable, Optional, Tuple, Union +from typing import TYPE_CHECKING, Any, Dict, Iterable, Optional, Tuple, Union import sentry_sdk @@ -12,6 +10,9 @@ have_minimetrics = True except ImportError: have_minimetrics = False + if TYPE_CHECKING: + Metric = Any + MetricsAggregator = Any from sentry import options from sentry.metrics.base import MetricsBackend, Tags From 6dc0fc26e1898887033d584ef89758bb76ff240e Mon Sep 17 00:00:00 2001 From: Armin Ronacher Date: Thu, 21 Sep 2023 15:23:48 +0200 Subject: [PATCH 09/11] Switch back to error ignore --- src/sentry/metrics/minimetrics.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/sentry/metrics/minimetrics.py b/src/sentry/metrics/minimetrics.py index 7b3e6e07089e..04b1c5d37e34 100644 --- a/src/sentry/metrics/minimetrics.py +++ b/src/sentry/metrics/minimetrics.py @@ -1,18 +1,17 @@ +# mypy: ignore-errors + import random from functools import wraps -from typing import TYPE_CHECKING, Any, Dict, Iterable, Optional, Tuple, Union +from typing import Any, Dict, Iterable, Optional, Tuple, Union import sentry_sdk try: - from sentry_sdk.metrics import Metric, MetricsAggregator + from sentry_sdk.metrics import Metric, MetricsAggregator # type: ignore have_minimetrics = True except ImportError: have_minimetrics = False - if TYPE_CHECKING: - Metric = Any - MetricsAggregator = Any from sentry import options from sentry.metrics.base import MetricsBackend, Tags From 883826fcc6583e79a3936153cdc5b1a1bdeea988 Mon Sep 17 00:00:00 2001 From: Armin Ronacher Date: Thu, 21 Sep 2023 15:32:38 +0200 Subject: [PATCH 10/11] feat: Add test to remind of #56651 --- tests/sentry/metrics/test_minimetrics.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/tests/sentry/metrics/test_minimetrics.py b/tests/sentry/metrics/test_minimetrics.py index 6a9093ea3e01..aaf63d616906 100644 --- a/tests/sentry/metrics/test_minimetrics.py +++ b/tests/sentry/metrics/test_minimetrics.py @@ -137,3 +137,15 @@ def test_gauge_as_counter(backend, hub): assert metrics[0][3] == ["42.0"] assert len(hub.client.metrics_aggregator.buckets) == 0 + + +def test_did_you_remove_type_ignore(): + from importlib.metadata import version + + ver = tuple(map(int, version("sentry-sdk").split(".")[:2])) + if ver > (1, 31): + raise RuntimeError( + "Released SDK version with minimetrics support. Please delete " + "this test and follow up instructions in " + "https://github.com/getsentry/sentry/issues/56651" + ) From 416d1fed34890351df23331a10ba1107b320f0d1 Mon Sep 17 00:00:00 2001 From: Armin Ronacher Date: Fri, 22 Sep 2023 08:23:07 +0200 Subject: [PATCH 11/11] Added common tags configuration (release, environment) --- src/sentry/metrics/minimetrics.py | 8 ++++++ src/sentry/options/defaults.py | 6 +++++ src/sentry/utils/sdk.py | 11 +++++--- tests/sentry/metrics/test_minimetrics.py | 34 +++++++++++++++++++++++- 4 files changed, 54 insertions(+), 5 deletions(-) diff --git a/src/sentry/metrics/minimetrics.py b/src/sentry/metrics/minimetrics.py index 04b1c5d37e34..d107d266bf4b 100644 --- a/src/sentry/metrics/minimetrics.py +++ b/src/sentry/metrics/minimetrics.py @@ -87,6 +87,14 @@ def patched_emit(self, flushable_buckets: Iterable[Tuple[int, Dict[Any, Metric]] MetricsAggregator._emit = patched_emit # type: ignore +def before_emit_metric(key: str, tags: Dict[str, Any]) -> bool: + if not options.get("delightful_metrics.enable_common_tags"): + tags.pop("transaction", None) + tags.pop("release", None) + tags.pop("environment", None) + return True + + class MiniMetricsMetricsBackend(MetricsBackend): def __init__(self, prefix: Optional[str] = None): super().__init__(prefix=prefix) diff --git a/src/sentry/options/defaults.py b/src/sentry/options/defaults.py index 94a918f1a3fb..3457c466663d 100644 --- a/src/sentry/options/defaults.py +++ b/src/sentry/options/defaults.py @@ -1540,6 +1540,12 @@ flags=FLAG_AUTOMATOR_MODIFIABLE, ) +register( + "delightful_metrics.enable_common_tags", + default=False, + flags=FLAG_AUTOMATOR_MODIFIABLE, +) + register( "outbox_replication.sentry_team.replication_version", type=Int, diff --git a/src/sentry/utils/sdk.py b/src/sentry/utils/sdk.py index e54ecefa9b0d..ba7e29d22b23 100644 --- a/src/sentry/utils/sdk.py +++ b/src/sentry/utils/sdk.py @@ -472,6 +472,8 @@ def flush( from sentry_sdk.integrations.redis import RedisIntegration from sentry_sdk.integrations.threading import ThreadingIntegration + from sentry.metrics import minimetrics + # exclude monitors with sub-minute schedules from using crons exclude_beat_tasks = [ "flush-buffers", @@ -480,7 +482,10 @@ def flush( ] # turn on minimetrics - sdk_options.setdefault("_experiments", {})["enable_metrics"] = True + sdk_options.setdefault("_experiments", {}).update( + enable_metrics=True, + before_emit_metric=minimetrics.before_emit_metric, + ) sentry_sdk.init( # set back the sentry4sentry_dsn popped above since we need a default dsn on the client @@ -503,9 +508,7 @@ def flush( **sdk_options, ) - from sentry.metrics.minimetrics import patch_sentry_sdk - - patch_sentry_sdk() + minimetrics.patch_sentry_sdk() class RavenShim: diff --git a/tests/sentry/metrics/test_minimetrics.py b/tests/sentry/metrics/test_minimetrics.py index aaf63d616906..4cd38f65a61b 100644 --- a/tests/sentry/metrics/test_minimetrics.py +++ b/tests/sentry/metrics/test_minimetrics.py @@ -3,7 +3,11 @@ import pytest from sentry_sdk import Client, Hub, Transport -from sentry.metrics.minimetrics import MiniMetricsMetricsBackend, have_minimetrics +from sentry.metrics.minimetrics import ( + MiniMetricsMetricsBackend, + before_emit_metric, + have_minimetrics, +) from sentry.testutils.helpers import override_options @@ -63,6 +67,7 @@ def hub(): transport=DummyTransport, _experiments={ "enable_metrics": True, # type: ignore + "before_emit_metric": before_emit_metric, }, ) ) @@ -79,6 +84,7 @@ def backend(): @override_options( { "delightful_metrics.enable_capture_envelope": True, + "delightful_metrics.enable_common_tags": True, } ) def test_incr_called_with_no_tags(backend, hub): @@ -102,6 +108,31 @@ def test_incr_called_with_no_tags(backend, hub): @override_options( { "delightful_metrics.enable_capture_envelope": True, + "delightful_metrics.enable_common_tags": False, + } +) +def test_incr_called_with_no_tags_and_no_common_tags(backend, hub): + backend.incr(key="foo", tags={"x": "y"}) + hub.client.flush() + + metrics = hub.client.transport.get_metrics() + + assert len(metrics) == 1 + assert metrics[0][1] == "sentrytest.foo@none" + assert metrics[0][2] == "c" + assert metrics[0][3] == ["1.0"] + assert metrics[0][4].get("release") is None + assert metrics[0][4].get("environment") is None + assert metrics[0][4]["x"] == "y" + + assert len(hub.client.metrics_aggregator.buckets) == 0 + + +@pytest.mark.skipif(not have_minimetrics, reason="no minimetrics") +@override_options( + { + "delightful_metrics.enable_capture_envelope": True, + "delightful_metrics.enable_common_tags": True, } ) def test_incr_called_with_tag_value_as_list(backend, hub): @@ -122,6 +153,7 @@ def test_incr_called_with_tag_value_as_list(backend, hub): @override_options( { "delightful_metrics.enable_capture_envelope": True, + "delightful_metrics.enable_common_tags": True, } ) def test_gauge_as_counter(backend, hub):