Merge branch 'master' into ivana/test-preserialization

Author: sentrivana

sentry_sdk/logger.py

@@ -4,7 +4,7 @@
 from typing import Any, TYPE_CHECKING
 
 import sentry_sdk
-from sentry_sdk.utils import safe_repr, capture_internal_exceptions
+from sentry_sdk.utils import format_attribute, safe_repr, capture_internal_exceptions
 
 if TYPE_CHECKING:
     from sentry_sdk._types import Attributes, Log
@@ -34,29 +34,28 @@ def _capture_log(
 ) -> None:
     body = template
 
-    attrs: "Attributes" = {}
+    attributes: "Attributes" = {}
 
     if "attributes" in kwargs:
-        attrs.update(kwargs.pop("attributes"))
+        provided_attributes = kwargs.pop("attributes") or {}
+        for attribute, value in provided_attributes.items():
+            attributes[attribute] = format_attribute(value)
 
     for k, v in kwargs.items():
-        attrs[f"sentry.message.parameter.{k}"] = v
+        attributes[f"sentry.message.parameter.{k}"] = format_attribute(v)
 
     if kwargs:
         # only attach template if there are parameters
-        attrs["sentry.message.template"] = template
+        attributes["sentry.message.template"] = format_attribute(template)
 
         with capture_internal_exceptions():
             body = template.format_map(_dict_default_key(kwargs))
 
-    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,
-            "attributes": attrs,
+            "attributes": attributes,
             "body": body,
             "time_unix_nano": time.time_ns(),
             "trace_id": None,

sentry_sdk/metrics.py

@@ -7,7 +7,7 @@
 from typing import Any, Optional, TYPE_CHECKING, Union
 
 import sentry_sdk
-from sentry_sdk.utils import safe_repr
+from sentry_sdk.utils import format_attribute, safe_repr
 
 if TYPE_CHECKING:
     from sentry_sdk._types import Attributes, Metric, MetricType
