From f213d6bec40c083f3fe21d30c93c2c0d7bd52507 Mon Sep 17 00:00:00 2001 From: Neel Shah Date: Fri, 4 Mar 2022 15:29:38 +0100 Subject: [PATCH 1/9] feat(measurements): Add experimental set_measurement api on transaction --- sentry_sdk/_types.py | 1 + sentry_sdk/consts.py | 1 + sentry_sdk/tracing.py | 45 +++++++++++++++++++++++++++---------- sentry_sdk/tracing_utils.py | 7 ++++++ tests/tracing/test_misc.py | 15 +++++++++++++ 5 files changed, 57 insertions(+), 12 deletions(-) diff --git a/sentry_sdk/_types.py b/sentry_sdk/_types.py index 7ce7e9e4f6..6e66ad91ef 100644 --- a/sentry_sdk/_types.py +++ b/sentry_sdk/_types.py @@ -48,3 +48,4 @@ ] SessionStatus = Literal["ok", "exited", "crashed", "abnormal"] EndpointType = Literal["store", "envelope"] + MeasurementUnit = Literal["ns", "ms", "s"] diff --git a/sentry_sdk/consts.py b/sentry_sdk/consts.py index 1418081511..ae808c64ee 100644 --- a/sentry_sdk/consts.py +++ b/sentry_sdk/consts.py @@ -33,6 +33,7 @@ "record_sql_params": Optional[bool], "smart_transaction_trimming": Optional[bool], "propagate_tracestate": Optional[bool], + "custom_measurements": Optional[bool], }, total=False, ) diff --git a/sentry_sdk/tracing.py b/sentry_sdk/tracing.py index 1b5b65e1af..fa449d2883 100644 --- a/sentry_sdk/tracing.py +++ b/sentry_sdk/tracing.py @@ -8,6 +8,7 @@ from sentry_sdk.utils import logger from sentry_sdk._types import MYPY +from sentry_sdk.tracing_utils import has_custom_measurements_enabled if MYPY: @@ -20,7 +21,7 @@ from typing import Tuple from typing import Iterator - from sentry_sdk._types import SamplingContext + from sentry_sdk._types import SamplingContext, MeasurementUnit class _SpanRecorder(object): @@ -487,6 +488,7 @@ class Transaction(Span): "_sentry_tracestate", # tracestate data from other vendors, of the form `dogs=yes,cats=maybe` "_third_party_tracestate", + "_measurements", ) def __init__( @@ -515,6 +517,7 @@ def __init__( # first time an event needs it for inclusion in the captured data self._sentry_tracestate = sentry_tracestate self._third_party_tracestate = third_party_tracestate + self._measurements = {} def __repr__(self): # type: () -> str @@ -594,17 +597,35 @@ def finish(self, hub=None): # to be garbage collected self._span_recorder = None - return hub.capture_event( - { - "type": "transaction", - "transaction": self.name, - "contexts": {"trace": self.get_trace_context()}, - "tags": self._tags, - "timestamp": self.timestamp, - "start_timestamp": self.start_timestamp, - "spans": finished_spans, - } - ) + event = { + "type": "transaction", + "transaction": self.name, + "contexts": {"trace": self.get_trace_context()}, + "tags": self._tags, + "timestamp": self.timestamp, + "start_timestamp": self.start_timestamp, + "spans": finished_spans, + } + + + if has_custom_measurements_enabled(): + event["measurements"] = self._measurements + + return hub.capture_event(event) + + + def set_measurement(self, name, value, unit="ms"): + # type: (str, float, MeasurementUnit) -> None + if not has_custom_measurements_enabled(): + logger.debug("[Tracing] Experimental custom_measurements feature is disabled") + return + + if unit not in ("ns", "ms", "s"): + logger.debug("[Tracing] Discarding measurement due to invalid unit") + return + + self._measurements[name] = { "value": value, "unit": unit } + def to_json(self): # type: () -> Dict[str, Any] diff --git a/sentry_sdk/tracing_utils.py b/sentry_sdk/tracing_utils.py index faed37cbb7..2d31b9903e 100644 --- a/sentry_sdk/tracing_utils.py +++ b/sentry_sdk/tracing_utils.py @@ -406,6 +406,13 @@ def has_tracestate_enabled(span=None): return bool(options and options["_experiments"].get("propagate_tracestate")) +def has_custom_measurements_enabled(): + # type: () -> bool + client = sentry_sdk.Hub.current.client + options = client and client.options + return bool(options and options["_experiments"].get("custom_measurements")) + + # Circular imports if MYPY: diff --git a/tests/tracing/test_misc.py b/tests/tracing/test_misc.py index 5d6613cd28..41b1f518b9 100644 --- a/tests/tracing/test_misc.py +++ b/tests/tracing/test_misc.py @@ -246,3 +246,18 @@ def test_has_tracestate_enabled(sentry_init, tracestate_enabled): assert has_tracestate_enabled() is True else: assert has_tracestate_enabled() is False + + +def test_set_meaurement(sentry_init, capture_events): + sentry_init(traces_sample_rate=1.0, _experiments={"custom_measurements": True}) + + events = capture_events() + + transaction = start_transaction(name="measuring stuff") + transaction.set_measurement("metric.foo", 123) + transaction.set_measurement("metric.bar", 456, unit="s") + transaction.finish() + + (event,) = events + assert event["measurements"]["metric.foo"] == { "value": 123, "unit": "ms" } + assert event["measurements"]["metric.bar"] == { "value": 456, "unit": "s" } From 752337e24fc1c84a4263d5ab5d4f3d000f6a64b8 Mon Sep 17 00:00:00 2001 From: Neel Shah Date: Fri, 4 Mar 2022 15:36:31 +0100 Subject: [PATCH 2/9] Lint --- sentry_sdk/tracing.py | 9 ++++----- tests/tracing/test_misc.py | 4 ++-- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/sentry_sdk/tracing.py b/sentry_sdk/tracing.py index fa449d2883..b3ca49e63b 100644 --- a/sentry_sdk/tracing.py +++ b/sentry_sdk/tracing.py @@ -607,25 +607,24 @@ def finish(self, hub=None): "spans": finished_spans, } - if has_custom_measurements_enabled(): event["measurements"] = self._measurements return hub.capture_event(event) - def set_measurement(self, name, value, unit="ms"): # type: (str, float, MeasurementUnit) -> None if not has_custom_measurements_enabled(): - logger.debug("[Tracing] Experimental custom_measurements feature is disabled") + logger.debug( + "[Tracing] Experimental custom_measurements feature is disabled" + ) return if unit not in ("ns", "ms", "s"): logger.debug("[Tracing] Discarding measurement due to invalid unit") return - self._measurements[name] = { "value": value, "unit": unit } - + self._measurements[name] = {"value": value, "unit": unit} def to_json(self): # type: () -> Dict[str, Any] diff --git a/tests/tracing/test_misc.py b/tests/tracing/test_misc.py index 41b1f518b9..3b4102c294 100644 --- a/tests/tracing/test_misc.py +++ b/tests/tracing/test_misc.py @@ -259,5 +259,5 @@ def test_set_meaurement(sentry_init, capture_events): transaction.finish() (event,) = events - assert event["measurements"]["metric.foo"] == { "value": 123, "unit": "ms" } - assert event["measurements"]["metric.bar"] == { "value": 456, "unit": "s" } + assert event["measurements"]["metric.foo"] == {"value": 123, "unit": "ms"} + assert event["measurements"]["metric.bar"] == {"value": 456, "unit": "s"} From 599fe386b2a6a62564b4f3cebb7dbc6b2ce78959 Mon Sep 17 00:00:00 2001 From: Neel Shah Date: Fri, 4 Mar 2022 15:42:02 +0100 Subject: [PATCH 3/9] Type --- sentry_sdk/tracing.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sentry_sdk/tracing.py b/sentry_sdk/tracing.py index b3ca49e63b..b8fdc4bb14 100644 --- a/sentry_sdk/tracing.py +++ b/sentry_sdk/tracing.py @@ -517,7 +517,7 @@ def __init__( # first time an event needs it for inclusion in the captured data self._sentry_tracestate = sentry_tracestate self._third_party_tracestate = third_party_tracestate - self._measurements = {} + self._measurements = {} # type: Dict[str, Any] def __repr__(self): # type: () -> str From 887e85ce711d8bf8938039372806b035dfe6bd04 Mon Sep 17 00:00:00 2001 From: Neel Shah Date: Fri, 4 Mar 2022 15:45:29 +0100 Subject: [PATCH 4/9] Lint again --- sentry_sdk/tracing.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sentry_sdk/tracing.py b/sentry_sdk/tracing.py index b8fdc4bb14..9e44c0405a 100644 --- a/sentry_sdk/tracing.py +++ b/sentry_sdk/tracing.py @@ -517,7 +517,7 @@ def __init__( # first time an event needs it for inclusion in the captured data self._sentry_tracestate = sentry_tracestate self._third_party_tracestate = third_party_tracestate - self._measurements = {} # type: Dict[str, Any] + self._measurements = {} # type: Dict[str, Any] def __repr__(self): # type: () -> str From 9634361ab8a1b8c8c3e88ebf11369eb2c7e4eb3f Mon Sep 17 00:00:00 2001 From: Neel Shah Date: Fri, 4 Mar 2022 16:16:57 +0100 Subject: [PATCH 5/9] Fix circular import --- sentry_sdk/tracing.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sentry_sdk/tracing.py b/sentry_sdk/tracing.py index 9e44c0405a..371e39b62a 100644 --- a/sentry_sdk/tracing.py +++ b/sentry_sdk/tracing.py @@ -8,7 +8,6 @@ from sentry_sdk.utils import logger from sentry_sdk._types import MYPY -from sentry_sdk.tracing_utils import has_custom_measurements_enabled if MYPY: @@ -747,4 +746,5 @@ def _set_initial_sampling_decision(self, sampling_context): has_tracing_enabled, is_valid_sample_rate, maybe_create_breadcrumbs_from_span, + has_custom_measurements_enabled, ) From 9dd1a481228aabe5cba7286a4d91fdfd8f50bb09 Mon Sep 17 00:00:00 2001 From: Neel Shah Date: Fri, 4 Mar 2022 16:19:29 +0100 Subject: [PATCH 6/9] Pre-commit and ci don't agree --- sentry_sdk/tracing.py | 38 ++++++++++++++++---------------------- 1 file changed, 16 insertions(+), 22 deletions(-) diff --git a/sentry_sdk/tracing.py b/sentry_sdk/tracing.py index 371e39b62a..45cf98d5a5 100644 --- a/sentry_sdk/tracing.py +++ b/sentry_sdk/tracing.py @@ -132,17 +132,14 @@ def init_span_recorder(self, maxlen): def __repr__(self): # type: () -> str - return ( - "<%s(op=%r, description:%r, trace_id=%r, span_id=%r, parent_span_id=%r, sampled=%r)>" - % ( - self.__class__.__name__, - self.op, - self.description, - self.trace_id, - self.span_id, - self.parent_span_id, - self.sampled, - ) + return "<%s(op=%r, description:%r, trace_id=%r, span_id=%r, parent_span_id=%r, sampled=%r)>" % ( + self.__class__.__name__, + self.op, + self.description, + self.trace_id, + self.span_id, + self.parent_span_id, + self.sampled, ) def __enter__(self): @@ -520,17 +517,14 @@ def __init__( def __repr__(self): # type: () -> str - return ( - "<%s(name=%r, op=%r, trace_id=%r, span_id=%r, parent_span_id=%r, sampled=%r)>" - % ( - self.__class__.__name__, - self.name, - self.op, - self.trace_id, - self.span_id, - self.parent_span_id, - self.sampled, - ) + return "<%s(name=%r, op=%r, trace_id=%r, span_id=%r, parent_span_id=%r, sampled=%r)>" % ( + self.__class__.__name__, + self.name, + self.op, + self.trace_id, + self.span_id, + self.parent_span_id, + self.sampled, ) @property From 822ad0f4148a55eba95e4f4564fb2735e8bd0f18 Mon Sep 17 00:00:00 2001 From: Neel Shah Date: Thu, 5 May 2022 14:31:12 +0200 Subject: [PATCH 7/9] Make types consistent with relay changes --- sentry_sdk/_types.py | 32 +++++++++++++++++++++++++++++++- sentry_sdk/tracing.py | 6 +----- tests/tracing/test_misc.py | 8 +++++--- 3 files changed, 37 insertions(+), 9 deletions(-) diff --git a/sentry_sdk/_types.py b/sentry_sdk/_types.py index 6e66ad91ef..59970ad60a 100644 --- a/sentry_sdk/_types.py +++ b/sentry_sdk/_types.py @@ -48,4 +48,34 @@ ] SessionStatus = Literal["ok", "exited", "crashed", "abnormal"] EndpointType = Literal["store", "envelope"] - MeasurementUnit = Literal["ns", "ms", "s"] + + DurationUnit = Literal[ + "nanosecond", + "microsecond", + "millisecond", + "second", + "minute", + "hour", + "day", + "week", + ] + + InformationUnit = Literal[ + "bit", + "byte", + "kilobyte", + "kibibyte", + "megabyte", + "mebibyte", + "gigabyte", + "gibibyte", + "terabyte", + "tebibyte", + "petabyte", + "pebibyte", + "exabyte", + "exbibyte", + ] + + FractionUnit = Literal["ratio", "percent"] + MeasurementUnit = Union[DurationUnit, InformationUnit, FractionUnit, str] diff --git a/sentry_sdk/tracing.py b/sentry_sdk/tracing.py index 45cf98d5a5..17b94be90f 100644 --- a/sentry_sdk/tracing.py +++ b/sentry_sdk/tracing.py @@ -605,7 +605,7 @@ def finish(self, hub=None): return hub.capture_event(event) - def set_measurement(self, name, value, unit="ms"): + def set_measurement(self, name, value, unit=""): # type: (str, float, MeasurementUnit) -> None if not has_custom_measurements_enabled(): logger.debug( @@ -613,10 +613,6 @@ def set_measurement(self, name, value, unit="ms"): ) return - if unit not in ("ns", "ms", "s"): - logger.debug("[Tracing] Discarding measurement due to invalid unit") - return - self._measurements[name] = {"value": value, "unit": unit} def to_json(self): diff --git a/tests/tracing/test_misc.py b/tests/tracing/test_misc.py index 3b4102c294..dd47f25220 100644 --- a/tests/tracing/test_misc.py +++ b/tests/tracing/test_misc.py @@ -255,9 +255,11 @@ def test_set_meaurement(sentry_init, capture_events): transaction = start_transaction(name="measuring stuff") transaction.set_measurement("metric.foo", 123) - transaction.set_measurement("metric.bar", 456, unit="s") + transaction.set_measurement("metric.bar", 456, unit="second") + transaction.set_measurement("metric.baz", 420.69, unit="custom") transaction.finish() (event,) = events - assert event["measurements"]["metric.foo"] == {"value": 123, "unit": "ms"} - assert event["measurements"]["metric.bar"] == {"value": 456, "unit": "s"} + assert event["measurements"]["metric.foo"] == {"value": 123, "unit": ""} + assert event["measurements"]["metric.bar"] == {"value": 456, "unit": "second"} + assert event["measurements"]["metric.baz"] == {"value": 420.69, "unit": "custom"} From 6b380b8d7c19a6daa679458eb75e87cb83640b03 Mon Sep 17 00:00:00 2001 From: Neel Shah Date: Thu, 5 May 2022 14:35:28 +0200 Subject: [PATCH 8/9] lint --- sentry_sdk/tracing.py | 38 ++++++++++++++++++++++---------------- 1 file changed, 22 insertions(+), 16 deletions(-) diff --git a/sentry_sdk/tracing.py b/sentry_sdk/tracing.py index 17b94be90f..f6f625acc8 100644 --- a/sentry_sdk/tracing.py +++ b/sentry_sdk/tracing.py @@ -132,14 +132,17 @@ def init_span_recorder(self, maxlen): def __repr__(self): # type: () -> str - return "<%s(op=%r, description:%r, trace_id=%r, span_id=%r, parent_span_id=%r, sampled=%r)>" % ( - self.__class__.__name__, - self.op, - self.description, - self.trace_id, - self.span_id, - self.parent_span_id, - self.sampled, + return ( + "<%s(op=%r, description:%r, trace_id=%r, span_id=%r, parent_span_id=%r, sampled=%r)>" + % ( + self.__class__.__name__, + self.op, + self.description, + self.trace_id, + self.span_id, + self.parent_span_id, + self.sampled, + ) ) def __enter__(self): @@ -517,14 +520,17 @@ def __init__( def __repr__(self): # type: () -> str - return "<%s(name=%r, op=%r, trace_id=%r, span_id=%r, parent_span_id=%r, sampled=%r)>" % ( - self.__class__.__name__, - self.name, - self.op, - self.trace_id, - self.span_id, - self.parent_span_id, - self.sampled, + return ( + "<%s(name=%r, op=%r, trace_id=%r, span_id=%r, parent_span_id=%r, sampled=%r)>" + % ( + self.__class__.__name__, + self.name, + self.op, + self.trace_id, + self.span_id, + self.parent_span_id, + self.sampled, + ) ) @property From f79837c3bb6089e1958188e31ecb9f1d25be09e3 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Thu, 5 May 2022 15:47:30 +0200 Subject: [PATCH 9/9] Added more test cases --- tests/tracing/test_misc.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/tests/tracing/test_misc.py b/tests/tracing/test_misc.py index dd47f25220..43d9597f1b 100644 --- a/tests/tracing/test_misc.py +++ b/tests/tracing/test_misc.py @@ -254,12 +254,23 @@ def test_set_meaurement(sentry_init, capture_events): events = capture_events() transaction = start_transaction(name="measuring stuff") + + with pytest.raises(TypeError): + transaction.set_measurement() + + with pytest.raises(TypeError): + transaction.set_measurement("metric.foo") + transaction.set_measurement("metric.foo", 123) transaction.set_measurement("metric.bar", 456, unit="second") transaction.set_measurement("metric.baz", 420.69, unit="custom") + transaction.set_measurement("metric.foobar", 12, unit="percent") + transaction.set_measurement("metric.foobar", 17.99, unit="percent") + transaction.finish() (event,) = events assert event["measurements"]["metric.foo"] == {"value": 123, "unit": ""} assert event["measurements"]["metric.bar"] == {"value": 456, "unit": "second"} assert event["measurements"]["metric.baz"] == {"value": 420.69, "unit": "custom"} + assert event["measurements"]["metric.foobar"] == {"value": 17.99, "unit": "percent"}