From 4d595f87e8b0a3c9d47f03c28ec2c2dfdaf455d9 Mon Sep 17 00:00:00 2001 From: Alex Boten Date: Mon, 9 Mar 2020 14:22:05 -0700 Subject: [PATCH 01/18] Initial commit for Correlation Context API This change removes Distributed Context and replaces it with the Correlations Context API. Things to do: - add more tests - implement correlation context propagation and add it to the default propagator Signed-off-by: Alex Boten --- .../correlationcontext/__init__.py | 124 +++++++++++++++ .../distributedcontext/__init__.py | 145 ------------------ .../test_correlation_context.py | 23 +++ .../test_distributed_context.py | 112 -------------- .../sdk/correlationcontext/__init__.py | 72 +++++++++ .../sdk/distributedcontext/__init__.py | 61 -------- .../propagation/__init__.py | 0 .../__init__.py | 0 .../test_correlation_context.py | 29 ++++ .../test_distributed_context.py | 42 ----- 10 files changed, 248 insertions(+), 360 deletions(-) create mode 100644 opentelemetry-api/src/opentelemetry/correlationcontext/__init__.py delete mode 100644 opentelemetry-api/src/opentelemetry/distributedcontext/__init__.py create mode 100644 opentelemetry-api/tests/correlationcontext/test_correlation_context.py delete mode 100644 opentelemetry-api/tests/distributedcontext/test_distributed_context.py create mode 100644 opentelemetry-sdk/src/opentelemetry/sdk/correlationcontext/__init__.py delete mode 100644 opentelemetry-sdk/src/opentelemetry/sdk/distributedcontext/__init__.py delete mode 100644 opentelemetry-sdk/src/opentelemetry/sdk/distributedcontext/propagation/__init__.py rename opentelemetry-sdk/tests/{distributedcontext => correlationcontext}/__init__.py (100%) create mode 100644 opentelemetry-sdk/tests/correlationcontext/test_correlation_context.py delete mode 100644 opentelemetry-sdk/tests/distributedcontext/test_distributed_context.py diff --git a/opentelemetry-api/src/opentelemetry/correlationcontext/__init__.py b/opentelemetry-api/src/opentelemetry/correlationcontext/__init__.py new file mode 100644 index 00000000000..aa0e2d47451 --- /dev/null +++ b/opentelemetry-api/src/opentelemetry/correlationcontext/__init__.py @@ -0,0 +1,124 @@ +# Copyright 2020, OpenTelemetry Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import abc +import itertools +import string +import typing +from contextlib import contextmanager + +from opentelemetry.context import attach, get_value, set_value +from opentelemetry.context.context import Context + +CORRELATION_CONTEXT_KEY = "correlation-context" + + +class CorrelationContext(abc.ABC): + """A container for correlation context""" + + @abc.abstractmethod + def get_correlations(self, context: typing.Optional[Context] = None): + """ Returns the name/value pairs in the CorrelationContext + + Args: + context: the Context to use. If not set, uses current Context + + Returns: + name/value pairs in the CorrelationContext + """ + + @abc.abstractmethod + def get_correlation( + self, name, context: typing.Optional[Context] = None + ) -> typing.Optional[object]: + """ Provides access to the value for a name/value pair by a prior event + + Args: + name: the name of the value to retrieve + context: the Context to use. If not set, uses current Context + + Returns: + the value associated with the given name, or null if the given name is + not present. + """ + + @abc.abstractmethod + def set_correlation( + self, name, value, context: typing.Optional[Context] = None + ) -> Context: + """ + + Args: + name: the name of the value to set + value: the value to set + context: the Context to use. If not set, uses current Context + + Returns: + a Context with the value updated + """ + + @abc.abstractmethod + def remove_correlation( + self, name, context: typing.Optional[Context] = None + ) -> Context: + """ + + Args: + name: the name of the value to remove + context: the Context to use. If not set, uses current Context + + Returns: + a Context with the name/value removed + """ + + @abc.abstractmethod + def clear_correlations( + self, context: typing.Optional[Context] = None + ) -> Context: + """ + Args: + context: the Context to use. If not set, uses current Context + + Returns: + a Context with all correlations removed + """ + + +class DefaultCorrelationContext(CorrelationContext): + """ Default no-op implementation of CorrelationContext """ + + def get_correlations( + self, context: typing.Optional[Context] = None + ) -> typing.Dict: + return {} + + def get_correlation( + self, name, context: typing.Optional[Context] = None + ) -> typing.Optional[object]: + return None + + def set_correlation( + self, name, value, context: typing.Optional[Context] = None + ) -> Context: + return context + + def remove_correlation( + self, name, context: typing.Optional[Context] = None + ) -> Context: + return context + + def clear_correlations( + self, context: typing.Optional[Context] = None + ) -> Context: + return context diff --git a/opentelemetry-api/src/opentelemetry/distributedcontext/__init__.py b/opentelemetry-api/src/opentelemetry/distributedcontext/__init__.py deleted file mode 100644 index dbc7b7e79bd..00000000000 --- a/opentelemetry-api/src/opentelemetry/distributedcontext/__init__.py +++ /dev/null @@ -1,145 +0,0 @@ -# Copyright 2019, OpenTelemetry Authors -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. - -import itertools -import string -import typing -from contextlib import contextmanager - -from opentelemetry.context import attach, get_value, set_value -from opentelemetry.context.context import Context - -PRINTABLE = frozenset( - itertools.chain( - string.ascii_letters, string.digits, string.punctuation, " " - ) -) - - -class EntryMetadata: - """A class representing metadata of a DistributedContext entry - - Args: - entry_ttl: The time to live (in service hops) of an entry. Must be - initially set to either :attr:`EntryMetadata.NO_PROPAGATION` - or :attr:`EntryMetadata.UNLIMITED_PROPAGATION`. - """ - - NO_PROPAGATION = 0 - UNLIMITED_PROPAGATION = -1 - - def __init__(self, entry_ttl: int) -> None: - self.entry_ttl = entry_ttl - - -class EntryKey(str): - """A class representing a key for a DistributedContext entry""" - - def __new__(cls, value: str) -> "EntryKey": - return cls.create(value) - - @staticmethod - def create(value: str) -> "EntryKey": - # pylint: disable=len-as-condition - if not 0 < len(value) <= 255 or any(c not in PRINTABLE for c in value): - raise ValueError("Invalid EntryKey", value) - - return typing.cast(EntryKey, value) - - -class EntryValue(str): - """A class representing the value of a DistributedContext entry""" - - def __new__(cls, value: str) -> "EntryValue": - return cls.create(value) - - @staticmethod - def create(value: str) -> "EntryValue": - if any(c not in PRINTABLE for c in value): - raise ValueError("Invalid EntryValue", value) - - return typing.cast(EntryValue, value) - - -class Entry: - def __init__( - self, metadata: EntryMetadata, key: EntryKey, value: EntryValue - ) -> None: - self.metadata = metadata - self.key = key - self.value = value - - -class DistributedContext: - """A container for distributed context entries""" - - def __init__(self, entries: typing.Iterable[Entry]) -> None: - self._container = {entry.key: entry for entry in entries} - - def get_entries(self) -> typing.Iterable[Entry]: - """Returns an immutable iterator to entries.""" - return self._container.values() - - def get_entry_value(self, key: EntryKey) -> typing.Optional[EntryValue]: - """Returns the entry associated with a key or None - - Args: - key: the key with which to perform a lookup - """ - if key in self._container: - return self._container[key].value - return None - - -class DistributedContextManager: - def get_current_context( - self, context: typing.Optional[Context] = None - ) -> typing.Optional[DistributedContext]: - """Gets the current DistributedContext. - - Returns: - A DistributedContext instance representing the current context. - """ - - @contextmanager # type: ignore - def use_context( - self, context: DistributedContext - ) -> typing.Iterator[DistributedContext]: - """Context manager for controlling a DistributedContext lifetime. - - Set the context as the active DistributedContext. - - On exiting, the context manager will restore the parent - DistributedContext. - - Args: - context: A DistributedContext instance to make current. - """ - # pylint: disable=no-self-use - yield context - - -_DISTRIBUTED_CONTEXT_KEY = "DistributedContext" - - -def distributed_context_from_context( - context: typing.Optional[Context] = None, -) -> DistributedContext: - return get_value(_DISTRIBUTED_CONTEXT_KEY, context) # type: ignore - - -def with_distributed_context( - dctx: DistributedContext, context: typing.Optional[Context] = None -) -> None: - attach(set_value(_DISTRIBUTED_CONTEXT_KEY, dctx, context=context)) diff --git a/opentelemetry-api/tests/correlationcontext/test_correlation_context.py b/opentelemetry-api/tests/correlationcontext/test_correlation_context.py new file mode 100644 index 00000000000..223dbe401da --- /dev/null +++ b/opentelemetry-api/tests/correlationcontext/test_correlation_context.py @@ -0,0 +1,23 @@ +# Copyright 2020, OpenTelemetry Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import unittest + +from opentelemetry import correlationcontext + + +class TestCorrelationContext(unittest.TestCase): + def test_correlation_context(self): + default_context = correlationcontext.DefaultCorrelationContext() + self.assertEqual(default_context.get_correlations(), {}) diff --git a/opentelemetry-api/tests/distributedcontext/test_distributed_context.py b/opentelemetry-api/tests/distributedcontext/test_distributed_context.py deleted file mode 100644 index c730603b162..00000000000 --- a/opentelemetry-api/tests/distributedcontext/test_distributed_context.py +++ /dev/null @@ -1,112 +0,0 @@ -# Copyright 2019, OpenTelemetry Authors -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. - -import unittest - -from opentelemetry import distributedcontext - - -class TestEntryMetadata(unittest.TestCase): - def test_entry_ttl_no_propagation(self): - metadata = distributedcontext.EntryMetadata( - distributedcontext.EntryMetadata.NO_PROPAGATION - ) - self.assertEqual(metadata.entry_ttl, 0) - - def test_entry_ttl_unlimited_propagation(self): - metadata = distributedcontext.EntryMetadata( - distributedcontext.EntryMetadata.UNLIMITED_PROPAGATION - ) - self.assertEqual(metadata.entry_ttl, -1) - - -class TestEntryKey(unittest.TestCase): - def test_create_empty(self): - with self.assertRaises(ValueError): - distributedcontext.EntryKey.create("") - - def test_create_too_long(self): - with self.assertRaises(ValueError): - distributedcontext.EntryKey.create("a" * 256) - - def test_create_invalid_character(self): - with self.assertRaises(ValueError): - distributedcontext.EntryKey.create("\x00") - - def test_create_valid(self): - key = distributedcontext.EntryKey.create("ok") - self.assertEqual(key, "ok") - - def test_key_new(self): - key = distributedcontext.EntryKey("ok") - self.assertEqual(key, "ok") - - -class TestEntryValue(unittest.TestCase): - def test_create_invalid_character(self): - with self.assertRaises(ValueError): - distributedcontext.EntryValue.create("\x00") - - def test_create_valid(self): - key = distributedcontext.EntryValue.create("ok") - self.assertEqual(key, "ok") - - def test_key_new(self): - key = distributedcontext.EntryValue("ok") - self.assertEqual(key, "ok") - - -class TestDistributedContext(unittest.TestCase): - def setUp(self): - entry = self.entry = distributedcontext.Entry( - distributedcontext.EntryMetadata( - distributedcontext.EntryMetadata.NO_PROPAGATION - ), - distributedcontext.EntryKey("key"), - distributedcontext.EntryValue("value"), - ) - self.context = distributedcontext.DistributedContext((entry,)) - - def test_get_entries(self): - self.assertIn(self.entry, self.context.get_entries()) - - def test_get_entry_value_present(self): - value = self.context.get_entry_value(self.entry.key) - self.assertIs(value, self.entry.value) - - def test_get_entry_value_missing(self): - key = distributedcontext.EntryKey("missing") - value = self.context.get_entry_value(key) - self.assertIsNone(value) - - -class TestDistributedContextManager(unittest.TestCase): - def setUp(self): - self.manager = distributedcontext.DistributedContextManager() - - def test_get_current_context(self): - self.assertIsNone(self.manager.get_current_context()) - - def test_use_context(self): - expected = distributedcontext.DistributedContext( - ( - distributedcontext.Entry( - distributedcontext.EntryMetadata(0), - distributedcontext.EntryKey("0"), - distributedcontext.EntryValue(""), - ), - ) - ) - with self.manager.use_context(expected) as output: - self.assertIs(output, expected) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/correlationcontext/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/correlationcontext/__init__.py new file mode 100644 index 00000000000..3ffa65ad54e --- /dev/null +++ b/opentelemetry-sdk/src/opentelemetry/sdk/correlationcontext/__init__.py @@ -0,0 +1,72 @@ +# Copyright 2020, OpenTelemetry Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import typing +from contextlib import contextmanager + +from opentelemetry import correlationcontext as cctx_api +from opentelemetry.context import get_value, set_value, get_current +from opentelemetry.context.context import Context + + +class CorrelationContext(cctx_api.CorrelationContext): + def get_correlations(self, context: typing.Optional[Context] = None): + correlations = get_value( + cctx_api.CORRELATION_CONTEXT_KEY, context=context + ) + if correlations: + return correlations + return {} + + def get_correlation( + self, name, context: typing.Optional[Context] = None + ) -> typing.Optional[object]: + correlations = get_value( + cctx_api.CORRELATION_CONTEXT_KEY, context=context + ) + if correlations: + return correlations.get(name) + return None + + def set_correlation( + self, name, value, context: typing.Optional[Context] = None + ) -> Context: + correlations = get_value( + cctx_api.CORRELATION_CONTEXT_KEY, context=context + ) + if correlations: + correlations[name] = value + else: + correlations = {name: value} + return set_value( + cctx_api.CORRELATION_CONTEXT_KEY, correlations, context=context + ) + + def remove_correlation( + self, name, context: typing.Optional[Context] = None + ) -> Context: + correlations = get_value( + cctx_api.CORRELATION_CONTEXT_KEY, context=context + ) + if correlations and name in correlations: + del correlations[name] + + return set_value( + cctx_api.CORRELATION_CONTEXT_KEY, correlations, context=context + ) + + def clear_correlations( + self, context: typing.Optional[Context] = None + ) -> Context: + return set_value(cctx_api.CORRELATION_CONTEXT_KEY, {}, context=context) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/distributedcontext/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/distributedcontext/__init__.py deleted file mode 100644 index 7a0a66a8a9a..00000000000 --- a/opentelemetry-sdk/src/opentelemetry/sdk/distributedcontext/__init__.py +++ /dev/null @@ -1,61 +0,0 @@ -# Copyright 2019, OpenTelemetry Authors -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. - -import typing -from contextlib import contextmanager - -from opentelemetry import distributedcontext as dctx_api -from opentelemetry.context import Context, get_value, set_value -from opentelemetry.distributedcontext import ( - distributed_context_from_context, - with_distributed_context, -) - - -class DistributedContextManager(dctx_api.DistributedContextManager): - """See `opentelemetry.distributedcontext.DistributedContextManager` - - """ - - def get_current_context( - self, context: typing.Optional[Context] = None - ) -> typing.Optional[dctx_api.DistributedContext]: - """Gets the current DistributedContext. - - Returns: - A DistributedContext instance representing the current context. - """ - return distributed_context_from_context(context=context) - - @contextmanager - def use_context( - self, context: dctx_api.DistributedContext - ) -> typing.Iterator[dctx_api.DistributedContext]: - """Context manager for controlling a DistributedContext lifetime. - - Set the context as the active DistributedContext. - - On exiting, the context manager will restore the parent - DistributedContext. - - Args: - context: A DistributedContext instance to make current. - """ - snapshot = distributed_context_from_context() - with_distributed_context(context) - - try: - yield context - finally: - with_distributed_context(snapshot) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/distributedcontext/propagation/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/distributedcontext/propagation/__init__.py deleted file mode 100644 index e69de29bb2d..00000000000 diff --git a/opentelemetry-sdk/tests/distributedcontext/__init__.py b/opentelemetry-sdk/tests/correlationcontext/__init__.py similarity index 100% rename from opentelemetry-sdk/tests/distributedcontext/__init__.py rename to opentelemetry-sdk/tests/correlationcontext/__init__.py diff --git a/opentelemetry-sdk/tests/correlationcontext/test_correlation_context.py b/opentelemetry-sdk/tests/correlationcontext/test_correlation_context.py new file mode 100644 index 00000000000..31da110143c --- /dev/null +++ b/opentelemetry-sdk/tests/correlationcontext/test_correlation_context.py @@ -0,0 +1,29 @@ +# Copyright 2019, OpenTelemetry Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import unittest + +from opentelemetry.sdk import correlationcontext + + +class TestCorrelationContextManager(unittest.TestCase): + def test_use_context(self): + cctx = correlationcontext.CorrelationContext() + self.assertEqual({}, cctx.get_correlations()) + + ctx = cctx.set_correlation("test", "value") + self.assertEqual(cctx.get_correlation("test", context=ctx), "value") + + ctx = cctx.set_correlation("test", "value2", context=ctx) + self.assertEqual(cctx.get_correlation("test", context=ctx), "value2") diff --git a/opentelemetry-sdk/tests/distributedcontext/test_distributed_context.py b/opentelemetry-sdk/tests/distributedcontext/test_distributed_context.py deleted file mode 100644 index eddb61330dc..00000000000 --- a/opentelemetry-sdk/tests/distributedcontext/test_distributed_context.py +++ /dev/null @@ -1,42 +0,0 @@ -# Copyright 2019, OpenTelemetry Authors -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. - -import unittest - -from opentelemetry import distributedcontext as dctx_api -from opentelemetry.sdk import distributedcontext - - -class TestDistributedContextManager(unittest.TestCase): - def setUp(self): - self.manager = distributedcontext.DistributedContextManager() - - def test_use_context(self): - # Context is None initially - self.assertIsNone(self.manager.get_current_context()) - - # Start initial context - dctx = dctx_api.DistributedContext(()) - with self.manager.use_context(dctx) as current: - self.assertIs(current, dctx) - self.assertIs(self.manager.get_current_context(), dctx) - - # Context is overridden - nested_dctx = dctx_api.DistributedContext(()) - with self.manager.use_context(nested_dctx) as current: - self.assertIs(current, nested_dctx) - self.assertIs(self.manager.get_current_context(), nested_dctx) - - # Context is restored - self.assertIs(self.manager.get_current_context(), dctx) From 764ee23ad21eb1d9a22830c10290fe3291c27815 Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Tue, 10 Mar 2020 15:21:45 -0600 Subject: [PATCH 02/18] Fix lint --- .../src/opentelemetry/correlationcontext/__init__.py | 8 ++------ .../src/opentelemetry/sdk/correlationcontext/__init__.py | 3 +-- 2 files changed, 3 insertions(+), 8 deletions(-) diff --git a/opentelemetry-api/src/opentelemetry/correlationcontext/__init__.py b/opentelemetry-api/src/opentelemetry/correlationcontext/__init__.py index aa0e2d47451..402daf1614b 100644 --- a/opentelemetry-api/src/opentelemetry/correlationcontext/__init__.py +++ b/opentelemetry-api/src/opentelemetry/correlationcontext/__init__.py @@ -13,12 +13,8 @@ # limitations under the License. import abc -import itertools -import string import typing -from contextlib import contextmanager -from opentelemetry.context import attach, get_value, set_value from opentelemetry.context.context import Context CORRELATION_CONTEXT_KEY = "correlation-context" @@ -30,7 +26,7 @@ class CorrelationContext(abc.ABC): @abc.abstractmethod def get_correlations(self, context: typing.Optional[Context] = None): """ Returns the name/value pairs in the CorrelationContext - + Args: context: the Context to use. If not set, uses current Context @@ -43,7 +39,7 @@ def get_correlation( self, name, context: typing.Optional[Context] = None ) -> typing.Optional[object]: """ Provides access to the value for a name/value pair by a prior event - + Args: name: the name of the value to retrieve context: the Context to use. If not set, uses current Context diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/correlationcontext/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/correlationcontext/__init__.py index 3ffa65ad54e..4acbd2fe518 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/correlationcontext/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/correlationcontext/__init__.py @@ -13,10 +13,9 @@ # limitations under the License. import typing -from contextlib import contextmanager from opentelemetry import correlationcontext as cctx_api -from opentelemetry.context import get_value, set_value, get_current +from opentelemetry.context import get_value, set_value from opentelemetry.context.context import Context From 868987dd6f317e1258f065d862cb28bfe3844e61 Mon Sep 17 00:00:00 2001 From: Diego Hurtado Date: Tue, 10 Mar 2020 16:49:04 -0600 Subject: [PATCH 03/18] Fix mypy --- .../correlationcontext/__init__.py | 28 +++++++++---------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/opentelemetry-api/src/opentelemetry/correlationcontext/__init__.py b/opentelemetry-api/src/opentelemetry/correlationcontext/__init__.py index 402daf1614b..e9942151b7c 100644 --- a/opentelemetry-api/src/opentelemetry/correlationcontext/__init__.py +++ b/opentelemetry-api/src/opentelemetry/correlationcontext/__init__.py @@ -24,7 +24,9 @@ class CorrelationContext(abc.ABC): """A container for correlation context""" @abc.abstractmethod - def get_correlations(self, context: typing.Optional[Context] = None): + def get_correlations( + self, context: typing.Optional[Context] = None + ) -> typing.Optional[object]: """ Returns the name/value pairs in the CorrelationContext Args: @@ -36,7 +38,7 @@ def get_correlations(self, context: typing.Optional[Context] = None): @abc.abstractmethod def get_correlation( - self, name, context: typing.Optional[Context] = None + self, name: str, context: typing.Optional[Context] = None ) -> typing.Optional[object]: """ Provides access to the value for a name/value pair by a prior event @@ -51,7 +53,7 @@ def get_correlation( @abc.abstractmethod def set_correlation( - self, name, value, context: typing.Optional[Context] = None + self, name: str, value: object, context: typing.Optional[Context] = None ) -> Context: """ @@ -66,7 +68,7 @@ def set_correlation( @abc.abstractmethod def remove_correlation( - self, name, context: typing.Optional[Context] = None + self, name: str, context: typing.Optional[Context] = None ) -> Context: """ @@ -96,25 +98,23 @@ class DefaultCorrelationContext(CorrelationContext): def get_correlations( self, context: typing.Optional[Context] = None - ) -> typing.Dict: + ) -> typing.Dict[object, object]: return {} def get_correlation( - self, name, context: typing.Optional[Context] = None + self, name: str, context: typing.Optional[Context] = None ) -> typing.Optional[object]: return None def set_correlation( - self, name, value, context: typing.Optional[Context] = None + self, name: str, value: object, context: typing.Optional[Context] = None ) -> Context: - return context + return context # type: ignore def remove_correlation( - self, name, context: typing.Optional[Context] = None + self, name: str, context: typing.Optional[Context] = None ) -> Context: - return context + return context # type: ignore - def clear_correlations( - self, context: typing.Optional[Context] = None - ) -> Context: - return context + def clear_correlations(self, context: typing.Optional[Context] = None) -> Context: + return context # type: ignore From 13ddb5cd5beaee282c754511b860ce1f2e4d5fcf Mon Sep 17 00:00:00 2001 From: Alex Boten Date: Wed, 11 Mar 2020 16:23:41 -0700 Subject: [PATCH 04/18] adding some unit tests --- .../correlationcontext/__init__.py | 14 ++++++-- .../test_correlation_context.py | 34 ++++++++++++++++++- 2 files changed, 44 insertions(+), 4 deletions(-) diff --git a/opentelemetry-api/src/opentelemetry/correlationcontext/__init__.py b/opentelemetry-api/src/opentelemetry/correlationcontext/__init__.py index e9942151b7c..721826db936 100644 --- a/opentelemetry-api/src/opentelemetry/correlationcontext/__init__.py +++ b/opentelemetry-api/src/opentelemetry/correlationcontext/__init__.py @@ -53,7 +53,10 @@ def get_correlation( @abc.abstractmethod def set_correlation( - self, name: str, value: object, context: typing.Optional[Context] = None + self, + name: str, + value: object, + context: typing.Optional[Context] = None, ) -> Context: """ @@ -107,7 +110,10 @@ def get_correlation( return None def set_correlation( - self, name: str, value: object, context: typing.Optional[Context] = None + self, + name: str, + value: object, + context: typing.Optional[Context] = None, ) -> Context: return context # type: ignore @@ -116,5 +122,7 @@ def remove_correlation( ) -> Context: return context # type: ignore - def clear_correlations(self, context: typing.Optional[Context] = None) -> Context: + def clear_correlations( + self, context: typing.Optional[Context] = None + ) -> Context: return context # type: ignore diff --git a/opentelemetry-sdk/tests/correlationcontext/test_correlation_context.py b/opentelemetry-sdk/tests/correlationcontext/test_correlation_context.py index 31da110143c..0ebf440fb1c 100644 --- a/opentelemetry-sdk/tests/correlationcontext/test_correlation_context.py +++ b/opentelemetry-sdk/tests/correlationcontext/test_correlation_context.py @@ -18,7 +18,7 @@ class TestCorrelationContextManager(unittest.TestCase): - def test_use_context(self): + def test_set_correlation(self): cctx = correlationcontext.CorrelationContext() self.assertEqual({}, cctx.get_correlations()) @@ -27,3 +27,35 @@ def test_use_context(self): ctx = cctx.set_correlation("test", "value2", context=ctx) self.assertEqual(cctx.get_correlation("test", context=ctx), "value2") + + def test_set_multiple_correlations(self): + cctx = correlationcontext.CorrelationContext() + ctx = cctx.set_correlation("test", "value") + ctx = cctx.set_correlation("test2", "value2", context=ctx) + self.assertEqual(cctx.get_correlation("test", context=ctx), "value") + self.assertEqual(cctx.get_correlation("test2", context=ctx), "value2") + self.assertEqual( + cctx.get_correlations(context=ctx), + {"test": "value", "test2": "value2"}, + ) + + def test_remove_correlations(self): + cctx = correlationcontext.CorrelationContext() + self.assertEqual({}, cctx.get_correlations()) + + ctx = cctx.set_correlation("test", "value") + ctx = cctx.set_correlation("test2", "value2", context=ctx) + ctx = cctx.remove_correlation("test", context=ctx) + self.assertEqual(cctx.get_correlation("test", context=ctx), None) + self.assertEqual(cctx.get_correlation("test2", context=ctx), "value2") + + def test_clear_correlations(self): + cctx = correlationcontext.CorrelationContext() + self.assertEqual({}, cctx.get_correlations()) + + ctx = cctx.set_correlation("test", "value") + self.assertEqual(cctx.get_correlation("test", context=ctx), "value") + + ctx = cctx.clear_correlations(context=ctx) + self.assertEqual(cctx.get_correlation("test", context=ctx), None) + From bba033e0726989e417fef0c37ac0760b6239f642 Mon Sep 17 00:00:00 2001 From: Alex Boten Date: Wed, 11 Mar 2020 16:41:07 -0700 Subject: [PATCH 05/18] fix lint --- .../tests/correlationcontext/test_correlation_context.py | 1 - 1 file changed, 1 deletion(-) diff --git a/opentelemetry-sdk/tests/correlationcontext/test_correlation_context.py b/opentelemetry-sdk/tests/correlationcontext/test_correlation_context.py index 0ebf440fb1c..be96e824143 100644 --- a/opentelemetry-sdk/tests/correlationcontext/test_correlation_context.py +++ b/opentelemetry-sdk/tests/correlationcontext/test_correlation_context.py @@ -58,4 +58,3 @@ def test_clear_correlations(self): ctx = cctx.clear_correlations(context=ctx) self.assertEqual(cctx.get_correlation("test", context=ctx), None) - From b6fc6cf37dfc39ef659559d3803c95da563c479d Mon Sep 17 00:00:00 2001 From: Alex Boten Date: Thu, 12 Mar 2020 12:11:52 -0700 Subject: [PATCH 06/18] adding propagator --- .../correlationcontext/__init__.py | 191 ++++++++---------- .../propagation/__init__.py | 101 +++++++++ .../test_correlation_context.py | 43 +++- .../test_correlation_context_propagation.py | 115 +++++++++++ .../sdk/correlationcontext/__init__.py | 71 ------- .../test_correlation_context.py | 60 ------ 6 files changed, 339 insertions(+), 242 deletions(-) create mode 100644 opentelemetry-api/src/opentelemetry/correlationcontext/propagation/__init__.py create mode 100644 opentelemetry-api/tests/correlationcontext/test_correlation_context_propagation.py delete mode 100644 opentelemetry-sdk/src/opentelemetry/sdk/correlationcontext/__init__.py delete mode 100644 opentelemetry-sdk/tests/correlationcontext/test_correlation_context.py diff --git a/opentelemetry-api/src/opentelemetry/correlationcontext/__init__.py b/opentelemetry-api/src/opentelemetry/correlationcontext/__init__.py index 721826db936..c07c702fc11 100644 --- a/opentelemetry-api/src/opentelemetry/correlationcontext/__init__.py +++ b/opentelemetry-api/src/opentelemetry/correlationcontext/__init__.py @@ -15,114 +15,93 @@ import abc import typing +from opentelemetry.context import get_value, set_value from opentelemetry.context.context import Context CORRELATION_CONTEXT_KEY = "correlation-context" -class CorrelationContext(abc.ABC): - """A container for correlation context""" - - @abc.abstractmethod - def get_correlations( - self, context: typing.Optional[Context] = None - ) -> typing.Optional[object]: - """ Returns the name/value pairs in the CorrelationContext - - Args: - context: the Context to use. If not set, uses current Context - - Returns: - name/value pairs in the CorrelationContext - """ - - @abc.abstractmethod - def get_correlation( - self, name: str, context: typing.Optional[Context] = None - ) -> typing.Optional[object]: - """ Provides access to the value for a name/value pair by a prior event - - Args: - name: the name of the value to retrieve - context: the Context to use. If not set, uses current Context - - Returns: - the value associated with the given name, or null if the given name is - not present. - """ - - @abc.abstractmethod - def set_correlation( - self, - name: str, - value: object, - context: typing.Optional[Context] = None, - ) -> Context: - """ - - Args: - name: the name of the value to set - value: the value to set - context: the Context to use. If not set, uses current Context - - Returns: - a Context with the value updated - """ - - @abc.abstractmethod - def remove_correlation( - self, name: str, context: typing.Optional[Context] = None - ) -> Context: - """ - - Args: - name: the name of the value to remove - context: the Context to use. If not set, uses current Context - - Returns: - a Context with the name/value removed - """ - - @abc.abstractmethod - def clear_correlations( - self, context: typing.Optional[Context] = None - ) -> Context: - """ - Args: - context: the Context to use. If not set, uses current Context - - Returns: - a Context with all correlations removed - """ - - -class DefaultCorrelationContext(CorrelationContext): - """ Default no-op implementation of CorrelationContext """ - - def get_correlations( - self, context: typing.Optional[Context] = None - ) -> typing.Dict[object, object]: - return {} - - def get_correlation( - self, name: str, context: typing.Optional[Context] = None - ) -> typing.Optional[object]: - return None - - def set_correlation( - self, - name: str, - value: object, - context: typing.Optional[Context] = None, - ) -> Context: - return context # type: ignore - - def remove_correlation( - self, name: str, context: typing.Optional[Context] = None - ) -> Context: - return context # type: ignore - - def clear_correlations( - self, context: typing.Optional[Context] = None - ) -> Context: - return context # type: ignore +def get_correlations( + context: typing.Optional[Context] = None, +) -> typing.Dict[str, object]: + """ Returns the name/value pairs in the CorrelationContext + + Args: + context: the Context to use. If not set, uses current Context + + Returns: + name/value pairs in the CorrelationContext + """ + correlations = get_value(CORRELATION_CONTEXT_KEY, context=context) + if correlations: + return correlations + return {} + + +def get_correlation( + name: str, context: typing.Optional[Context] = None +) -> typing.Optional[object]: + """ Provides access to the value for a name/value pair in the CorrelationContext + + Args: + name: the name of the value to retrieve + context: the Context to use. If not set, uses current Context + + Returns: + the value associated with the given name, or null if the given name is + not present. + """ + correlations = get_value(CORRELATION_CONTEXT_KEY, context=context) + if correlations: + return correlations.get(name) + return None + + +def set_correlation( + name: str, value, context: typing.Optional[Context] = None +) -> Context: + """Sets a value in the CorrelationContext + + Args: + name: the name of the value to set + value: the value to set + context: the Context to use. If not set, uses current Context + + Returns: + a Context with the value updated + """ + correlations = get_value(CORRELATION_CONTEXT_KEY, context=context) + if correlations: + correlations[name] = value + else: + correlations = {name: value} + return set_value(CORRELATION_CONTEXT_KEY, correlations, context=context) + + +def remove_correlation( + name: str, context: typing.Optional[Context] = None +) -> Context: + """Removes a value from the CorrelationContext + Args: + name: the name of the value to remove + context: the Context to use. If not set, uses current Context + + Returns: + a Context with the name/value removed + """ + correlations = get_value(CORRELATION_CONTEXT_KEY, context=context) + if correlations and name in correlations: + del correlations[name] + + return set_value(CORRELATION_CONTEXT_KEY, correlations, context=context) + + +def clear_correlations(context: typing.Optional[Context] = None) -> Context: + """Removes all values from the CorrelationContext + Args: + context: the Context to use. If not set, uses current Context + + Returns: + a Context with all correlations removed + """ + return set_value(CORRELATION_CONTEXT_KEY, {}, context=context) diff --git a/opentelemetry-api/src/opentelemetry/correlationcontext/propagation/__init__.py b/opentelemetry-api/src/opentelemetry/correlationcontext/propagation/__init__.py new file mode 100644 index 00000000000..51001020a08 --- /dev/null +++ b/opentelemetry-api/src/opentelemetry/correlationcontext/propagation/__init__.py @@ -0,0 +1,101 @@ +# Copyright 2020, OpenTelemetry Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# +import re +import typing +import urllib.parse + +from opentelemetry import correlationcontext +from opentelemetry.context import get_current +from opentelemetry.context.context import Context +from opentelemetry.trace.propagation import httptextformat + + +class CorrelationContextPropagator(httptextformat.HTTPTextFormat): + _CORRELATION_CONTEXT_HEADER_NAME = "otcorrelationcontext" + + def extract( + self, + get_from_carrier: httptextformat.Getter[ + httptextformat.HTTPTextFormatT + ], + carrier: httptextformat.HTTPTextFormatT, + context: typing.Optional[Context] = None, + ) -> Context: + """ Extract CorrelationContext from the carrier. + + See `opentelemetry.trace.propagation.httptextformat.HTTPTextFormat.extract` + """ + + if not context: + context = get_current() + + header = get_from_carrier( + carrier, self._CORRELATION_CONTEXT_HEADER_NAME + ) + + if not header: + return context + + correlations = header.split(",") + + for correlation in correlations: + try: + name, value = correlation.split("=", 1) + except Exception: # pylint: disable=broad-except + continue + context = correlationcontext.set_correlation( + name.strip(), + urllib.parse.unquote(value).strip(), + context=context, + ) + + return context + + def inject( + self, + set_in_carrier: httptextformat.Setter[httptextformat.HTTPTextFormatT], + carrier: httptextformat.HTTPTextFormatT, + context: typing.Optional[Context] = None, + ) -> None: + """Injects CorrelationContext into the carrier. + + See `opentelemetry.trace.propagation.httptextformat.HTTPTextFormat.inject` + """ + correlations = correlationcontext.get_correlations(context=context) + if not correlations: + return + + correlation_context_string = _format_correlations(correlations) + set_in_carrier( + carrier, + self._CORRELATION_CONTEXT_HEADER_NAME, + correlation_context_string, + ) + + +def _format_correlations(correlations: typing.Dict[str, object]) -> str: + """Format correlations into a string. + + Args: + correlations: the correlations to format + + Returns: + A string that adheres to the w3c correlationcontext + header format. + """ + return ",".join( + key + "=" + urllib.parse.quote_plus(str(value).lower()) + for key, value in correlations.items() + ) diff --git a/opentelemetry-api/tests/correlationcontext/test_correlation_context.py b/opentelemetry-api/tests/correlationcontext/test_correlation_context.py index 223dbe401da..5ea2ff69d2d 100644 --- a/opentelemetry-api/tests/correlationcontext/test_correlation_context.py +++ b/opentelemetry-api/tests/correlationcontext/test_correlation_context.py @@ -14,10 +14,43 @@ import unittest -from opentelemetry import correlationcontext +from opentelemetry import correlationcontext as cctx -class TestCorrelationContext(unittest.TestCase): - def test_correlation_context(self): - default_context = correlationcontext.DefaultCorrelationContext() - self.assertEqual(default_context.get_correlations(), {}) +class TestCorrelationContextManager(unittest.TestCase): + def test_set_correlation(self): + self.assertEqual({}, cctx.get_correlations()) + + ctx = cctx.set_correlation("test", "value") + self.assertEqual(cctx.get_correlation("test", context=ctx), "value") + + ctx = cctx.set_correlation("test", "value2", context=ctx) + self.assertEqual(cctx.get_correlation("test", context=ctx), "value2") + + def test_set_multiple_correlations(self): + ctx = cctx.set_correlation("test", "value") + ctx = cctx.set_correlation("test2", "value2", context=ctx) + self.assertEqual(cctx.get_correlation("test", context=ctx), "value") + self.assertEqual(cctx.get_correlation("test2", context=ctx), "value2") + self.assertEqual( + cctx.get_correlations(context=ctx), + {"test": "value", "test2": "value2"}, + ) + + def test_remove_correlations(self): + self.assertEqual({}, cctx.get_correlations()) + + ctx = cctx.set_correlation("test", "value") + ctx = cctx.set_correlation("test2", "value2", context=ctx) + ctx = cctx.remove_correlation("test", context=ctx) + self.assertEqual(cctx.get_correlation("test", context=ctx), None) + self.assertEqual(cctx.get_correlation("test2", context=ctx), "value2") + + def test_clear_correlations(self): + self.assertEqual({}, cctx.get_correlations()) + + ctx = cctx.set_correlation("test", "value") + self.assertEqual(cctx.get_correlation("test", context=ctx), "value") + + ctx = cctx.clear_correlations(context=ctx) + self.assertEqual(cctx.get_correlation("test", context=ctx), None) diff --git a/opentelemetry-api/tests/correlationcontext/test_correlation_context_propagation.py b/opentelemetry-api/tests/correlationcontext/test_correlation_context_propagation.py new file mode 100644 index 00000000000..18eb8ab10e9 --- /dev/null +++ b/opentelemetry-api/tests/correlationcontext/test_correlation_context_propagation.py @@ -0,0 +1,115 @@ +# Copyright 2020, OpenTelemetry Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# +import typing +import unittest + +from opentelemetry import correlationcontext +from opentelemetry.context import get_current +from opentelemetry.correlationcontext.propagation import ( + CorrelationContextPropagator, +) + + +def get_as_list( + dict_object: typing.Dict[str, typing.List[str]], key: str +) -> typing.List[str]: + value = dict_object.get(key) + return value if value is not None else [] + + +class TestCorrelationContextPropagation(unittest.TestCase): + def setUp(self): + self.propagator = CorrelationContextPropagator() + + def _extract(self, header_value): + """Test helper""" + header = {"otcorrelationcontext": header_value} + return correlationcontext.get_correlations( + self.propagator.extract(get_as_list, header) + ) + + def _inject(self, values): + """Test helper""" + ctx = get_current() + for k, v in values.items(): + ctx = correlationcontext.set_correlation(k, v, context=ctx) + output = {} + self.propagator.inject(dict.__setitem__, output, context=ctx) + print(output) + return output.get("otcorrelationcontext") + + def test_no_context_header(self): + header = {} # type:typing.Dict[str, typing.List[str]] + self.assertEqual(self._extract(header), {}) + + def test_valid_header(self): + header = "key1=val1,key2=val2" + expected = {"key1": "val1", "key2": "val2"} + self.assertEqual(self._extract(header), expected) + + def test_valid_header_with_space(self): + header = "key1 = val1, key2 =val2 " + expected = {"key1": "val1", "key2": "val2"} + self.assertEqual(self._extract(header), expected) + + def test_valid_header_with_properties(self): + header = "key1=val1,key2=val2;prop=1" + expected = {"key1": "val1", "key2": "val2;prop=1"} + self.assertEqual(self._extract(header), expected) + + def test_valid_header_with_url_escaped_comma(self): + header = "key1=val1,key2=val2%2Cval3" + expected = {"key1": "val1", "key2": "val2,val3"} + self.assertEqual(self._extract(header), expected) + + def test_valid_header_with_invalid_value(self): + header = "key1=val1,key2=val2,a,val3" + expected = {"key1": "val1", "key2": "val2"} + self.assertEqual(self._extract(header), expected) + + def test_valid_header_with_empty_value(self): + header = "key1=,key2=val2" + expected = {"key1": "", "key2": "val2"} + self.assertEqual(self._extract(header), expected) + + def test_invalid_header(self): + header = "header1" + expected = {} + self.assertEqual(self._extract(header), expected) + + def test_inject(self): + expected = "key1=val1,key2=val2" + values = { + "key1": "val1", + "key2": "val2", + } + self.assertEqual(self._inject(values), expected) + + def test_inject_escaped_values(self): + expected = "key1=val1%2Cval2,key2=val3%3D4" + values = { + "key1": "val1,val2", + "key2": "val3=4", + } + self.assertEqual(self._inject(values), expected) + + def test_inject_non_string_values(self): + expected = "key1=true,key2=123,key3=123.567" + values = { + "key1": True, + "key2": 123, + "key3": 123.567, + } + self.assertEqual(self._inject(values), expected) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/correlationcontext/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/correlationcontext/__init__.py deleted file mode 100644 index 4acbd2fe518..00000000000 --- a/opentelemetry-sdk/src/opentelemetry/sdk/correlationcontext/__init__.py +++ /dev/null @@ -1,71 +0,0 @@ -# Copyright 2020, OpenTelemetry Authors -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. - -import typing - -from opentelemetry import correlationcontext as cctx_api -from opentelemetry.context import get_value, set_value -from opentelemetry.context.context import Context - - -class CorrelationContext(cctx_api.CorrelationContext): - def get_correlations(self, context: typing.Optional[Context] = None): - correlations = get_value( - cctx_api.CORRELATION_CONTEXT_KEY, context=context - ) - if correlations: - return correlations - return {} - - def get_correlation( - self, name, context: typing.Optional[Context] = None - ) -> typing.Optional[object]: - correlations = get_value( - cctx_api.CORRELATION_CONTEXT_KEY, context=context - ) - if correlations: - return correlations.get(name) - return None - - def set_correlation( - self, name, value, context: typing.Optional[Context] = None - ) -> Context: - correlations = get_value( - cctx_api.CORRELATION_CONTEXT_KEY, context=context - ) - if correlations: - correlations[name] = value - else: - correlations = {name: value} - return set_value( - cctx_api.CORRELATION_CONTEXT_KEY, correlations, context=context - ) - - def remove_correlation( - self, name, context: typing.Optional[Context] = None - ) -> Context: - correlations = get_value( - cctx_api.CORRELATION_CONTEXT_KEY, context=context - ) - if correlations and name in correlations: - del correlations[name] - - return set_value( - cctx_api.CORRELATION_CONTEXT_KEY, correlations, context=context - ) - - def clear_correlations( - self, context: typing.Optional[Context] = None - ) -> Context: - return set_value(cctx_api.CORRELATION_CONTEXT_KEY, {}, context=context) diff --git a/opentelemetry-sdk/tests/correlationcontext/test_correlation_context.py b/opentelemetry-sdk/tests/correlationcontext/test_correlation_context.py deleted file mode 100644 index be96e824143..00000000000 --- a/opentelemetry-sdk/tests/correlationcontext/test_correlation_context.py +++ /dev/null @@ -1,60 +0,0 @@ -# Copyright 2019, OpenTelemetry Authors -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. - -import unittest - -from opentelemetry.sdk import correlationcontext - - -class TestCorrelationContextManager(unittest.TestCase): - def test_set_correlation(self): - cctx = correlationcontext.CorrelationContext() - self.assertEqual({}, cctx.get_correlations()) - - ctx = cctx.set_correlation("test", "value") - self.assertEqual(cctx.get_correlation("test", context=ctx), "value") - - ctx = cctx.set_correlation("test", "value2", context=ctx) - self.assertEqual(cctx.get_correlation("test", context=ctx), "value2") - - def test_set_multiple_correlations(self): - cctx = correlationcontext.CorrelationContext() - ctx = cctx.set_correlation("test", "value") - ctx = cctx.set_correlation("test2", "value2", context=ctx) - self.assertEqual(cctx.get_correlation("test", context=ctx), "value") - self.assertEqual(cctx.get_correlation("test2", context=ctx), "value2") - self.assertEqual( - cctx.get_correlations(context=ctx), - {"test": "value", "test2": "value2"}, - ) - - def test_remove_correlations(self): - cctx = correlationcontext.CorrelationContext() - self.assertEqual({}, cctx.get_correlations()) - - ctx = cctx.set_correlation("test", "value") - ctx = cctx.set_correlation("test2", "value2", context=ctx) - ctx = cctx.remove_correlation("test", context=ctx) - self.assertEqual(cctx.get_correlation("test", context=ctx), None) - self.assertEqual(cctx.get_correlation("test2", context=ctx), "value2") - - def test_clear_correlations(self): - cctx = correlationcontext.CorrelationContext() - self.assertEqual({}, cctx.get_correlations()) - - ctx = cctx.set_correlation("test", "value") - self.assertEqual(cctx.get_correlation("test", context=ctx), "value") - - ctx = cctx.clear_correlations(context=ctx) - self.assertEqual(cctx.get_correlation("test", context=ctx), None) From 495c8408c59da332d4a8d3c4f5066be4bf793aaa Mon Sep 17 00:00:00 2001 From: Alex Boten Date: Thu, 12 Mar 2020 13:23:47 -0700 Subject: [PATCH 07/18] add correlation context propagator to global propagator --- .../src/opentelemetry/propagators/__init__.py | 8 +- .../propagators/test_global_httptextformat.py | 74 +++++++++++++++++++ 2 files changed, 80 insertions(+), 2 deletions(-) create mode 100644 opentelemetry-api/tests/propagators/test_global_httptextformat.py diff --git a/opentelemetry-api/src/opentelemetry/propagators/__init__.py b/opentelemetry-api/src/opentelemetry/propagators/__init__.py index f9b537cd866..264fe06b5c3 100644 --- a/opentelemetry-api/src/opentelemetry/propagators/__init__.py +++ b/opentelemetry-api/src/opentelemetry/propagators/__init__.py @@ -58,6 +58,10 @@ def example_route(): import opentelemetry.trace as trace from opentelemetry.context import get_current from opentelemetry.context.context import Context +from opentelemetry.correlationcontext.propagation import ( + CorrelationContextPropagator, +) +from opentelemetry.propagators import composite from opentelemetry.trace.propagation import httptextformat from opentelemetry.trace.propagation.tracecontexthttptextformat import ( TraceContextHTTPTextFormat, @@ -106,8 +110,8 @@ def inject( get_global_httptextformat().inject(set_in_carrier, carrier, context) -_HTTP_TEXT_FORMAT = ( - TraceContextHTTPTextFormat() +_HTTP_TEXT_FORMAT = composite.CompositeHTTPPropagator( + [TraceContextHTTPTextFormat(), CorrelationContextPropagator()], ) # type: httptextformat.HTTPTextFormat diff --git a/opentelemetry-api/tests/propagators/test_global_httptextformat.py b/opentelemetry-api/tests/propagators/test_global_httptextformat.py new file mode 100644 index 00000000000..cc3ccfc9de5 --- /dev/null +++ b/opentelemetry-api/tests/propagators/test_global_httptextformat.py @@ -0,0 +1,74 @@ +# Copyright 2020, OpenTelemetry Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import typing +import unittest + +from opentelemetry import correlationcontext, trace +from opentelemetry.context import set_value +from opentelemetry.propagators import extract, inject +from opentelemetry.trace.propagation import ( + get_span_from_context, + set_span_in_context, +) + + +def get_as_list( + dict_object: typing.Dict[str, typing.List[str]], key: str +) -> typing.List[str]: + value = dict_object.get(key) + return value if value is not None else [] + + +class TestDefaultGlobalPropagator(unittest.TestCase): + """Test ensures the default global composite propagator works as intended + """ + + TRACE_ID = int("12345678901234567890123456789012", 16) # type:int + SPAN_ID = int("1234567890123456", 16) # type:int + + def test_propagation(self): + traceparent_value = "00-{trace_id}-{span_id}-00".format( + trace_id=format(self.TRACE_ID, "032x"), + span_id=format(self.SPAN_ID, "016x"), + ) + tracestate_value = "foo=1,bar=2,baz=3" + headers = { + "otcorrelationcontext": "key1=val1,key2=val2", + "traceparent": [traceparent_value], + "tracestate": [tracestate_value], + } + # create a valid request with both trace and correlation + ctx = extract(get_as_list, headers) + correlations = correlationcontext.get_correlations(context=ctx) + expected = {"key1": "val1", "key2": "val2"} + self.assertEqual(correlations, expected) + span_context = get_span_from_context(context=ctx).get_context() + + self.assertEqual(span_context.trace_id, self.TRACE_ID) + self.assertEqual(span_context.span_id, self.SPAN_ID) + + span = trace.DefaultSpan(span_context) + # create a valid request with both trace and correlation + ctx = correlationcontext.set_correlation("key3", "val3") + ctx = correlationcontext.set_correlation("key4", "val4", context=ctx) + ctx = set_span_in_context(span, context=ctx) + output = {} + inject(dict.__setitem__, output, context=ctx) + expected = { + "otcorrelationcontext": "key3=val3,key4=val4", + "traceparent": traceparent_value, + "tracestate": tracestate_value, + } + self.assertEqual(output, expected) From 2a732f7bbeb35bd7955eeffc79a2fb1a9e22c4f9 Mon Sep 17 00:00:00 2001 From: Alex Boten Date: Thu, 12 Mar 2020 13:28:12 -0700 Subject: [PATCH 08/18] fix test --- .../test_correlation_context_propagation.py | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/opentelemetry-api/tests/correlationcontext/test_correlation_context_propagation.py b/opentelemetry-api/tests/correlationcontext/test_correlation_context_propagation.py index 18eb8ab10e9..8540a554a36 100644 --- a/opentelemetry-api/tests/correlationcontext/test_correlation_context_propagation.py +++ b/opentelemetry-api/tests/correlationcontext/test_correlation_context_propagation.py @@ -90,26 +90,30 @@ def test_invalid_header(self): self.assertEqual(self._extract(header), expected) def test_inject(self): - expected = "key1=val1,key2=val2" values = { "key1": "val1", "key2": "val2", } - self.assertEqual(self._inject(values), expected) + output = self._inject(values) + self.assertIn("key1=val1", output) + self.assertIn("key2=val2", output) def test_inject_escaped_values(self): - expected = "key1=val1%2Cval2,key2=val3%3D4" values = { "key1": "val1,val2", "key2": "val3=4", } - self.assertEqual(self._inject(values), expected) + output = self._inject(values) + self.assertIn("key1=val1%2Cval2", output) + self.assertIn("key2=val3%3D4", output) def test_inject_non_string_values(self): - expected = "key1=true,key2=123,key3=123.567" values = { "key1": True, "key2": 123, "key3": 123.567, } - self.assertEqual(self._inject(values), expected) + output = self._inject(values) + self.assertIn("key1=true", output) + self.assertIn("key2=123", output) + self.assertIn("key3=123.567", output) From 8449360efc7f6524d3c4f94b2a052175e5d4dc69 Mon Sep 17 00:00:00 2001 From: Alex Boten Date: Thu, 12 Mar 2020 13:35:40 -0700 Subject: [PATCH 09/18] fix test --- .../tests/propagators/test_global_httptextformat.py | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/opentelemetry-api/tests/propagators/test_global_httptextformat.py b/opentelemetry-api/tests/propagators/test_global_httptextformat.py index cc3ccfc9de5..d84114137cb 100644 --- a/opentelemetry-api/tests/propagators/test_global_httptextformat.py +++ b/opentelemetry-api/tests/propagators/test_global_httptextformat.py @@ -16,7 +16,6 @@ import unittest from opentelemetry import correlationcontext, trace -from opentelemetry.context import set_value from opentelemetry.propagators import extract, inject from opentelemetry.trace.propagation import ( get_span_from_context, @@ -66,9 +65,9 @@ def test_propagation(self): ctx = set_span_in_context(span, context=ctx) output = {} inject(dict.__setitem__, output, context=ctx) - expected = { - "otcorrelationcontext": "key3=val3,key4=val4", - "traceparent": traceparent_value, - "tracestate": tracestate_value, - } - self.assertEqual(output, expected) + self.assertEqual(traceparent_value, output["traceparent"]) + self.assertIn("key3=val3", output["otcorrelationcontext"]) + self.assertIn("key4=val4", output["otcorrelationcontext"]) + self.assertIn("foo=1", output["tracestate"]) + self.assertIn("bar=2", output["tracestate"]) + self.assertIn("baz=3", output["tracestate"]) From 2e813af71db22fb516847829cc1178a0ee603ccb Mon Sep 17 00:00:00 2001 From: Alex Boten Date: Thu, 12 Mar 2020 13:45:57 -0700 Subject: [PATCH 10/18] mypy fixes --- .../correlationcontext/__init__.py | 20 +++++++------------ .../propagators/test_global_httptextformat.py | 2 -- 2 files changed, 7 insertions(+), 15 deletions(-) diff --git a/opentelemetry-api/src/opentelemetry/correlationcontext/__init__.py b/opentelemetry-api/src/opentelemetry/correlationcontext/__init__.py index c07c702fc11..1f0bac83732 100644 --- a/opentelemetry-api/src/opentelemetry/correlationcontext/__init__.py +++ b/opentelemetry-api/src/opentelemetry/correlationcontext/__init__.py @@ -33,7 +33,7 @@ def get_correlations( name/value pairs in the CorrelationContext """ correlations = get_value(CORRELATION_CONTEXT_KEY, context=context) - if correlations: + if isinstance(correlations, dict): return correlations return {} @@ -51,14 +51,11 @@ def get_correlation( the value associated with the given name, or null if the given name is not present. """ - correlations = get_value(CORRELATION_CONTEXT_KEY, context=context) - if correlations: - return correlations.get(name) - return None + return get_correlations(context=context).get(name) def set_correlation( - name: str, value, context: typing.Optional[Context] = None + name: str, value: object, context: typing.Optional[Context] = None ) -> Context: """Sets a value in the CorrelationContext @@ -70,11 +67,8 @@ def set_correlation( Returns: a Context with the value updated """ - correlations = get_value(CORRELATION_CONTEXT_KEY, context=context) - if correlations: - correlations[name] = value - else: - correlations = {name: value} + correlations = get_correlations(context=context) + correlations[name] = value return set_value(CORRELATION_CONTEXT_KEY, correlations, context=context) @@ -89,8 +83,8 @@ def remove_correlation( Returns: a Context with the name/value removed """ - correlations = get_value(CORRELATION_CONTEXT_KEY, context=context) - if correlations and name in correlations: + correlations = get_correlations(context=context) + if name in correlations: del correlations[name] return set_value(CORRELATION_CONTEXT_KEY, correlations, context=context) diff --git a/opentelemetry-api/tests/propagators/test_global_httptextformat.py b/opentelemetry-api/tests/propagators/test_global_httptextformat.py index d84114137cb..20cef4e238e 100644 --- a/opentelemetry-api/tests/propagators/test_global_httptextformat.py +++ b/opentelemetry-api/tests/propagators/test_global_httptextformat.py @@ -48,7 +48,6 @@ def test_propagation(self): "traceparent": [traceparent_value], "tracestate": [tracestate_value], } - # create a valid request with both trace and correlation ctx = extract(get_as_list, headers) correlations = correlationcontext.get_correlations(context=ctx) expected = {"key1": "val1", "key2": "val2"} @@ -59,7 +58,6 @@ def test_propagation(self): self.assertEqual(span_context.span_id, self.SPAN_ID) span = trace.DefaultSpan(span_context) - # create a valid request with both trace and correlation ctx = correlationcontext.set_correlation("key3", "val3") ctx = correlationcontext.set_correlation("key4", "val4", context=ctx) ctx = set_span_in_context(span, context=ctx) From 778d8f497194f440e6e0fb8c393dbfe43c58ef37 Mon Sep 17 00:00:00 2001 From: Alex Boten Date: Thu, 12 Mar 2020 13:58:38 -0700 Subject: [PATCH 11/18] more mypy fixes --- .../correlationcontext/propagation/__init__.py | 12 ++++++++++-- .../test_correlation_context_propagation.py | 4 ++-- .../tests/propagators/test_global_httptextformat.py | 2 +- 3 files changed, 13 insertions(+), 5 deletions(-) diff --git a/opentelemetry-api/src/opentelemetry/correlationcontext/propagation/__init__.py b/opentelemetry-api/src/opentelemetry/correlationcontext/propagation/__init__.py index 51001020a08..ebd945c96a7 100644 --- a/opentelemetry-api/src/opentelemetry/correlationcontext/propagation/__init__.py +++ b/opentelemetry-api/src/opentelemetry/correlationcontext/propagation/__init__.py @@ -41,8 +41,8 @@ def extract( if not context: context = get_current() - header = get_from_carrier( - carrier, self._CORRELATION_CONTEXT_HEADER_NAME + header = _extract_first_element( + get_from_carrier(carrier, self._CORRELATION_CONTEXT_HEADER_NAME) ) if not header: @@ -99,3 +99,11 @@ def _format_correlations(correlations: typing.Dict[str, object]) -> str: key + "=" + urllib.parse.quote_plus(str(value).lower()) for key, value in correlations.items() ) + + +def _extract_first_element( + items: typing.Iterable[httptextformat.HTTPTextFormatT], +) -> typing.Optional[httptextformat.HTTPTextFormatT]: + if items is None: + return None + return next(iter(items), None) diff --git a/opentelemetry-api/tests/correlationcontext/test_correlation_context_propagation.py b/opentelemetry-api/tests/correlationcontext/test_correlation_context_propagation.py index 8540a554a36..9b81cbd7a54 100644 --- a/opentelemetry-api/tests/correlationcontext/test_correlation_context_propagation.py +++ b/opentelemetry-api/tests/correlationcontext/test_correlation_context_propagation.py @@ -35,7 +35,7 @@ def setUp(self): def _extract(self, header_value): """Test helper""" - header = {"otcorrelationcontext": header_value} + header = {"otcorrelationcontext": [header_value]} return correlationcontext.get_correlations( self.propagator.extract(get_as_list, header) ) @@ -51,7 +51,7 @@ def _inject(self, values): return output.get("otcorrelationcontext") def test_no_context_header(self): - header = {} # type:typing.Dict[str, typing.List[str]] + header = "" self.assertEqual(self._extract(header), {}) def test_valid_header(self): diff --git a/opentelemetry-api/tests/propagators/test_global_httptextformat.py b/opentelemetry-api/tests/propagators/test_global_httptextformat.py index 20cef4e238e..6045feb6754 100644 --- a/opentelemetry-api/tests/propagators/test_global_httptextformat.py +++ b/opentelemetry-api/tests/propagators/test_global_httptextformat.py @@ -44,7 +44,7 @@ def test_propagation(self): ) tracestate_value = "foo=1,bar=2,baz=3" headers = { - "otcorrelationcontext": "key1=val1,key2=val2", + "otcorrelationcontext": ["key1=val1,key2=val2"], "traceparent": [traceparent_value], "tracestate": [tracestate_value], } From 1bc5a83dbc390bbda6ca65c758648cebba819d1f Mon Sep 17 00:00:00 2001 From: Alex Boten Date: Fri, 13 Mar 2020 10:17:04 -0700 Subject: [PATCH 12/18] review feedback --- .../correlationcontext/__init__.py | 31 +++++++++---------- .../propagation/__init__.py | 2 +- .../test_correlation_context_propagation.py | 10 ++++-- 3 files changed, 23 insertions(+), 20 deletions(-) diff --git a/opentelemetry-api/src/opentelemetry/correlationcontext/__init__.py b/opentelemetry-api/src/opentelemetry/correlationcontext/__init__.py index 1f0bac83732..de6cf16ca3d 100644 --- a/opentelemetry-api/src/opentelemetry/correlationcontext/__init__.py +++ b/opentelemetry-api/src/opentelemetry/correlationcontext/__init__.py @@ -27,10 +27,10 @@ def get_correlations( """ Returns the name/value pairs in the CorrelationContext Args: - context: the Context to use. If not set, uses current Context + context: The Context to use. If not set, uses current Context Returns: - name/value pairs in the CorrelationContext + Name/value pairs in the CorrelationContext """ correlations = get_value(CORRELATION_CONTEXT_KEY, context=context) if isinstance(correlations, dict): @@ -44,11 +44,11 @@ def get_correlation( """ Provides access to the value for a name/value pair in the CorrelationContext Args: - name: the name of the value to retrieve - context: the Context to use. If not set, uses current Context + name: The name of the value to retrieve + context: The Context to use. If not set, uses current Context Returns: - the value associated with the given name, or null if the given name is + The value associated with the given name, or null if the given name is not present. """ return get_correlations(context=context).get(name) @@ -60,12 +60,12 @@ def set_correlation( """Sets a value in the CorrelationContext Args: - name: the name of the value to set - value: the value to set - context: the Context to use. If not set, uses current Context + name: The name of the value to set + value: The value to set + context: The Context to use. If not set, uses current Context Returns: - a Context with the value updated + A Context with the value updated """ correlations = get_correlations(context=context) correlations[name] = value @@ -77,15 +77,14 @@ def remove_correlation( ) -> Context: """Removes a value from the CorrelationContext Args: - name: the name of the value to remove - context: the Context to use. If not set, uses current Context + name: The name of the value to remove + context: The Context to use. If not set, uses current Context Returns: - a Context with the name/value removed + A Context with the name/value removed """ correlations = get_correlations(context=context) - if name in correlations: - del correlations[name] + correlations.pop(name, None) return set_value(CORRELATION_CONTEXT_KEY, correlations, context=context) @@ -93,9 +92,9 @@ def remove_correlation( def clear_correlations(context: typing.Optional[Context] = None) -> Context: """Removes all values from the CorrelationContext Args: - context: the Context to use. If not set, uses current Context + context: The Context to use. If not set, uses current Context Returns: - a Context with all correlations removed + A Context with all correlations removed """ return set_value(CORRELATION_CONTEXT_KEY, {}, context=context) diff --git a/opentelemetry-api/src/opentelemetry/correlationcontext/propagation/__init__.py b/opentelemetry-api/src/opentelemetry/correlationcontext/propagation/__init__.py index ebd945c96a7..41464268989 100644 --- a/opentelemetry-api/src/opentelemetry/correlationcontext/propagation/__init__.py +++ b/opentelemetry-api/src/opentelemetry/correlationcontext/propagation/__init__.py @@ -89,7 +89,7 @@ def _format_correlations(correlations: typing.Dict[str, object]) -> str: """Format correlations into a string. Args: - correlations: the correlations to format + correlations: The correlations to format Returns: A string that adheres to the w3c correlationcontext diff --git a/opentelemetry-api/tests/correlationcontext/test_correlation_context_propagation.py b/opentelemetry-api/tests/correlationcontext/test_correlation_context_propagation.py index 9b81cbd7a54..ede83143b6c 100644 --- a/opentelemetry-api/tests/correlationcontext/test_correlation_context_propagation.py +++ b/opentelemetry-api/tests/correlationcontext/test_correlation_context_propagation.py @@ -25,8 +25,7 @@ def get_as_list( dict_object: typing.Dict[str, typing.List[str]], key: str ) -> typing.List[str]: - value = dict_object.get(key) - return value if value is not None else [] + return dict_object.get(key, []) class TestCorrelationContextPropagation(unittest.TestCase): @@ -47,10 +46,15 @@ def _inject(self, values): ctx = correlationcontext.set_correlation(k, v, context=ctx) output = {} self.propagator.inject(dict.__setitem__, output, context=ctx) - print(output) return output.get("otcorrelationcontext") def test_no_context_header(self): + correlations = correlationcontext.get_correlations( + self.propagator.extract(get_as_list, {}) + ) + self.assertEqual(correlations, {}) + + def test_empty_context_header(self): header = "" self.assertEqual(self._extract(header), {}) From 270ddc204675036d55f314600e047889473ba7b3 Mon Sep 17 00:00:00 2001 From: Alex Boten Date: Fri, 13 Mar 2020 10:26:35 -0700 Subject: [PATCH 13/18] return a copy of the dict for now, added test to catch mutating behaviour --- .../src/opentelemetry/correlationcontext/__init__.py | 2 +- .../tests/correlationcontext/test_correlation_context.py | 9 ++++++++- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/opentelemetry-api/src/opentelemetry/correlationcontext/__init__.py b/opentelemetry-api/src/opentelemetry/correlationcontext/__init__.py index de6cf16ca3d..1564cae4673 100644 --- a/opentelemetry-api/src/opentelemetry/correlationcontext/__init__.py +++ b/opentelemetry-api/src/opentelemetry/correlationcontext/__init__.py @@ -34,7 +34,7 @@ def get_correlations( """ correlations = get_value(CORRELATION_CONTEXT_KEY, context=context) if isinstance(correlations, dict): - return correlations + return correlations.copy() return {} diff --git a/opentelemetry-api/tests/correlationcontext/test_correlation_context.py b/opentelemetry-api/tests/correlationcontext/test_correlation_context.py index 5ea2ff69d2d..a3cf4e54ea1 100644 --- a/opentelemetry-api/tests/correlationcontext/test_correlation_context.py +++ b/opentelemetry-api/tests/correlationcontext/test_correlation_context.py @@ -37,6 +37,13 @@ def test_set_multiple_correlations(self): {"test": "value", "test2": "value2"}, ) + def test_modifying_correlations(self): + ctx = cctx.set_correlation("test", "value") + self.assertEqual(cctx.get_correlation("test", context=ctx), "value") + correlations = cctx.get_correlations(context=ctx) + correlations["test"] = "mess-this-up" + self.assertEqual(cctx.get_correlation("test", context=ctx), "value") + def test_remove_correlations(self): self.assertEqual({}, cctx.get_correlations()) @@ -53,4 +60,4 @@ def test_clear_correlations(self): self.assertEqual(cctx.get_correlation("test", context=ctx), "value") ctx = cctx.clear_correlations(context=ctx) - self.assertEqual(cctx.get_correlation("test", context=ctx), None) + self.assertEqual(cctx.get_correlations(context=ctx), {}) From 54ac2cc58fec5582becb027a3a4e713f76bb5bf7 Mon Sep 17 00:00:00 2001 From: Alex Boten Date: Fri, 13 Mar 2020 13:21:08 -0700 Subject: [PATCH 14/18] adding checks for limits on extract --- .../propagation/__init__.py | 14 +++++++--- .../test_correlation_context_propagation.py | 27 +++++++++++++++++-- 2 files changed, 36 insertions(+), 5 deletions(-) diff --git a/opentelemetry-api/src/opentelemetry/correlationcontext/propagation/__init__.py b/opentelemetry-api/src/opentelemetry/correlationcontext/propagation/__init__.py index 41464268989..0bb687160f0 100644 --- a/opentelemetry-api/src/opentelemetry/correlationcontext/propagation/__init__.py +++ b/opentelemetry-api/src/opentelemetry/correlationcontext/propagation/__init__.py @@ -23,6 +23,9 @@ class CorrelationContextPropagator(httptextformat.HTTPTextFormat): + MAX_HEADER_LENGTH = 8192 + MAX_PAIR_LENGTH = 4096 + MAX_PAIRS = 180 _CORRELATION_CONTEXT_HEADER_NAME = "otcorrelationcontext" def extract( @@ -45,18 +48,23 @@ def extract( get_from_carrier(carrier, self._CORRELATION_CONTEXT_HEADER_NAME) ) - if not header: + if not header or len(header) > self.MAX_HEADER_LENGTH: return context correlations = header.split(",") - + total_correlations = self.MAX_PAIRS for correlation in correlations: + if total_correlations <= 0: + return context + total_correlations -= 1 + if len(correlation) > self.MAX_PAIR_LENGTH: + continue try: name, value = correlation.split("=", 1) except Exception: # pylint: disable=broad-except continue context = correlationcontext.set_correlation( - name.strip(), + urllib.parse.unquote(name).strip(), urllib.parse.unquote(value).strip(), context=context, ) diff --git a/opentelemetry-api/tests/correlationcontext/test_correlation_context_propagation.py b/opentelemetry-api/tests/correlationcontext/test_correlation_context_propagation.py index ede83143b6c..51a21c7af8f 100644 --- a/opentelemetry-api/tests/correlationcontext/test_correlation_context_propagation.py +++ b/opentelemetry-api/tests/correlationcontext/test_correlation_context_propagation.py @@ -74,8 +74,8 @@ def test_valid_header_with_properties(self): self.assertEqual(self._extract(header), expected) def test_valid_header_with_url_escaped_comma(self): - header = "key1=val1,key2=val2%2Cval3" - expected = {"key1": "val1", "key2": "val2,val3"} + header = "key%2C1=val1,key2=val2%2Cval3" + expected = {"key,1": "val1", "key2": "val2,val3"} self.assertEqual(self._extract(header), expected) def test_valid_header_with_invalid_value(self): @@ -93,6 +93,29 @@ def test_invalid_header(self): expected = {} self.assertEqual(self._extract(header), expected) + def test_header_too_long(self): + long_value = "s" * (CorrelationContextPropagator.MAX_HEADER_LENGTH + 1) + header = "key1={}".format(long_value) + expected = {} + self.assertEqual(self._extract(header), expected) + + def test_header_contains_too_many_entries(self): + header = ",".join( + [ + "key{}=val".format(k) + for k in range(CorrelationContextPropagator.MAX_PAIRS + 1) + ] + ) + self.assertEqual( + len(self._extract(header)), CorrelationContextPropagator.MAX_PAIRS + ) + + def test_header_contains_pair_too_long(self): + long_value = "s" * (CorrelationContextPropagator.MAX_PAIR_LENGTH + 1) + header = "key1=value1,key2={},key3=value3".format(long_value) + expected = {"key1": "value1", "key3": "value3"} + self.assertEqual(self._extract(header), expected) + def test_inject(self): values = { "key1": "val1", From 2c5d20f2825513acb39822b73723ae9ecc6e46cc Mon Sep 17 00:00:00 2001 From: Alex Boten Date: Fri, 13 Mar 2020 13:28:00 -0700 Subject: [PATCH 15/18] respect casing --- .../opentelemetry/correlationcontext/propagation/__init__.py | 2 +- .../correlationcontext/test_correlation_context_propagation.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/opentelemetry-api/src/opentelemetry/correlationcontext/propagation/__init__.py b/opentelemetry-api/src/opentelemetry/correlationcontext/propagation/__init__.py index 0bb687160f0..6300bdb4f0f 100644 --- a/opentelemetry-api/src/opentelemetry/correlationcontext/propagation/__init__.py +++ b/opentelemetry-api/src/opentelemetry/correlationcontext/propagation/__init__.py @@ -104,7 +104,7 @@ def _format_correlations(correlations: typing.Dict[str, object]) -> str: header format. """ return ",".join( - key + "=" + urllib.parse.quote_plus(str(value).lower()) + key + "=" + urllib.parse.quote_plus(str(value)) for key, value in correlations.items() ) diff --git a/opentelemetry-api/tests/correlationcontext/test_correlation_context_propagation.py b/opentelemetry-api/tests/correlationcontext/test_correlation_context_propagation.py index 51a21c7af8f..2a6053015ea 100644 --- a/opentelemetry-api/tests/correlationcontext/test_correlation_context_propagation.py +++ b/opentelemetry-api/tests/correlationcontext/test_correlation_context_propagation.py @@ -141,6 +141,6 @@ def test_inject_non_string_values(self): "key3": 123.567, } output = self._inject(values) - self.assertIn("key1=true", output) + self.assertIn("key1=True", output) self.assertIn("key2=123", output) self.assertIn("key3=123.567", output) From 19342ceccba65315c553c32e5f5126b76211e65a Mon Sep 17 00:00:00 2001 From: Alex Boten Date: Fri, 13 Mar 2020 13:42:01 -0700 Subject: [PATCH 16/18] empty inject test --- .../test_correlation_context_propagation.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/opentelemetry-api/tests/correlationcontext/test_correlation_context_propagation.py b/opentelemetry-api/tests/correlationcontext/test_correlation_context_propagation.py index 2a6053015ea..d0ecbdb17fd 100644 --- a/opentelemetry-api/tests/correlationcontext/test_correlation_context_propagation.py +++ b/opentelemetry-api/tests/correlationcontext/test_correlation_context_propagation.py @@ -116,6 +116,11 @@ def test_header_contains_pair_too_long(self): expected = {"key1": "value1", "key3": "value3"} self.assertEqual(self._extract(header), expected) + def test_inject_no_correlations(self): + values = {} + output = self._inject(values) + self.assertEqual(None, output) + def test_inject(self): values = { "key1": "val1", From 09617efe3735de5c9d0239fd9b0f9a9259796fa6 Mon Sep 17 00:00:00 2001 From: Alex Boten Date: Fri, 13 Mar 2020 14:01:11 -0700 Subject: [PATCH 17/18] adding test to ensure correlations are working in the current context --- .../tests/correlationcontext/test_correlation_context.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/opentelemetry-api/tests/correlationcontext/test_correlation_context.py b/opentelemetry-api/tests/correlationcontext/test_correlation_context.py index a3cf4e54ea1..fc5ac062072 100644 --- a/opentelemetry-api/tests/correlationcontext/test_correlation_context.py +++ b/opentelemetry-api/tests/correlationcontext/test_correlation_context.py @@ -14,6 +14,7 @@ import unittest +from opentelemetry import context from opentelemetry import correlationcontext as cctx @@ -27,6 +28,11 @@ def test_set_correlation(self): ctx = cctx.set_correlation("test", "value2", context=ctx) self.assertEqual(cctx.get_correlation("test", context=ctx), "value2") + def test_correlations_current_context(self): + token = context.attach(cctx.set_correlation("test", "value")) + self.assertEqual(cctx.get_correlation("test"), "value") + context.detach(token) + def test_set_multiple_correlations(self): ctx = cctx.set_correlation("test", "value") ctx = cctx.set_correlation("test2", "value2", context=ctx) From 4fd969f55bdb3794a74ba14769fa428234b3a95c Mon Sep 17 00:00:00 2001 From: Alex Boten Date: Sat, 14 Mar 2020 14:26:06 -0700 Subject: [PATCH 18/18] review feedback --- .../src/opentelemetry/correlationcontext/__init__.py | 10 +++++----- .../correlationcontext/propagation/__init__.py | 11 +---------- .../correlationcontext/test_correlation_context.py | 1 + 3 files changed, 7 insertions(+), 15 deletions(-) diff --git a/opentelemetry-api/src/opentelemetry/correlationcontext/__init__.py b/opentelemetry-api/src/opentelemetry/correlationcontext/__init__.py index 1564cae4673..b0b3624d720 100644 --- a/opentelemetry-api/src/opentelemetry/correlationcontext/__init__.py +++ b/opentelemetry-api/src/opentelemetry/correlationcontext/__init__.py @@ -18,7 +18,7 @@ from opentelemetry.context import get_value, set_value from opentelemetry.context.context import Context -CORRELATION_CONTEXT_KEY = "correlation-context" +_CORRELATION_CONTEXT_KEY = "correlation-context" def get_correlations( @@ -32,7 +32,7 @@ def get_correlations( Returns: Name/value pairs in the CorrelationContext """ - correlations = get_value(CORRELATION_CONTEXT_KEY, context=context) + correlations = get_value(_CORRELATION_CONTEXT_KEY, context=context) if isinstance(correlations, dict): return correlations.copy() return {} @@ -69,7 +69,7 @@ def set_correlation( """ correlations = get_correlations(context=context) correlations[name] = value - return set_value(CORRELATION_CONTEXT_KEY, correlations, context=context) + return set_value(_CORRELATION_CONTEXT_KEY, correlations, context=context) def remove_correlation( @@ -86,7 +86,7 @@ def remove_correlation( correlations = get_correlations(context=context) correlations.pop(name, None) - return set_value(CORRELATION_CONTEXT_KEY, correlations, context=context) + return set_value(_CORRELATION_CONTEXT_KEY, correlations, context=context) def clear_correlations(context: typing.Optional[Context] = None) -> Context: @@ -97,4 +97,4 @@ def clear_correlations(context: typing.Optional[Context] = None) -> Context: Returns: A Context with all correlations removed """ - return set_value(CORRELATION_CONTEXT_KEY, {}, context=context) + return set_value(_CORRELATION_CONTEXT_KEY, {}, context=context) diff --git a/opentelemetry-api/src/opentelemetry/correlationcontext/propagation/__init__.py b/opentelemetry-api/src/opentelemetry/correlationcontext/propagation/__init__.py index 6300bdb4f0f..72ad80de1a9 100644 --- a/opentelemetry-api/src/opentelemetry/correlationcontext/propagation/__init__.py +++ b/opentelemetry-api/src/opentelemetry/correlationcontext/propagation/__init__.py @@ -41,7 +41,7 @@ def extract( See `opentelemetry.trace.propagation.httptextformat.HTTPTextFormat.extract` """ - if not context: + if context is None: context = get_current() header = _extract_first_element( @@ -94,15 +94,6 @@ def inject( def _format_correlations(correlations: typing.Dict[str, object]) -> str: - """Format correlations into a string. - - Args: - correlations: The correlations to format - - Returns: - A string that adheres to the w3c correlationcontext - header format. - """ return ",".join( key + "=" + urllib.parse.quote_plus(str(value)) for key, value in correlations.items() diff --git a/opentelemetry-api/tests/correlationcontext/test_correlation_context.py b/opentelemetry-api/tests/correlationcontext/test_correlation_context.py index fc5ac062072..3bddb951e26 100644 --- a/opentelemetry-api/tests/correlationcontext/test_correlation_context.py +++ b/opentelemetry-api/tests/correlationcontext/test_correlation_context.py @@ -32,6 +32,7 @@ def test_correlations_current_context(self): token = context.attach(cctx.set_correlation("test", "value")) self.assertEqual(cctx.get_correlation("test"), "value") context.detach(token) + self.assertEqual(cctx.get_correlation("test"), None) def test_set_multiple_correlations(self): ctx = cctx.set_correlation("test", "value")