ref: Make logs, metrics go via scope (#5213)

### Description
Logs and metrics were going through a completely separate pipeline
compared to other events.

Conceptually, they're still different from regular events since they're
more lightweight and attribute-based (no data, contexts, etc.,
everything is an attribute) so separate handling makes sense. However,
the pipeline should still conceptually resemble the one we use for other
event types, for consistency.

## Current pipeline for non-log, non-metric events
- The top-level API calls `scope.capture_XXX`. This merges the active
scope stack (global + isolation + current scope) and calls
`client.capture_XXX` with the resulting merged scope.
- `client.capture_XXX` contains virtually all of the logic, most
notably:
- It applies the scope to the event by calling `scope.apply_to_event`,
populating contexts, user data, etc.
  - It serializes the event.
  - It constructs the final envelope and sends it to the transport.

## This PR
- Instead of the logging/metrics functionality going straight to
`client.capture_XXX`, we call `scope.capture_XXX`, like we do for other
event types, and then call `client.capture_XXX` from there.
- Instead of inlining (and duplicating) all the attribute logic,
`client.capture_XXX` now calls a new `scope.apply_to_telemetry` function
internally (akin to `scope.apply_to_event`, but sets attributes
instead).
- The rest of the pipeline was left as-is for now, so metrics and logs
are directly put into the batcher which itself serializes them. It's
questionable whether making this part of the pipeline more similar to
the event one would be a good idea since in Span First it'll be
beneficial to have unserialized telemetry in the buffer, as is the case
now with logs and metrics.

Additionally:
- Unify attribute-related types
- Move duplicated `format_attribute` to utils

Re: naming: I'm calling the new-style, attribute-based things simply
"telemetry", since not all of them are events (for example, spans v2
which are coming with span streaming).

Note: I might refactor further. I'd like to have proper classes for Logs
and Metrics and give them ownership of how to serialize themselves, how
to call before_send, etc., but need to see whether there's a nice way to
do this without breaking backwards compat (the log/metric needs to be a
dict in before_send_x).

#### Issues
<!--
* resolves: #1234
* resolves: LIN-1234
-->

#### Reminders
- Please add tests to validate your changes, and lint your code using
`tox -e linters`.
- Add GH Issue ID _&_ Linear ID (if applicable)
- PR title should use [conventional
commit](https://develop.sentry.dev/engineering-practices/commit-messages/#type)
style (`feat:`, `fix:`, `ref:`, `meta:`)
- For external contributors:
[CONTRIBUTING.md](https://github.com/getsentry/sentry-python/blob/master/CONTRIBUTING.md),
[Sentry SDK development docs](https://develop.sentry.dev/sdk/), [Discord
community](https://discord.gg/Ww9hbqr)

Author: sentrivana

sentry_sdk/_log_batcher.py

@@ -4,7 +4,7 @@
 from datetime import datetime, timezone
 from typing import Optional, List, Callable, TYPE_CHECKING, Any
 
-from sentry_sdk.utils import format_timestamp, safe_repr
+from sentry_sdk.utils import format_timestamp, safe_repr, serialize_attribute
 from sentry_sdk.envelope import Envelope, Item, PayloadRef
 
 if TYPE_CHECKING:
@@ -115,17 +115,6 @@ def flush(self) -> None:
 
     @staticmethod
     def _log_to_transport_format(log: "Log") -> "Any":
-        def format_attribute(val: "int | float | str | bool") -> "Any":
-            if isinstance(val, bool):
-                return {"value": val, "type": "boolean"}
-            if isinstance(val, int):
-                return {"value": val, "type": "integer"}
-            if isinstance(val, float):
-                return {"value": val, "type": "double"}
-            if isinstance(val, str):
-                return {"value": val, "type": "string"}
-            return {"value": safe_repr(val), "type": "string"}
-
         if "sentry.severity_number" not in log["attributes"]:
             log["attributes"]["sentry.severity_number"] = log["severity_number"]
         if "sentry.severity_text" not in log["attributes"]:
@@ -138,7 +127,7 @@ def format_attribute(val: "int | float | str | bool") -> "Any":
             "level": str(log["severity_text"]),
             "body": str(log["body"]),
             "attributes": {
-                k: format_attribute(v) for (k, v) in log["attributes"].items()
+                k: serialize_attribute(v) for (k, v) in log["attributes"].items()
             },
         }
 

sentry_sdk/_metrics_batcher.py

@@ -4,7 +4,7 @@
 from datetime import datetime, timezone
 from typing import Optional, List, Callable, TYPE_CHECKING, Any, Union
 
-from sentry_sdk.utils import format_timestamp, safe_repr
+from sentry_sdk.utils import format_timestamp, safe_repr, serialize_attribute
 from sentry_sdk.envelope import Envelope, Item, PayloadRef
 
 if TYPE_CHECKING:
@@ -96,25 +96,14 @@ def flush(self) -> None:
 
     @staticmethod
     def _metric_to_transport_format(metric: "Metric") -> "Any":
-        def format_attribute(val: "Union[int, float, str, bool]") -> "Any":
-            if isinstance(val, bool):
-                return {"value": val, "type": "boolean"}
-            if isinstance(val, int):
-                return {"value": val, "type": "integer"}
-            if isinstance(val, float):
-                return {"value": val, "type": "double"}
-            if isinstance(val, str):
-                return {"value": val, "type": "string"}
-            return {"value": safe_repr(val), "type": "string"}
-
         res = {
             "timestamp": metric["timestamp"],
             "trace_id": metric["trace_id"],
             "name": metric["name"],
             "type": metric["type"],
             "value": metric["value"],
             "attributes": {
-                k: format_attribute(v) for (k, v) in metric["attributes"].items()
+                k: serialize_attribute(v) for (k, v) in metric["attributes"].items()
             },
         }
 

sentry_sdk/_types.py

@@ -215,13 +215,39 @@ class SDKInfo(TypedDict):
     # TODO: Make a proper type definition for this (PRs welcome!)
     Hint = Dict[str, Any]
 
+    AttributeValue = (
+        str | bool | float | int
+        # TODO: relay support coming soon for
+        # | list[str] | list[bool] | list[float] | list[int]
+    )
+    Attributes = dict[str, AttributeValue]
+
+    SerializedAttributeValue = TypedDict(
+        # https://develop.sentry.dev/sdk/telemetry/attributes/#supported-types
+        "SerializedAttributeValue",
+        {
+            "type": Literal[
+                "string",
+                "boolean",
+                "double",
+                "integer",
+                # TODO: relay support coming soon for:
+                # "string[]",
+                # "boolean[]",
+                # "double[]",
+                # "integer[]",
+            ],
+            "value": AttributeValue,
+        },
+    )
+
     Log = TypedDict(
         "Log",
         {
             "severity_text": str,
             "severity_number": int,
             "body": str,
-            "attributes": dict[str, str | bool | float | int],
+            "attributes": Attributes,
             "time_unix_nano": int,
             "trace_id": Optional[str],
             "span_id": Optional[str],
@@ -230,14 +256,6 @@ class SDKInfo(TypedDict):
 
     MetricType = Literal["counter", "gauge", "distribution"]
 
-    MetricAttributeValue = TypedDict(
-        "MetricAttributeValue",
-        {
-            "value": Union[str, bool, float, int],
-            "type": Literal["string", "boolean", "double", "integer"],
-        },
-    )
-
     Metric = TypedDict(
         "Metric",
         {
@@ -248,7 +266,7 @@ class SDKInfo(TypedDict):
             "type": MetricType,
             "value": float,
             "unit": Optional[str],
-            "attributes": dict[str, str | bool | float | int],
+            "attributes": Attributes,
         },
     )
 

sentry_sdk/client.py

@@ -217,10 +217,10 @@ def is_active(self) -> bool:
     def capture_event(self, *args: "Any", **kwargs: "Any") -> "Optional[str]":
         return None
 
-    def _capture_log(self, log: "Log") -> None:
+    def _capture_log(self, log: "Log", scope: "Scope") -> None:
         pass
 
-    def _capture_metric(self, metric: "Metric") -> None:
+    def _capture_metric(self, metric: "Metric", scope: "Scope") -> None:
         pass
 
     def capture_session(self, *args: "Any", **kwargs: "Any") -> None:
@@ -898,132 +898,41 @@ def capture_event(
 
         return return_value
 
-    def _capture_log(self, log: "Optional[Log]") -> None:
-        if not has_logs_enabled(self.options) or log is None:
+    def _capture_telemetry(
+        self, telemetry: "Optional[Union[Log, Metric]]", ty: str, scope: "Scope"
+    ) -> None:
+        # Capture attributes-based telemetry (logs, metrics, spansV2)
+        if telemetry is None:
             return
 
-        current_scope = sentry_sdk.get_current_scope()
-        isolation_scope = sentry_sdk.get_isolation_scope()
-
-        log["attributes"]["sentry.sdk.name"] = SDK_INFO["name"]
-        log["attributes"]["sentry.sdk.version"] = SDK_INFO["version"]
-
-        server_name = self.options.get("server_name")
-        if server_name is not None and SPANDATA.SERVER_ADDRESS not in log["attributes"]:
-            log["attributes"][SPANDATA.SERVER_ADDRESS] = server_name
-
-        environment = self.options.get("environment")
-        if environment is not None and "sentry.environment" not in log["attributes"]:
-            log["attributes"]["sentry.environment"] = environment
-
-        release = self.options.get("release")
-        if release is not None and "sentry.release" not in log["attributes"]:
-            log["attributes"]["sentry.release"] = release
-
-        trace_context = current_scope.get_trace_context()
-        trace_id = trace_context.get("trace_id")
-        span_id = trace_context.get("span_id")
-
-        if trace_id is not None and log.get("trace_id") is None:
-            log["trace_id"] = trace_id
-
-        if span_id is not None and log.get("span_id") is None:
-            log["span_id"] = span_id
-
-        # The user, if present, is always set on the isolation scope.
-        if self.should_send_default_pii() and isolation_scope._user is not None:
-            for log_attribute, user_attribute in (
-                ("user.id", "id"),
-                ("user.name", "username"),
-                ("user.email", "email"),
-            ):
-                if (
-                    user_attribute in isolation_scope._user
-                    and log_attribute not in log["attributes"]
-                ):
-                    log["attributes"][log_attribute] = isolation_scope._user[
-                        user_attribute
-                    ]
-
-        # If debug is enabled, log the log to the console
-        debug = self.options.get("debug", False)
-        if debug:
-            logger.debug(
-                f"[Sentry Logs] [{log.get('severity_text')}] {log.get('body')}"
-            )
-
-        before_send_log = get_before_send_log(self.options)
-        if before_send_log is not None:
-            log = before_send_log(log, {})
+        scope.apply_to_telemetry(telemetry)
 
-        if log is None:
-            return
+        before_send = None
+        if ty == "log":
+            before_send = get_before_send_log(self.options)
+        elif ty == "metric":
+            before_send = get_before_send_metric(self.options)  # type: ignore
 
-        if self.log_batcher:
-            self.log_batcher.add(log)
+        if before_send is not None:
+            telemetry = before_send(telemetry, {})  # type: ignore
 
-    def _capture_metric(self, metric: "Optional[Metric]") -> None:
-        if not has_metrics_enabled(self.options) or metric is None:
+        if telemetry is None:
             return
 
-        current_scope = sentry_sdk.get_current_scope()
-        isolation_scope = sentry_sdk.get_isolation_scope()
+        batcher = None
+        if ty == "log":
+            batcher = self.log_batcher
+        elif ty == "metric":
+            batcher = self.metrics_batcher  # type: ignore
 
-        metric["attributes"]["sentry.sdk.name"] = SDK_INFO["name"]
-        metric["attributes"]["sentry.sdk.version"] = SDK_INFO["version"]
+        if batcher is not None:
+            batcher.add(telemetry)  # type: ignore
 
-        server_name = self.options.get("server_name")
-        if (
-            server_name is not None
-            and SPANDATA.SERVER_ADDRESS not in metric["attributes"]
-        ):
-            metric["attributes"][SPANDATA.SERVER_ADDRESS] = server_name
-
-        environment = self.options.get("environment")
-        if environment is not None and "sentry.environment" not in metric["attributes"]:
-            metric["attributes"]["sentry.environment"] = environment
-
-        release = self.options.get("release")
-        if release is not None and "sentry.release" not in metric["attributes"]:
-            metric["attributes"]["sentry.release"] = release
-
-        trace_context = current_scope.get_trace_context()
-        trace_id = trace_context.get("trace_id")
-        span_id = trace_context.get("span_id")
-
-        metric["trace_id"] = trace_id or "00000000-0000-0000-0000-000000000000"
-        if span_id is not None:
-            metric["span_id"] = span_id
-
-        if self.should_send_default_pii() and isolation_scope._user is not None:
-            for metric_attribute, user_attribute in (
-                ("user.id", "id"),
-                ("user.name", "username"),
-                ("user.email", "email"),
-            ):
-                if (
-                    user_attribute in isolation_scope._user
-                    and metric_attribute not in metric["attributes"]
-                ):
-                    metric["attributes"][metric_attribute] = isolation_scope._user[
-                        user_attribute
-                    ]
-
-        debug = self.options.get("debug", False)
-        if debug:
-            logger.debug(
-                f"[Sentry Metrics] [{metric.get('type')}] {metric.get('name')}: {metric.get('value')}"
-            )
-
-        before_send_metric = get_before_send_metric(self.options)
-        if before_send_metric is not None:
-            metric = before_send_metric(metric, {})
-
-        if metric is None:
-            return
+    def _capture_log(self, log: "Optional[Log]", scope: "Scope") -> None:
+        self._capture_telemetry(log, "log", scope)
 
-        if self.metrics_batcher:
-            self.metrics_batcher.add(metric)
+    def _capture_metric(self, metric: "Optional[Metric]", scope: "Scope") -> None:
+        self._capture_telemetry(metric, "metric", scope)
 
     def capture_session(
         self,

sentry_sdk/integrations/logging.py

@@ -396,7 +396,7 @@ def _capture_log_from_record(
             attrs["logger.name"] = record.name
 
         # noinspection PyProtectedMember
-        client._capture_log(
+        sentry_sdk.get_current_scope()._capture_log(
             {
                 "severity_text": otel_severity_text,
                 "severity_number": otel_severity_number,

sentry_sdk/integrations/loguru.py

@@ -196,7 +196,7 @@ def loguru_sentry_logs_handler(message: "Message") -> None:
             else:
                 attrs[f"sentry.message.parameter.{key}"] = safe_repr(value)
 
-    client._capture_log(
+    sentry_sdk.get_current_scope()._capture_log(
         {
             "severity_text": otel_severity_text,
             "severity_number": otel_severity_number,

sentry_sdk/logger.py

@@ -1,11 +1,15 @@
 # NOTE: this is the logger sentry exposes to users, not some generic logger.
 import functools
 import time
-from typing import Any
+from typing import Any, TYPE_CHECKING
 
-from sentry_sdk import get_client
+import sentry_sdk
 from sentry_sdk.utils import safe_repr, capture_internal_exceptions
 
+if TYPE_CHECKING:
+    from sentry_sdk._types import Attributes, Log
+
+
 OTEL_RANGES = [
     # ((severity level range), severity text)
     # https://opentelemetry.io/docs/specs/otel/logs/data-model
@@ -28,37 +32,27 @@ def __missing__(self, key: str) -> str:
 def _capture_log(
     severity_text: str, severity_number: int, template: str, **kwargs: "Any"
 ) -> None:
-    client = get_client()
-
     body = template
-    attrs: "dict[str, str | bool | float | int]" = {}
+
+    attrs: "Attributes" = {}
+
     if "attributes" in kwargs:
         attrs.update(kwargs.pop("attributes"))
+
     for k, v in kwargs.items():
         attrs[f"sentry.message.parameter.{k}"] = v
+
     if kwargs:
         # only attach template if there are parameters
         attrs["sentry.message.template"] = template
 
         with capture_internal_exceptions():
             body = template.format_map(_dict_default_key(kwargs))
 
-    attrs = {
-        k: (
-            v
-            if (
-                isinstance(v, str)
-                or isinstance(v, int)
-                or isinstance(v, bool)
-                or isinstance(v, float)
-            )
-            else safe_repr(v)
-        )
-        for (k, v) in attrs.items()
-    }
-
-    # noinspection PyProtectedMember
-    client._capture_log(
+    for k, v in attrs.items():
+        attrs[k] = v if isinstance(v, (str, int, bool, float)) else safe_repr(v)
+
+    sentry_sdk.get_current_scope()._capture_log(
         {
             "severity_text": severity_text,
             "severity_number": severity_number,
@@ -67,7 +61,7 @@ def _capture_log(
             "time_unix_nano": time.time_ns(),
             "trace_id": None,
             "span_id": None,
-        },
+        }
     )