From 3bbab028f70cf84e2222e39e12777e8adb5fb517 Mon Sep 17 00:00:00 2001
From: Leighton Chen <lechen@microsoft.com>
Date: Thu, 3 Sep 2020 09:07:03 -0700
Subject: [PATCH] Attributes returned from SamplingResult must be immutable
 (#1066)

---
 .../setup.cfg                                 |  1 +
 .../test_baggage.py                           |  0
 .../test_baggage_propagation.py               |  0
 .../src/opentelemetry/sdk/trace/__init__.py   |  2 +-
 .../src/opentelemetry/sdk/trace/sampling.py   |  6 +--
 .../tests/trace/test_sampling.py              | 37 +++++++++++++++++++
 6 files changed, 42 insertions(+), 4 deletions(-)
 rename opentelemetry-api/tests/{correlationcontext => baggage}/test_baggage.py (100%)
 rename opentelemetry-api/tests/{correlationcontext => baggage}/test_baggage_propagation.py (100%)

diff --git a/instrumentation/opentelemetry-instrumentation-system-metrics/setup.cfg b/instrumentation/opentelemetry-instrumentation-system-metrics/setup.cfg
index 4a93873ce19..70c784c6068 100644
--- 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/correlationcontext/test_baggage.py b/opentelemetry-api/tests/baggage/test_baggage.py
similarity index 100%
rename from opentelemetry-api/tests/correlationcontext/test_baggage.py
rename to opentelemetry-api/tests/baggage/test_baggage.py
diff --git a/opentelemetry-api/tests/correlationcontext/test_baggage_propagation.py b/opentelemetry-api/tests/baggage/test_baggage_propagation.py
similarity index 100%
rename from opentelemetry-api/tests/correlationcontext/test_baggage_propagation.py
rename to 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
index 95d04d85895..a082cffa7b4 100644
--- 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
index c723e18adba..2a1d6e89db3 100644
--- 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
index 2c03f0bf737..de1c019fb56 100644
--- 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(