From 8cda345b2def4829ffc8c7cc133a1b5bf4385475 Mon Sep 17 00:00:00 2001 From: Ivana Kellyerova Date: Wed, 10 Apr 2024 11:08:07 +0200 Subject: [PATCH 1/4] feat(metrics): Add value, unit to before_emit_metric --- sentry_sdk/consts.py | 5 ++++- sentry_sdk/metrics.py | 31 ++++++++++++++++++++++--------- tests/test_metrics.py | 7 +++++-- 3 files changed, 31 insertions(+), 12 deletions(-) diff --git a/sentry_sdk/consts.py b/sentry_sdk/consts.py index 047cb1384c..67ee047451 100644 --- a/sentry_sdk/consts.py +++ b/sentry_sdk/consts.py @@ -24,6 +24,7 @@ Event, EventProcessor, Hint, + MeasurementUnit, ProfilerMode, TracesSampler, TransactionProcessor, @@ -49,7 +50,9 @@ "enable_metrics": Optional[bool], "metrics_summary_sample_rate": Optional[float], "should_summarize_metric": Optional[Callable[[str, MetricTags], bool]], - "before_emit_metric": Optional[Callable[[str, MetricTags], bool]], + "before_emit_metric": Optional[ + Callable[[str, MetricTags, Any, MeasurementUnit], bool] + ], "metric_code_locations": Optional[bool], }, total=False, diff --git a/sentry_sdk/metrics.py b/sentry_sdk/metrics.py index 2b030e9fe1..010698b20f 100644 --- a/sentry_sdk/metrics.py +++ b/sentry_sdk/metrics.py @@ -703,8 +703,8 @@ def _get_aggregator(): ) -def _get_aggregator_and_update_tags(key, tags): - # type: (str, Optional[MetricTags]) -> Tuple[Optional[MetricsAggregator], Optional[LocalAggregator], Optional[MetricTags]] +def _get_aggregator_and_update_tags(key, tags, value, unit): + # type: (str, Optional[MetricTags], Any, MeasurementUnit) -> Tuple[Optional[MetricsAggregator], Optional[LocalAggregator], Optional[MetricTags]] hub = sentry_sdk.Hub.current client = hub.client if client is None or client.metrics_aggregator is None: @@ -745,7 +745,7 @@ def _get_aggregator_and_update_tags(key, tags): if before_emit_callback is not None: with recursion_protection() as in_metrics: if not in_metrics: - if not before_emit_callback(key, updated_tags): + if not before_emit_callback(key, updated_tags, value, unit): return None, None, updated_tags return client.metrics_aggregator, local_aggregator, updated_tags @@ -761,7 +761,9 @@ def increment( ): # type: (...) -> None """Increments a counter.""" - aggregator, local_aggregator, tags = _get_aggregator_and_update_tags(key, tags) + aggregator, local_aggregator, tags = _get_aggregator_and_update_tags( + key, tags, value, unit + ) if aggregator is not None: aggregator.add( "c", key, value, unit, tags, timestamp, local_aggregator, stacklevel @@ -822,7 +824,10 @@ def __exit__(self, exc_type, exc_value, tb): # type: (Any, Any, Any) -> None assert self._span, "did not enter" aggregator, local_aggregator, tags = _get_aggregator_and_update_tags( - self.key, self.tags + self.key, + self.tags, + self.value, + self.unit, ) if aggregator is not None: elapsed = TIMING_FUNCTIONS[self.unit]() - self.entered # type: ignore @@ -877,7 +882,9 @@ def timing( - it can be used as a decorator """ if value is not None: - aggregator, local_aggregator, tags = _get_aggregator_and_update_tags(key, tags) + aggregator, local_aggregator, tags = _get_aggregator_and_update_tags( + key, tags, value, unit + ) if aggregator is not None: aggregator.add( "d", key, value, unit, tags, timestamp, local_aggregator, stacklevel @@ -895,7 +902,9 @@ def distribution( ): # type: (...) -> None """Emits a distribution.""" - aggregator, local_aggregator, tags = _get_aggregator_and_update_tags(key, tags) + aggregator, local_aggregator, tags = _get_aggregator_and_update_tags( + key, tags, value, unit + ) if aggregator is not None: aggregator.add( "d", key, value, unit, tags, timestamp, local_aggregator, stacklevel @@ -912,7 +921,9 @@ def set( ): # type: (...) -> None """Emits a set.""" - aggregator, local_aggregator, tags = _get_aggregator_and_update_tags(key, tags) + aggregator, local_aggregator, tags = _get_aggregator_and_update_tags( + key, tags, value, unit + ) if aggregator is not None: aggregator.add( "s", key, value, unit, tags, timestamp, local_aggregator, stacklevel @@ -929,7 +940,9 @@ def gauge( ): # type: (...) -> None """Emits a gauge.""" - aggregator, local_aggregator, tags = _get_aggregator_and_update_tags(key, tags) + aggregator, local_aggregator, tags = _get_aggregator_and_update_tags( + key, tags, value, unit + ) if aggregator is not None: aggregator.add( "g", key, value, unit, tags, timestamp, local_aggregator, stacklevel diff --git a/tests/test_metrics.py b/tests/test_metrics.py index d9b26b52a6..c4cc1e4bc2 100644 --- a/tests/test_metrics.py +++ b/tests/test_metrics.py @@ -854,9 +854,10 @@ def test_tag_normalization( def test_before_emit_metric( sentry_init, capture_envelopes, maybe_monkeypatched_threading ): - def before_emit(key, tags): - if key == "removed-metric": + def before_emit(key, tags, value, unit): + if key == "removed-metric" or value == 47 or unit == "unsupported": return False + tags["extra"] = "foo" del tags["release"] # this better be a noop! @@ -875,6 +876,8 @@ def before_emit(key, tags): envelopes = capture_envelopes() metrics.increment("removed-metric", 1.0) + metrics.increment("another-removed-metric", 47) + metrics.increment("yet-another-removed-metric", 1.0, unit="unsupported") metrics.increment("actual-metric", 1.0) Hub.current.flush() From 268fe4d1a9ec553d19909b6388304019e3f5c329 Mon Sep 17 00:00:00 2001 From: Ivana Kellyerova Date: Wed, 10 Apr 2024 11:10:54 +0200 Subject: [PATCH 2/4] better order --- sentry_sdk/consts.py | 2 +- sentry_sdk/metrics.py | 18 +++++++++--------- tests/test_metrics.py | 2 +- 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/sentry_sdk/consts.py b/sentry_sdk/consts.py index 67ee047451..9b8ad85eeb 100644 --- a/sentry_sdk/consts.py +++ b/sentry_sdk/consts.py @@ -51,7 +51,7 @@ "metrics_summary_sample_rate": Optional[float], "should_summarize_metric": Optional[Callable[[str, MetricTags], bool]], "before_emit_metric": Optional[ - Callable[[str, MetricTags, Any, MeasurementUnit], bool] + Callable[[str, Any, MeasurementUnit, MetricTags], bool] ], "metric_code_locations": Optional[bool], }, diff --git a/sentry_sdk/metrics.py b/sentry_sdk/metrics.py index 010698b20f..4af7259f20 100644 --- a/sentry_sdk/metrics.py +++ b/sentry_sdk/metrics.py @@ -703,8 +703,8 @@ def _get_aggregator(): ) -def _get_aggregator_and_update_tags(key, tags, value, unit): - # type: (str, Optional[MetricTags], Any, MeasurementUnit) -> Tuple[Optional[MetricsAggregator], Optional[LocalAggregator], Optional[MetricTags]] +def _get_aggregator_and_update_tags(key, value, unit, tags): + # type: (str, Any, MeasurementUnit, Optional[MetricTags]) -> Tuple[Optional[MetricsAggregator], Optional[LocalAggregator], Optional[MetricTags]] hub = sentry_sdk.Hub.current client = hub.client if client is None or client.metrics_aggregator is None: @@ -745,7 +745,7 @@ def _get_aggregator_and_update_tags(key, tags, value, unit): if before_emit_callback is not None: with recursion_protection() as in_metrics: if not in_metrics: - if not before_emit_callback(key, updated_tags, value, unit): + if not before_emit_callback(key, value, unit, updated_tags): return None, None, updated_tags return client.metrics_aggregator, local_aggregator, updated_tags @@ -762,7 +762,7 @@ def increment( # type: (...) -> None """Increments a counter.""" aggregator, local_aggregator, tags = _get_aggregator_and_update_tags( - key, tags, value, unit + key, value, unit, tags ) if aggregator is not None: aggregator.add( @@ -825,9 +825,9 @@ def __exit__(self, exc_type, exc_value, tb): assert self._span, "did not enter" aggregator, local_aggregator, tags = _get_aggregator_and_update_tags( self.key, - self.tags, self.value, self.unit, + self.tags, ) if aggregator is not None: elapsed = TIMING_FUNCTIONS[self.unit]() - self.entered # type: ignore @@ -883,7 +883,7 @@ def timing( """ if value is not None: aggregator, local_aggregator, tags = _get_aggregator_and_update_tags( - key, tags, value, unit + key, value, unit, tags ) if aggregator is not None: aggregator.add( @@ -903,7 +903,7 @@ def distribution( # type: (...) -> None """Emits a distribution.""" aggregator, local_aggregator, tags = _get_aggregator_and_update_tags( - key, tags, value, unit + key, value, unit, tags ) if aggregator is not None: aggregator.add( @@ -922,7 +922,7 @@ def set( # type: (...) -> None """Emits a set.""" aggregator, local_aggregator, tags = _get_aggregator_and_update_tags( - key, tags, value, unit + key, value, unit, tags ) if aggregator is not None: aggregator.add( @@ -941,7 +941,7 @@ def gauge( # type: (...) -> None """Emits a gauge.""" aggregator, local_aggregator, tags = _get_aggregator_and_update_tags( - key, tags, value, unit + key, value, unit, tags ) if aggregator is not None: aggregator.add( diff --git a/tests/test_metrics.py b/tests/test_metrics.py index c4cc1e4bc2..d6653230a0 100644 --- a/tests/test_metrics.py +++ b/tests/test_metrics.py @@ -854,7 +854,7 @@ def test_tag_normalization( def test_before_emit_metric( sentry_init, capture_envelopes, maybe_monkeypatched_threading ): - def before_emit(key, tags, value, unit): + def before_emit(key, value, unit, tags): if key == "removed-metric" or value == 47 or unit == "unsupported": return False From 385a19ca52a06a07a9e94b8a0e257792ad5424d4 Mon Sep 17 00:00:00 2001 From: Ivana Kellyerova Date: Wed, 10 Apr 2024 11:53:13 +0200 Subject: [PATCH 3/4] fix types --- sentry_sdk/consts.py | 3 ++- sentry_sdk/metrics.py | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/sentry_sdk/consts.py b/sentry_sdk/consts.py index 9b8ad85eeb..b57cf0979c 100644 --- a/sentry_sdk/consts.py +++ b/sentry_sdk/consts.py @@ -29,6 +29,7 @@ TracesSampler, TransactionProcessor, MetricTags, + MetricValue, ) # Experiments are feature flags to enable and disable certain unstable SDK @@ -51,7 +52,7 @@ "metrics_summary_sample_rate": Optional[float], "should_summarize_metric": Optional[Callable[[str, MetricTags], bool]], "before_emit_metric": Optional[ - Callable[[str, Any, MeasurementUnit, MetricTags], bool] + Callable[[str, MetricValue, MeasurementUnit, MetricTags], bool] ], "metric_code_locations": Optional[bool], }, diff --git a/sentry_sdk/metrics.py b/sentry_sdk/metrics.py index 4af7259f20..ef77814bea 100644 --- a/sentry_sdk/metrics.py +++ b/sentry_sdk/metrics.py @@ -704,7 +704,7 @@ def _get_aggregator(): def _get_aggregator_and_update_tags(key, value, unit, tags): - # type: (str, Any, MeasurementUnit, Optional[MetricTags]) -> Tuple[Optional[MetricsAggregator], Optional[LocalAggregator], Optional[MetricTags]] + # type: (str, MetricValue, MeasurementUnit, Optional[MetricTags]) -> Tuple[Optional[MetricsAggregator], Optional[LocalAggregator], Optional[MetricTags]] hub = sentry_sdk.Hub.current client = hub.client if client is None or client.metrics_aggregator is None: From ccf29a1e5104bf716d0b9fc3e3227816d4128556 Mon Sep 17 00:00:00 2001 From: Ivana Kellyerova Date: Wed, 10 Apr 2024 13:02:55 +0200 Subject: [PATCH 4/4] fix type --- sentry_sdk/metrics.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sentry_sdk/metrics.py b/sentry_sdk/metrics.py index aa3f8b5ffc..57f44e6533 100644 --- a/sentry_sdk/metrics.py +++ b/sentry_sdk/metrics.py @@ -704,7 +704,7 @@ def _get_aggregator(): def _get_aggregator_and_update_tags(key, value, unit, tags): - # type: (str, MetricValue, MeasurementUnit, Optional[MetricTags]) -> Tuple[Optional[MetricsAggregator], Optional[LocalAggregator], Optional[MetricTags]] + # type: (str, Optional[MetricValue], MeasurementUnit, Optional[MetricTags]) -> Tuple[Optional[MetricsAggregator], Optional[LocalAggregator], Optional[MetricTags]] hub = sentry_sdk.Hub.current client = hub.client if client is None or client.metrics_aggregator is None: