chore(span): make fields immutable after serialization

ddtrace/_trace/processor/__init__.py

@@ -335,16 +335,21 @@ def on_span_finish(self, span: Span) -> None:
                     finished[0].set_metric("_dd.py.partial_flush", num_finished)
 
                 spans: Optional[List[Span]] = finished
+                if not spans:
+                    return
                 for tp in self._trace_processors:
                     try:
-                        if spans is None:
-                            return
                         spans = tp.process_trace(spans)
                     except Exception:
                         log.error("error applying processor %r", tp, exc_info=True)
 
                 self._queue_span_count_metrics("spans_finished", "integration_name")
                 self._writer.write(spans)
+                # At this point the span object has been sent to the writer for encoding
+                # and should be considered immutable. Updating the span after this point
+                # will may not be reflected in the encoded span.
+                for span in spans:
+                    span._make_immutable()
                 return
 
             log.debug("trace %d has %d spans, %d finished", span.trace_id, len(trace.spans), trace.num_finished)

ddtrace/_trace/span.py

@@ -19,6 +19,8 @@
 from ddtrace._trace._span_pointer import _SpanPointer
 from ddtrace._trace._span_pointer import _SpanPointerDirection
 from ddtrace._trace.context import Context
+from ddtrace._trace.types import _ImmutableDict
+from ddtrace._trace.types import _ImmutableList
 from ddtrace._trace.types import _MetaDictType
 from ddtrace._trace.types import _MetricDictType
 from ddtrace._trace.types import _TagNameType
@@ -108,6 +110,7 @@ class Span(object):
         "error",
         "_metrics",
         "_store",
+        "_mutable",
         "span_type",
         "start_ns",
         "duration_ns",
