From 1dcf3bccabf1f041c7ab07f766590974099f54e2 Mon Sep 17 00:00:00 2001
From: Eoin Noble <eoin.noble@googlemail.com>
Date: Fri, 2 Oct 2020 00:08:00 +0100
Subject: [PATCH] Make event attributes immutable

This PR makes it more difficult for someone to edit the attributes
of an `Event` once it has been added to a `Span`, as well as
adding some regression tests around the other properties of an
`Event` to ensure they have similar protections.

It isn't particularly straightforward or idiomatic to make objects
truly immutable in Python, but if these steps don't go far enough
let me know!

Fixes #1038
---
 .../src/opentelemetry/sdk/trace/__init__.py   | 10 ++++---
 .../src/opentelemetry/sdk/util/__init__.py    |  7 +++++
 opentelemetry-sdk/tests/trace/test_trace.py   | 27 ++++++++++++++++++-
 3 files changed, 40 insertions(+), 4 deletions(-)

diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py
index 0134ec7e775..7bdca0a0385 100644
--- a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py
+++ b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py
@@ -41,7 +41,7 @@
 from opentelemetry.sdk import util
 from opentelemetry.sdk.resources import Resource
 from opentelemetry.sdk.trace import sampling
-from opentelemetry.sdk.util import BoundedDict, BoundedList
+from opentelemetry.sdk.util import BoundedDict, BoundedList, make_immutable_dict
 from opentelemetry.sdk.util.instrumentation import InstrumentationInfo
 from opentelemetry.trace import SpanContext
 from opentelemetry.trace.propagation import SPAN_KEY
@@ -336,6 +336,10 @@ def _filter_attribute_values(attributes: types.Attributes):
                 attributes.pop(attr_key)
 
 
+def _make_event_attributes_immutable(event: EventBase) -> None:
+    event._attributes = make_immutable_dict(event.attributes)
+
+
 class Span(trace_api.Span):
     """See `opentelemetry.trace.Span`.
 
@@ -399,6 +403,7 @@ def __init__(
         if events:
             for event in events:
                 _filter_attribute_values(event.attributes)
+                _make_event_attributes_immutable(event)
                 self.events.append(event)
 
         if links is None:
@@ -557,8 +562,7 @@ def add_event(
         timestamp: Optional[int] = None,
     ) -> None:
         _filter_attribute_values(attributes)
-        if not attributes:
-            attributes = self._new_attributes()
+        attributes = make_immutable_dict(attributes)
         self._add_event(
             Event(
                 name=name,
diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/util/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/util/__init__.py
index 09d7283cab7..e78f5324b63 100644
--- a/opentelemetry-sdk/src/opentelemetry/sdk/util/__init__.py
+++ b/opentelemetry-sdk/src/opentelemetry/sdk/util/__init__.py
@@ -14,6 +14,7 @@
 import datetime
 import threading
 from collections import OrderedDict, deque
+from types import MappingProxyType
 
 try:
     # pylint: disable=ungrouped-imports
@@ -45,6 +46,12 @@ def get_dict_as_key(labels):
     )
 
 
+def make_immutable_dict(attributes):
+    return MappingProxyType(
+        attributes.copy() if attributes else {}
+    )
+
+
 class BoundedList(Sequence):
     """An append only list with a fixed max size.
 
diff --git a/opentelemetry-sdk/tests/trace/test_trace.py b/opentelemetry-sdk/tests/trace/test_trace.py
index 8c5544578d4..44c350847bf 100644
--- a/opentelemetry-sdk/tests/trace/test_trace.py
+++ b/opentelemetry-sdk/tests/trace/test_trace.py
@@ -561,7 +561,6 @@ def test_events(self):
             root.add_event("event0")
 
             # event name and attributes
-            now = time_ns()
             root.add_event(
                 "event1", {"name": "pluto", "some_bools": [True, False]}
             )
@@ -599,6 +598,32 @@ def test_events(self):
                 root.events[3].attributes, {"name": ("original_contents",)}
             )
 
+
+    def test_events_are_immutable(self):
+        event_properties = [
+            prop for prop in dir(trace.EventBase)
+            if not prop.startswith("_")
+        ]
+
+        with self.tracer.start_as_current_span("root") as root:
+            root.add_event("event0", {"name": ["birthday"]})
+            event = root.events[0]
+
+            for prop in event_properties:
+                with self.assertRaises(AttributeError):
+                    setattr(event, prop, "something")
+
+    def test_event_attributes_are_immutable(self):
+        with self.tracer.start_as_current_span("root") as root:
+            root.add_event("event0", {"name": ["birthday"]})
+            event = root.events[0]
+
+            with self.assertRaises(TypeError):
+                event.attributes["name"][0] = "happy"
+
+            with self.assertRaises(TypeError):
+                event.attributes["name"] = "hello"
+
     def test_invalid_event_attributes(self):
         self.assertEqual(trace_api.get_current_span(), trace_api.INVALID_SPAN)