@@ -24,7 +24,7 @@ def _capture_metric(
 
     if attributes:
         for k, v in attributes.items():
-            attrs[k] = v if isinstance(v, (str, int, bool, float)) else safe_repr(v)
+            attrs[k] = format_attribute(v)
 
     metric: "Metric" = {
         "timestamp": time.time(),

sentry_sdk/scope.py

@@ -46,6 +46,7 @@
     disable_capture_event,
     event_from_exception,
     exc_info_from_error,
+    format_attribute,
     logger,
     has_logs_enabled,
     has_metrics_enabled,
@@ -73,6 +74,8 @@
     from typing_extensions import Unpack
 
     from sentry_sdk._types import (
+        Attributes,
+        AttributeValue,
         Breadcrumb,
         BreadcrumbHint,
         ErrorProcessor,
@@ -230,6 +233,7 @@ class Scope:
         "_type",
         "_last_event_id",
         "_flags",
+        "_attributes",
     )
 
     def __init__(
@@ -296,6 +300,8 @@ def __copy__(self) -> "Scope":
 
         rv._flags = deepcopy(self._flags)
 
+        rv._attributes = self._attributes.copy()
+
         return rv
 
     @classmethod
@@ -684,6 +690,8 @@ def clear(self) -> None:
         self._last_event_id: "Optional[str]" = None
         self._flags: "Optional[FlagBuffer]" = None
 
+        self._attributes: "Attributes" = {}
+
     @_attr_setter
     def level(self, value: "LogLevelStr") -> None:
         """
@@ -1487,6 +1495,13 @@ def _apply_global_attributes_to_telemetry(
         if release is not None and "sentry.release" not in attributes:
             attributes["sentry.release"] = release
 
+    def _apply_scope_attributes_to_telemetry(
+        self, telemetry: "Union[Log, Metric]"
+    ) -> None:
+        for attribute, value in self._attributes.items():
+            if attribute not in telemetry["attributes"]:
+                telemetry["attributes"][attribute] = value
+
     def _apply_user_attributes_to_telemetry(
         self, telemetry: "Union[Log, Metric]"
     ) -> None:
@@ -1621,8 +1636,9 @@ def apply_to_telemetry(self, telemetry: "Union[Log, Metric]") -> None:
         if telemetry.get("span_id") is None and span_id:
             telemetry["span_id"] = span_id
 
-        self._apply_global_attributes_to_telemetry(telemetry)
+        self._apply_scope_attributes_to_telemetry(telemetry)
         self._apply_user_attributes_to_telemetry(telemetry)
+        self._apply_global_attributes_to_telemetry(telemetry)
 
     def update_from_scope(self, scope: "Scope") -> None:
         """Update the scope with another scope's data."""
@@ -1668,6 +1684,8 @@ def update_from_scope(self, scope: "Scope") -> None:
             else:
                 for flag in scope._flags.get():
                     self._flags.set(flag["flag"], flag["result"])
+        if scope._attributes:
+            self._attributes.update(scope._attributes)
 
     def update_from_kwargs(
         self,
@@ -1677,6 +1695,7 @@ def update_from_kwargs(
         contexts: "Optional[Dict[str, Dict[str, Any]]]" = None,
         tags: "Optional[Dict[str, str]]" = None,
         fingerprint: "Optional[List[str]]" = None,
+        attributes: "Optional[Attributes]" = None,
     ) -> None:
         """Update the scope's attributes."""
         if level is not None:
@@ -1691,6 +1710,8 @@ def update_from_kwargs(
             self._tags.update(tags)
         if fingerprint is not None:
             self._fingerprint = fingerprint
+        if attributes is not None:
+            self._attributes.update(attributes)
 
     def __repr__(self) -> str:
         return "<%s id=%s name=%s type=%s>" % (
@@ -1710,6 +1731,22 @@ def flags(self) -> "FlagBuffer":
             self._flags = FlagBuffer(capacity=max_flags)
         return self._flags
 
+    def set_attribute(self, attribute: str, value: "AttributeValue") -> None:
+        """
+        Set an attribute on the scope.
+
+        Any attributes-based telemetry (logs, metrics) captured while this scope
+        is active will inherit attributes set on the scope.
+        """
+        self._attributes[attribute] = format_attribute(value)
+
+    def remove_attribute(self, attribute: str) -> None:
+        """Remove an attribute if set on the scope. No-op if there is no such attribute."""
+        try:
+            del self._attributes[attribute]
+        except KeyError:
+            pass
+
 
 @contextmanager
 def new_scope() -> "Generator[Scope, None, None]":

sentry_sdk/utils.py

@@ -2047,7 +2047,25 @@ def get_before_send_metric(
     )
 
 
+def format_attribute(val: "Any") -> "AttributeValue":
+    """
+    Turn unsupported attribute value types into an AttributeValue.
+
+    We do this as soon as a user-provided attribute is set, to prevent spans,
+    logs, metrics and similar from having live references to various objects.
+
+    Note: This is not the final attribute value format. Before they're sent,
+    they're serialized further into the actual format the protocol expects:
+    https://develop.sentry.dev/sdk/telemetry/attributes/
+    """
+    if isinstance(val, (bool, int, float, str)):
+        return val
+
+    return safe_repr(val)
+
+
 def serialize_attribute(val: "AttributeValue") -> "SerializedAttributeValue":
+    """Serialize attribute value to the transport format."""
     if isinstance(val, bool):
         return {"value": val, "type": "boolean"}
     if isinstance(val, int):
@@ -2057,5 +2075,6 @@ def serialize_attribute(val: "AttributeValue") -> "SerializedAttributeValue":
     if isinstance(val, str):
         return {"value": val, "type": "string"}
 
-    # Coerce to string if we don't know what to do with the value
+    # Coerce to string if we don't know what to do with the value. This should
+    # never happen as we pre-format early in format_attribute, but let's be safe.
     return {"value": safe_repr(val), "type": "string"}

tests/test_attributes.py

@@ -0,0 +1,119 @@
+import sentry_sdk
+
+from tests.test_metrics import envelopes_to_metrics
+
+
+def test_scope_precedence(sentry_init, capture_envelopes):
+    # Order of precedence, from most important to least:
+    # 1. telemetry attributes (directly supplying attributes on creation or using set_attribute)
+    # 2. current scope attributes
+    # 3. isolation scope attributes
+    # 4. global scope attributes
+    sentry_init()
+
+    envelopes = capture_envelopes()
+
+    global_scope = sentry_sdk.get_global_scope()
+    global_scope.set_attribute("global.attribute", "global")
+    global_scope.set_attribute("overwritten.attribute", "global")
+
+    isolation_scope = sentry_sdk.get_isolation_scope()
+    isolation_scope.set_attribute("isolation.attribute", "isolation")
+    isolation_scope.set_attribute("overwritten.attribute", "isolation")
+
+    current_scope = sentry_sdk.get_current_scope()
+    current_scope.set_attribute("current.attribute", "current")
+    current_scope.set_attribute("overwritten.attribute", "current")
+
+    sentry_sdk.metrics.count("test", 1)
+    sentry_sdk.get_client().flush()
+
+    metrics = envelopes_to_metrics(envelopes)
+    (metric,) = metrics
+
+    assert metric["attributes"]["global.attribute"] == "global"
+    assert metric["attributes"]["isolation.attribute"] == "isolation"
+    assert metric["attributes"]["current.attribute"] == "current"
+
+    assert metric["attributes"]["overwritten.attribute"] == "current"
+
+
+def test_telemetry_precedence(sentry_init, capture_envelopes):
+    # Order of precedence, from most important to least:
+    # 1. telemetry attributes (directly supplying attributes on creation or using set_attribute)
+    # 2. current scope attributes
+    # 3. isolation scope attributes
+    # 4. global scope attributes
+    sentry_init()
+
+    envelopes = capture_envelopes()
+
+    global_scope = sentry_sdk.get_global_scope()
+    global_scope.set_attribute("global.attribute", "global")
+    global_scope.set_attribute("overwritten.attribute", "global")
+
+    isolation_scope = sentry_sdk.get_isolation_scope()
+    isolation_scope.set_attribute("isolation.attribute", "isolation")
+    isolation_scope.set_attribute("overwritten.attribute", "isolation")
+
+    current_scope = sentry_sdk.get_current_scope()
+    current_scope.set_attribute("current.attribute", "current")
+    current_scope.set_attribute("overwritten.attribute", "current")
+
+    sentry_sdk.metrics.count(
+        "test",
+        1,
+        attributes={
+            "telemetry.attribute": "telemetry",
+            "overwritten.attribute": "telemetry",
+        },
+    )
+
+    sentry_sdk.get_client().flush()
+
+    metrics = envelopes_to_metrics(envelopes)
+    (metric,) = metrics
+
+    assert metric["attributes"]["global.attribute"] == "global"
+    assert metric["attributes"]["isolation.attribute"] == "isolation"
+    assert metric["attributes"]["current.attribute"] == "current"
+    assert metric["attributes"]["telemetry.attribute"] == "telemetry"
+
+    assert metric["attributes"]["overwritten.attribute"] == "telemetry"
+
+
+def test_attribute_out_of_scope(sentry_init, capture_envelopes):
+    sentry_init()
+
+    envelopes = capture_envelopes()
+
+    with sentry_sdk.new_scope() as scope:
+        scope.set_attribute("outofscope.attribute", "out of scope")
+
+    sentry_sdk.metrics.count("test", 1)
+
+    sentry_sdk.get_client().flush()
+
+    metrics = envelopes_to_metrics(envelopes)
+    (metric,) = metrics
+
+    assert "outofscope.attribute" not in metric["attributes"]
+
+
+def test_remove_attribute(sentry_init, capture_envelopes):
+    sentry_init()
+
+    envelopes = capture_envelopes()
+
+    with sentry_sdk.new_scope() as scope:
+        scope.set_attribute("some.attribute", 123)
+        scope.remove_attribute("some.attribute")
+
+        sentry_sdk.metrics.count("test", 1)
+
+    sentry_sdk.get_client().flush()
+
+    metrics = envelopes_to_metrics(envelopes)
+    (metric,) = metrics
+
+    assert "some.attribute" not in metric["attributes"]

tests/test_logs.py

@@ -550,6 +550,56 @@ def record_lost_event(reason, data_category=None, item=None, *, quantity=1):
         }
 
 
+@minimum_python_37
+def test_log_gets_attributes_from_scopes(sentry_init, capture_envelopes):
+    sentry_init(enable_logs=True)
+
+    envelopes = capture_envelopes()
+
+    global_scope = sentry_sdk.get_global_scope()
+    global_scope.set_attribute("global.attribute", "value")
+
+    with sentry_sdk.new_scope() as scope:
+        scope.set_attribute("current.attribute", "value")
+        sentry_sdk.logger.warning("Hello, world!")
+
+    sentry_sdk.logger.warning("Hello again!")
+
+    get_client().flush()
+
+    logs = envelopes_to_logs(envelopes)
+    (log1, log2) = logs
+
+    assert log1["attributes"]["global.attribute"] == "value"
+    assert log1["attributes"]["current.attribute"] == "value"
+
+    assert log2["attributes"]["global.attribute"] == "value"
+    assert "current.attribute" not in log2["attributes"]
+
+
+@minimum_python_37
+def test_log_attributes_override_scope_attributes(sentry_init, capture_envelopes):
+    sentry_init(enable_logs=True)
+
+    envelopes = capture_envelopes()
+
+    with sentry_sdk.new_scope() as scope:
+        scope.set_attribute("durable.attribute", "value1")
+        scope.set_attribute("temp.attribute", "value1")
+        sentry_sdk.logger.warning(
+            "Hello, world!", attributes={"temp.attribute": "value2"}
+        )
+
+    get_client().flush()
+
+    logs = envelopes_to_logs(envelopes)
+    (log,) = logs
+
+    assert log["attributes"]["durable.attribute"] == "value1"
+    assert log["attributes"]["temp.attribute"] == "value2"
+
+
+@minimum_python_37
 def test_preserialization(sentry_init, capture_envelopes):
     """We don't store references to objects in attributes."""
     sentry_init(enable_logs=True)