@@ -176,6 +179,7 @@ def __init__(
         self._resource = [resource or name]
         self.span_type = span_type
         self._span_api = span_api
+        self._mutable = True
 
         self._meta: _MetaDictType = {}
         self.error = 0
@@ -279,6 +283,27 @@ def duration(self) -> Optional[float]:
     def duration(self, value: float) -> None:
         self.duration_ns = int(value * 1e9)
 
+    def _make_immutable(self) -> None:
+        self._meta = _ImmutableDict(self._meta)
+        self._metrics = _ImmutableDict(self._metrics)
+        self._store = _ImmutableDict(self._store or {})
+        self._meta_struct = _ImmutableDict(self._meta_struct)
+        self._events = _ImmutableList(self._events)
+        self._links = _ImmutableList(self._links)
+        self._mutable = False
+
+    def __setattr__(self, key, value):
+        if hasattr(self, "_mutable") and not self._mutable:
+            log.warning(
+                "Span with the name %s has been serialized and is now immutable, "
+                "ignoring set attribute %s and value %r",
+                self.name,
+                key,
+                value,
+            )
+        else:
+            super(Span, self).__setattr__(key, value)
+
     @property
     def sampled(self) -> Optional[bool]:
         deprecate(

ddtrace/_trace/types.py

@@ -3,8 +3,43 @@
 from typing import Union
 
 from ddtrace.internal.compat import NumericType
+from ddtrace.internal.logger import get_logger
 
 
+log = get_logger(__name__)
+
 _TagNameType = Union[Text, bytes]
 _MetaDictType = Dict[_TagNameType, Text]
 _MetricDictType = Dict[_TagNameType, NumericType]
+
+
+class _ImmutableDict(dict):
+    def __setitem__(self, name, value):
+        # log a warning when trying to set an attribute on an immutable map, do not raise a ValueError
+        log.warning("Map is immutable, ignoring set attribute %s and value %r", name, value)
+
+
+class _ImmutableList(list):
+    def __setitem__(self, name, value):
+        # log a warning when trying to set an attribute on an immutable list, do not raise a ValueError
+        log.warning("List is immutable, ignoring set attribute %s and value %r", name, value)
+
+    def append(self, value):
+        # log a warning when trying to append to an immutable list, do not raise a ValueError
+        log.warning("List is immutable, ignoring append of value %r", value)
+
+    def extend(self, values):
+        # log a warning when trying to extend an immutable list, do not raise a ValueError
+        log.warning("List is immutable, ignoring extend with iterable %r", values)
+
+    def insert(self, index, value):
+        # log a warning when trying to insert into an immutable list, do not raise a ValueError
+        log.warning("List is immutable, ignoring insert at index %d with value %r", index, value)
+
+    def remove(self, value):
+        # log a warning when trying to remove from an immutable list, do not raise a ValueError
+        log.warning("List is immutable, ignoring remove of value %r", value)
+
+    def pop(self, index=-1):
+        # log a warning when trying to pop from an immutable list, do not raise a ValueError
+        log.warning("List is immutable, ignoring pop at index %d", index)

tests/contrib/djangorestframework/test_appsec.py

@@ -11,7 +11,6 @@
 @pytest.mark.skipif(django.VERSION < (1, 10), reason="requires django version >= 1.10")
 def test_djangorest_request_body_urlencoded(client, test_spans, tracer):
     with override_global_config(dict(_asm_enabled=True)):
-        tracer._asm_enabled = True
         # Hack: need to pass an argument to configure so that the processors are recreated
         tracer.configure(api_version="v0.4")
         payload = urlencode({"mytestingbody_key": "mytestingbody_value"})
@@ -29,7 +28,6 @@ def test_djangorest_request_body_urlencoded(client, test_spans, tracer):
 @pytest.mark.skipif(django.VERSION < (1, 10), reason="requires django version >= 1.10")
 def test_djangorest_request_body_custom_parser(client, test_spans, tracer):
     with override_global_config(dict(_asm_enabled=True)):
-        tracer._asm_enabled = True
         # Hack: need to pass an argument to configure so that the processors are recreated
         tracer.configure(api_version="v0.4")
         payload, content_type = (

tests/tracer/test_span.py

@@ -874,3 +874,60 @@ def _(span, *exc_info):
         raise AssertionError("should have raised")
     finally:
         core.reset_listeners("span.exception")
+
+
+def test_span_make_immutable():
+    """
+    Ensure that a span can be made immutable
+    """
+    s = Span(
+        "test.span", service="s", resource="r", span_type=SpanTypes.WEB, trace_id=1, span_id=2, parent_id=3, start=123
+    )
+    s.set_tag("a", "a")
+    s.set_metric("b", 1)
+    s._add_event("message")
+    s.set_link(trace_id=11, span_id=12)
+    s._set_ctx_item("ctx1", "val1")
+    s._make_immutable()
+
+    s.service = "new-service"
+    s.resource = "new-resource"
+    s.span_type = "new-type"
+    s.trace_id = 10000000
+    s.span_id = 20000000
+    s.parent_id = 30000000
+    s.start = 45600000
+    s.set_tag("a", "banana")
+    s.set_metric("b", 10000000)
+    s._add_event("message2")
+    s.set_link(trace_id=1100000, span_id=12000)
+    s._set_ctx_item("ctx2", "val2")
+
+    assert s.service == "s"
+    assert s.resource == "r"
+    assert s.span_type == "web"
+    assert s.trace_id == 1
+    assert s.span_id == 2
+    assert s.parent_id == 3
+    assert s.start == 123
+    assert s.get_tag("a") == "a"
+    assert s.get_metric("b") == 1
+    assert len(s._events) == 1
+    assert len(s._links) == 1
+    assert len(s._store) == 1
+
+
+def test_span_make_immutable_warning():
+    """
+    Ensure that an immutable span logs a warning when trying to set an attribute
+    """
+    with mock.patch("ddtrace._trace.span.log") as log:
+        s = Span("test.span")
+        s._make_immutable()
+        s.name = "new-name"
+        log.warning.assert_called_once_with(
+            "Span with the name %s has been serialized and is now immutable, ignoring set attribute %s and value %r",
+            "test.span",
+            "name",
+            "new-name",
+        )