Attributes returned from SamplingResult must be immutable (open-telemetry#1066)

lzchen · web-flow · commit 3bbab028f70c · 2020-09-03T09:07:03.000-07:00
diff --git a/instrumentation/opentelemetry-instrumentation-system-metrics/setup.cfg b/instrumentation/opentelemetry-instrumentation-system-metrics/setup.cfg
@@ -41,6 +41,7 @@ package_dir=
 packages=find_namespace:
 install_requires =
     opentelemetry-api == 0.13dev0
+    opentelemetry-sdk == 0.13dev0
     psutil ~= 5.7.0
 
 [options.extras_require]
diff --git a/opentelemetry-api/tests/baggage/test_baggage.py b/opentelemetry-api/tests/baggage/test_baggage.py
diff --git a/opentelemetry-api/tests/baggage/test_baggage_propagation.py b/opentelemetry-api/tests/baggage/test_baggage_propagation.py
diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py
@@ -772,7 +772,7 @@ def start_span(  # pylint: disable=too-many-locals
                 parent=parent_context,
                 sampler=self.source.sampler,
                 resource=self.source.resource,
-                attributes=sampling_result.attributes,
+                attributes=sampling_result.attributes.copy(),
                 span_processor=self.source._active_span_processor,
                 kind=kind,
                 links=links,
diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/trace/sampling.py b/opentelemetry-sdk/src/opentelemetry/sdk/trace/sampling.py
@@ -62,6 +62,7 @@
 """
 import abc
 import enum
+from types import MappingProxyType
 from typing import Optional, Sequence
 
 # pylint: disable=unused-import
@@ -102,11 +103,10 @@ def __init__(
         self, decision: Decision, attributes: Attributes = None,
     ) -> None:
         self.decision = decision
-        # TODO: attributes must be immutable
         if attributes is None:
-            self.attributes = {}
+            self.attributes = MappingProxyType({})
         else:
-            self.attributes = attributes
+            self.attributes = MappingProxyType(attributes)
 
 
 class Sampler(abc.ABC):
diff --git a/opentelemetry-sdk/tests/trace/test_sampling.py b/opentelemetry-sdk/tests/trace/test_sampling.py
@@ -22,6 +22,43 @@
 TO_SAMPLED = trace.TraceFlags(trace.TraceFlags.SAMPLED)
 
 
+class TestDecision(unittest.TestCase):
+    def test_is_recording(self):
+        self.assertTrue(
+            sampling.Decision.is_recording(sampling.Decision.RECORD)
+        )
+        self.assertTrue(
+            sampling.Decision.is_recording(
+                sampling.Decision.RECORD_AND_SAMPLED
+            )
+        )
+        self.assertFalse(
+            sampling.Decision.is_recording(sampling.Decision.NOT_RECORD)
+        )
+
+    def test_is_sampled(self):
+        self.assertFalse(
+            sampling.Decision.is_sampled(sampling.Decision.RECORD)
+        )
+        self.assertTrue(
+            sampling.Decision.is_sampled(sampling.Decision.RECORD_AND_SAMPLED)
+        )
+        self.assertFalse(
+            sampling.Decision.is_sampled(sampling.Decision.NOT_RECORD)
+        )
+
+
+class TestSamplingResult(unittest.TestCase):
+    def test_ctr(self):
+        attributes = {"asd": "test"}
+        result = sampling.SamplingResult(sampling.Decision.RECORD, attributes)
+        self.assertIs(result.decision, sampling.Decision.RECORD)
+        with self.assertRaises(TypeError):
+            result.attributes["test"] = "mess-this-up"
+        self.assertTrue(len(result.attributes), 1)
+        self.assertEqual(result.attributes["asd"], "test")
+
+
 class TestSampler(unittest.TestCase):
     def test_always_on(self):
         no_record_always_on = sampling.ALWAYS_ON.should_sample(
