From ce8b47b9c316f25970b8c1d7c2b2fafbb1ffec57 Mon Sep 17 00:00:00 2001 From: Alex Boten Date: Fri, 1 Nov 2019 16:16:48 -0700 Subject: [PATCH 01/12] Removing add_link and add_lazy_link from api/sdk Signed-off-by: Alex Boten --- .../examples/jaeger_exporter_example.py | 7 ++-- .../ext/opentracing_shim/__init__.py | 11 ++++-- .../src/opentelemetry/trace/__init__.py | 23 ++++--------- .../src/opentelemetry/sdk/trace/__init__.py | 34 ++++--------------- opentelemetry-sdk/tests/trace/test_trace.py | 17 ++++------ 5 files changed, 33 insertions(+), 59 deletions(-) diff --git a/ext/opentelemetry-ext-jaeger/examples/jaeger_exporter_example.py b/ext/opentelemetry-ext-jaeger/examples/jaeger_exporter_example.py index 5cf57bfccad..0935bebaa36 100644 --- a/ext/opentelemetry-ext-jaeger/examples/jaeger_exporter_example.py +++ b/ext/opentelemetry-ext-jaeger/examples/jaeger_exporter_example.py @@ -1,6 +1,6 @@ import time -from opentelemetry import trace +from opentelemetry import trace, trace_api from opentelemetry.ext import jaeger from opentelemetry.sdk.trace import Tracer from opentelemetry.sdk.trace.export import BatchExportSpanProcessor @@ -33,10 +33,11 @@ time.sleep(0.1) foo.set_attribute("my_atribbute", True) foo.add_event("event in foo", {"name": "foo1"}) - with tracer.start_as_current_span("bar") as bar: + with tracer.start_as_current_span( + "bar", links=[trace_api.Link(foo.get_context())] + ) as bar: time.sleep(0.2) bar.set_attribute("speed", 100.0) - bar.add_link(foo.get_context()) with tracer.start_as_current_span("baz") as baz: time.sleep(0.3) diff --git a/ext/opentelemetry-ext-opentracing-shim/src/opentelemetry/ext/opentracing_shim/__init__.py b/ext/opentelemetry-ext-opentracing-shim/src/opentelemetry/ext/opentracing_shim/__init__.py index 7ee8ad71e09..a1b39c209b1 100644 --- a/ext/opentelemetry-ext-opentracing-shim/src/opentelemetry/ext/opentracing_shim/__init__.py +++ b/ext/opentelemetry-ext-opentracing-shim/src/opentelemetry/ext/opentracing_shim/__init__.py @@ -15,8 +15,10 @@ import logging import opentracing + from deprecated import deprecated +import opentelemetry.trace as trace_api from opentelemetry.ext.opentracing_shim import util logger = logging.getLogger(__name__) @@ -226,11 +228,16 @@ def start_span( # Use the specified parent or the active span if possible. Otherwise, # use a `None` parent, which triggers the creation of a new trace. parent = child_of.unwrap() if child_of else None - span = self._otel_tracer.create_span(operation_name, parent) + links = None if references: + links = [] for ref in references: - span.add_link(ref.referenced_context.unwrap()) + links.append(trace_api.Link(ref.referenced_context.unwrap())) + + span = self._otel_tracer.create_span( + operation_name, parent, links=links + ) if tags: for key, value in tags.items(): diff --git a/opentelemetry-api/src/opentelemetry/trace/__init__.py b/opentelemetry-api/src/opentelemetry/trace/__init__.py index 1ac7d73d49e..375ad331cac 100644 --- a/opentelemetry-api/src/opentelemetry/trace/__init__.py +++ b/opentelemetry-api/src/opentelemetry/trace/__init__.py @@ -75,12 +75,15 @@ class Link: """A link to a `Span`.""" + empty_attributes: typing.Dict[str, types.AttributeValue] = dict() def __init__( self, context: "SpanContext", attributes: types.Attributes = None ) -> None: self._context = context self._attributes = attributes + if attributes is None: + self._attributes = Link.empty_attributes @property def context(self) -> "SpanContext": @@ -198,23 +201,6 @@ def add_lazy_event(self, event: Event) -> None: Adds an `Event` that has previously been created. """ - def add_link( - self, - link_target_context: "SpanContext", - attributes: types.Attributes = None, - ) -> None: - """Adds a `Link` to another span. - - Adds a single `Link` from this Span to another Span identified by the - `SpanContext` passed as argument. - """ - - def add_lazy_link(self, link: "Link") -> None: - """Adds a `Link` to another span. - - Adds a `Link` that has previously been created. - """ - def update_name(self, name: str) -> None: """Updates the `Span` name. @@ -416,6 +402,7 @@ def start_span( name: str, parent: ParentSpan = CURRENT_SPAN, kind: SpanKind = SpanKind.INTERNAL, + links: typing.Sequence[Link] = (), ) -> "Span": """Starts a span. @@ -457,6 +444,7 @@ def start_as_current_span( name: str, parent: ParentSpan = CURRENT_SPAN, kind: SpanKind = SpanKind.INTERNAL, + links: typing.Sequence[Link] = (), ) -> typing.Iterator["Span"]: """Context manager for creating a new span and set it as the current span in this tracer's context. @@ -505,6 +493,7 @@ def create_span( name: str, parent: ParentSpan = CURRENT_SPAN, kind: SpanKind = SpanKind.INTERNAL, + links: typing.Sequence[Link] = (), ) -> "Span": """Creates a span. diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py index 0909b9b434c..4ae3f9f1120 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py @@ -130,7 +130,7 @@ def __init__( resource: None = None, # TODO attributes: types.Attributes = None, # TODO events: Sequence[trace_api.Event] = None, # TODO - links: Sequence[trace_api.Link] = None, # TODO + links: Sequence[trace_api.Link] = None, kind: trace_api.SpanKind = trace_api.SpanKind.INTERNAL, span_processor: SpanProcessor = SpanProcessor(), ) -> None: @@ -226,30 +226,6 @@ def add_lazy_event(self, event: trace_api.Event) -> None: return self.events.append(event) - def add_link( - self, - link_target_context: "trace_api.SpanContext", - attributes: types.Attributes = None, - ) -> None: - if attributes is None: - attributes = ( - Span.empty_attributes - ) # TODO: empty_attributes is not a Dict. Use Mapping? - self.add_lazy_link(trace_api.Link(link_target_context, attributes)) - - def add_lazy_link(self, link: "trace_api.Link") -> None: - with self._lock: - if not self.is_recording_events(): - return - has_ended = self.end_time is not None - if not has_ended: - if self.links is Span.empty_links: - self.links = BoundedList(MAX_NUM_LINKS) - if has_ended: - logger.warning("Calling add_link() on an ended span.") - return - self.links.append(link) - def start(self, start_time: Optional[int] = None) -> None: with self._lock: if not self.is_recording_events(): @@ -343,10 +319,11 @@ def start_span( name: str, parent: trace_api.ParentSpan = trace_api.Tracer.CURRENT_SPAN, kind: trace_api.SpanKind = trace_api.SpanKind.INTERNAL, + links: Sequence[trace_api.Link] = (), ) -> "Span": """See `opentelemetry.trace.Tracer.start_span`.""" - span = self.create_span(name, parent, kind) + span = self.create_span(name, parent, kind, links) span.start() return span @@ -355,10 +332,11 @@ def start_as_current_span( name: str, parent: trace_api.ParentSpan = trace_api.Tracer.CURRENT_SPAN, kind: trace_api.SpanKind = trace_api.SpanKind.INTERNAL, + links: Sequence[trace_api.Link] = (), ) -> Iterator[trace_api.Span]: """See `opentelemetry.trace.Tracer.start_as_current_span`.""" - span = self.start_span(name, parent, kind) + span = self.start_span(name, parent, kind, links) return self.use_span(span, end_on_exit=True) def create_span( @@ -366,6 +344,7 @@ def create_span( name: str, parent: trace_api.ParentSpan = trace_api.Tracer.CURRENT_SPAN, kind: trace_api.SpanKind = trace_api.SpanKind.INTERNAL, + links: Sequence[trace_api.Link] = (), ) -> "trace_api.Span": """See `opentelemetry.trace.Tracer.create_span`. @@ -424,6 +403,7 @@ def create_span( attributes=sampling_decision.attributes, span_processor=self._active_span_processor, kind=kind, + links=links, ) return trace_api.DefaultSpan(context=context) diff --git a/opentelemetry-sdk/tests/trace/test_trace.py b/opentelemetry-sdk/tests/trace/test_trace.py index 626a5499ecf..d44474f0d88 100644 --- a/opentelemetry-sdk/tests/trace/test_trace.py +++ b/opentelemetry-sdk/tests/trace/test_trace.py @@ -233,9 +233,15 @@ def test_span_members(self): span_id=trace.generate_span_id(), ) + links = [ + trace_api.Link(other_context1), + trace_api.Link(other_context2, {"name": "neighbor"}), + trace_api.Link(other_context3, {"component": "http"}), + ] + self.assertIsNone(tracer.get_current_span()) - with tracer.start_as_current_span("root") as root: + with tracer.start_as_current_span("root", links=links) as root: # attributes root.set_attribute("component", "http") root.set_attribute("http.method", "GET") @@ -287,12 +293,6 @@ def test_span_members(self): self.assertEqual(root.events[2].timestamp, now) # links - root.add_link(other_context1) - root.add_link(other_context2, {"name": "neighbor"}) - root.add_lazy_link( - trace_api.Link(other_context3, {"component": "http"}) - ) - self.assertEqual(len(root.links), 3) self.assertEqual( root.links[0].context.trace_id, other_context1.trace_id @@ -387,9 +387,6 @@ def test_ended_span(self): root.add_event("event1") self.assertEqual(len(root.events), 0) - root.add_link(other_context1) - self.assertEqual(len(root.links), 0) - root.update_name("xxx") self.assertEqual(root.name, "root") From 7ff2ddee5b77749be9e9329e199a2283e8004893 Mon Sep 17 00:00:00 2001 From: Alex Boten Date: Mon, 4 Nov 2019 08:11:38 -0800 Subject: [PATCH 02/12] Fixing an import and typo Signed-off-by: Alex Boten --- .../examples/jaeger_exporter_example.py | 4 ++-- opentelemetry-api/src/opentelemetry/trace/__init__.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/ext/opentelemetry-ext-jaeger/examples/jaeger_exporter_example.py b/ext/opentelemetry-ext-jaeger/examples/jaeger_exporter_example.py index 0935bebaa36..89d93809e40 100644 --- a/ext/opentelemetry-ext-jaeger/examples/jaeger_exporter_example.py +++ b/ext/opentelemetry-ext-jaeger/examples/jaeger_exporter_example.py @@ -1,6 +1,6 @@ import time -from opentelemetry import trace, trace_api +from opentelemetry import trace from opentelemetry.ext import jaeger from opentelemetry.sdk.trace import Tracer from opentelemetry.sdk.trace.export import BatchExportSpanProcessor @@ -34,7 +34,7 @@ foo.set_attribute("my_atribbute", True) foo.add_event("event in foo", {"name": "foo1"}) with tracer.start_as_current_span( - "bar", links=[trace_api.Link(foo.get_context())] + "bar", links=[trace.Link(foo.get_context())] ) as bar: time.sleep(0.2) bar.set_attribute("speed", 100.0) diff --git a/opentelemetry-api/src/opentelemetry/trace/__init__.py b/opentelemetry-api/src/opentelemetry/trace/__init__.py index 375ad331cac..7650427ae4b 100644 --- a/opentelemetry-api/src/opentelemetry/trace/__init__.py +++ b/opentelemetry-api/src/opentelemetry/trace/__init__.py @@ -75,7 +75,7 @@ class Link: """A link to a `Span`.""" - empty_attributes: typing.Dict[str, types.AttributeValue] = dict() + empty_attributes = {} def __init__( self, context: "SpanContext", attributes: types.Attributes = None From 98de7694cdeefbca516551804b39e88cb29ac8cc Mon Sep 17 00:00:00 2001 From: Alex Boten Date: Mon, 4 Nov 2019 08:31:44 -0800 Subject: [PATCH 03/12] Fix lint and add annotation Signed-off-by: Alex Boten --- opentelemetry-api/src/opentelemetry/trace/__init__.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/opentelemetry-api/src/opentelemetry/trace/__init__.py b/opentelemetry-api/src/opentelemetry/trace/__init__.py index 7650427ae4b..2a32253caa0 100644 --- a/opentelemetry-api/src/opentelemetry/trace/__init__.py +++ b/opentelemetry-api/src/opentelemetry/trace/__init__.py @@ -75,7 +75,8 @@ class Link: """A link to a `Span`.""" - empty_attributes = {} + + empty_attributes = {} # type: typing.Dict[str, typing.Union[str, bool, float]] def __init__( self, context: "SpanContext", attributes: types.Attributes = None From 01f2fd3eadbb21d983b81d66b26fdd453b9f418d Mon Sep 17 00:00:00 2001 From: Alex Boten Date: Mon, 4 Nov 2019 09:55:07 -0800 Subject: [PATCH 04/12] More linting fixes Signed-off-by: Alex Boten --- .../src/opentelemetry/ext/opentracing_shim/__init__.py | 1 - opentelemetry-api/src/opentelemetry/trace/__init__.py | 2 +- opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py | 6 +----- opentelemetry-sdk/tests/trace/test_trace.py | 5 ----- 4 files changed, 2 insertions(+), 12 deletions(-) diff --git a/ext/opentelemetry-ext-opentracing-shim/src/opentelemetry/ext/opentracing_shim/__init__.py b/ext/opentelemetry-ext-opentracing-shim/src/opentelemetry/ext/opentracing_shim/__init__.py index a1b39c209b1..b4b33760af3 100644 --- a/ext/opentelemetry-ext-opentracing-shim/src/opentelemetry/ext/opentracing_shim/__init__.py +++ b/ext/opentelemetry-ext-opentracing-shim/src/opentelemetry/ext/opentracing_shim/__init__.py @@ -15,7 +15,6 @@ import logging import opentracing - from deprecated import deprecated import opentelemetry.trace as trace_api diff --git a/opentelemetry-api/src/opentelemetry/trace/__init__.py b/opentelemetry-api/src/opentelemetry/trace/__init__.py index 2a32253caa0..f647102c78e 100644 --- a/opentelemetry-api/src/opentelemetry/trace/__init__.py +++ b/opentelemetry-api/src/opentelemetry/trace/__init__.py @@ -76,7 +76,7 @@ class Link: """A link to a `Span`.""" - empty_attributes = {} # type: typing.Dict[str, typing.Union[str, bool, float]] + empty_attributes = {} # type: typing.Dict[str, types.AttributeValue] def __init__( self, context: "SpanContext", attributes: types.Attributes = None diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py index 4ae3f9f1120..b78a47330c7 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py @@ -387,11 +387,7 @@ def create_span( # The sampler may also add attributes to the newly-created span, e.g. # to include information about the sampling decision. sampling_decision = self.sampler.should_sample( - parent_context, - context.trace_id, - context.span_id, - name, - {}, # TODO: links + parent_context, context.trace_id, context.span_id, name, links, ) if sampling_decision.sampled: diff --git a/opentelemetry-sdk/tests/trace/test_trace.py b/opentelemetry-sdk/tests/trace/test_trace.py index d44474f0d88..18272a4ebfd 100644 --- a/opentelemetry-sdk/tests/trace/test_trace.py +++ b/opentelemetry-sdk/tests/trace/test_trace.py @@ -361,11 +361,6 @@ def test_ended_span(self): """"Events, attributes are not allowed after span is ended""" tracer = trace.Tracer("test_ended_span") - other_context1 = trace_api.SpanContext( - trace_id=trace.generate_trace_id(), - span_id=trace.generate_span_id(), - ) - with tracer.start_as_current_span("root") as root: # everything should be empty at the beginning self.assertEqual(len(root.attributes), 0) From ac9d6f67a40de2d47083dbd1c39ecb78e0684d2f Mon Sep 17 00:00:00 2001 From: Alex Boten Date: Tue, 5 Nov 2019 16:55:20 -0800 Subject: [PATCH 05/12] Don't need empty_attributes here. Signed-off-by: Alex Boten --- opentelemetry-api/src/opentelemetry/trace/__init__.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/opentelemetry-api/src/opentelemetry/trace/__init__.py b/opentelemetry-api/src/opentelemetry/trace/__init__.py index f647102c78e..c1fc89805d3 100644 --- a/opentelemetry-api/src/opentelemetry/trace/__init__.py +++ b/opentelemetry-api/src/opentelemetry/trace/__init__.py @@ -76,15 +76,15 @@ class Link: """A link to a `Span`.""" - empty_attributes = {} # type: typing.Dict[str, types.AttributeValue] - def __init__( self, context: "SpanContext", attributes: types.Attributes = None ) -> None: self._context = context self._attributes = attributes if attributes is None: - self._attributes = Link.empty_attributes + self._attributes = ( + {} + ) # type: typing.Dict[str, types.AttributeValue] @property def context(self) -> "SpanContext": From f888308884cc6b6b982a6d90984e0ba782b4692f Mon Sep 17 00:00:00 2001 From: Alex Boten Date: Wed, 6 Nov 2019 09:06:49 -0800 Subject: [PATCH 06/12] Don't modify _attributes Signed-off-by: Alex Boten --- opentelemetry-api/src/opentelemetry/trace/__init__.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/opentelemetry-api/src/opentelemetry/trace/__init__.py b/opentelemetry-api/src/opentelemetry/trace/__init__.py index 9fa623a51f7..84a379845ad 100644 --- a/opentelemetry-api/src/opentelemetry/trace/__init__.py +++ b/opentelemetry-api/src/opentelemetry/trace/__init__.py @@ -80,11 +80,12 @@ def __init__( self, context: "SpanContext", attributes: types.Attributes = None ) -> None: self._context = context - self._attributes = attributes if attributes is None: self._attributes = ( {} ) # type: typing.Dict[str, types.AttributeValue] + else: + self._attributes = attributes @property def context(self) -> "SpanContext": From 544be127f1dce617f09071d2aa64ff3cd7400649 Mon Sep 17 00:00:00 2001 From: Alex Boten Date: Wed, 6 Nov 2019 17:02:00 -0800 Subject: [PATCH 07/12] Use types.Attributes alias Signed-off-by: Alex Boten --- opentelemetry-api/src/opentelemetry/trace/__init__.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/opentelemetry-api/src/opentelemetry/trace/__init__.py b/opentelemetry-api/src/opentelemetry/trace/__init__.py index 84a379845ad..65c37eb30cb 100644 --- a/opentelemetry-api/src/opentelemetry/trace/__init__.py +++ b/opentelemetry-api/src/opentelemetry/trace/__init__.py @@ -81,9 +81,7 @@ def __init__( ) -> None: self._context = context if attributes is None: - self._attributes = ( - {} - ) # type: typing.Dict[str, types.AttributeValue] + self._attributes = {} # type: types.Attributes else: self._attributes = attributes From 47dfa13fe3c1c95536faab962fcc98cb9f8f045c Mon Sep 17 00:00:00 2001 From: Alex Boten Date: Thu, 7 Nov 2019 11:32:50 -0800 Subject: [PATCH 08/12] Documenting the links argument Signed-off-by: Alex Boten --- opentelemetry-api/src/opentelemetry/trace/__init__.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/opentelemetry-api/src/opentelemetry/trace/__init__.py b/opentelemetry-api/src/opentelemetry/trace/__init__.py index 65c37eb30cb..474d6a44c62 100644 --- a/opentelemetry-api/src/opentelemetry/trace/__init__.py +++ b/opentelemetry-api/src/opentelemetry/trace/__init__.py @@ -431,6 +431,7 @@ def start_span( parent: The span's parent. Defaults to the current span. kind: The span's kind (relationship to parent). Note that is meaningful even if there is no parent. + links: The span's links, to be used in sampling decisions Returns: The newly-created span. @@ -480,6 +481,7 @@ def start_as_current_span( parent: The span's parent. Defaults to the current span. kind: The span's kind (relationship to parent). Note that is meaningful even if there is no parent. + links: The span's links, to be used in sampling decisions Yields: The newly-created span. @@ -523,6 +525,7 @@ def create_span( parent: The span's parent. Defaults to the current span. kind: The span's kind (relationship to parent). Note that is meaningful even if there is no parent. + links: The span's links, to be used in sampling decisions Returns: The newly-created span. From 8d95e12b2d6e92db8cbd57d456fd5d4e4b85aeed Mon Sep 17 00:00:00 2001 From: Alex Boten Date: Fri, 8 Nov 2019 13:33:15 -0800 Subject: [PATCH 09/12] Changes from review feedback - updating comments - updating default value of links for Span constructor Signed-off-by: Alex Boten --- .../src/opentelemetry/ext/opentracing_shim/__init__.py | 2 +- opentelemetry-api/src/opentelemetry/trace/__init__.py | 6 +++--- opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/ext/opentelemetry-ext-opentracing-shim/src/opentelemetry/ext/opentracing_shim/__init__.py b/ext/opentelemetry-ext-opentracing-shim/src/opentelemetry/ext/opentracing_shim/__init__.py index 5c659879102..dcaa91bb410 100644 --- a/ext/opentelemetry-ext-opentracing-shim/src/opentelemetry/ext/opentracing_shim/__init__.py +++ b/ext/opentelemetry-ext-opentracing-shim/src/opentelemetry/ext/opentracing_shim/__init__.py @@ -228,7 +228,7 @@ def start_span( # use a `None` parent, which triggers the creation of a new trace. parent = child_of.unwrap() if child_of else None - links = None + links = () if references: links = [] for ref in references: diff --git a/opentelemetry-api/src/opentelemetry/trace/__init__.py b/opentelemetry-api/src/opentelemetry/trace/__init__.py index 474d6a44c62..91e2c1d9da0 100644 --- a/opentelemetry-api/src/opentelemetry/trace/__init__.py +++ b/opentelemetry-api/src/opentelemetry/trace/__init__.py @@ -431,7 +431,7 @@ def start_span( parent: The span's parent. Defaults to the current span. kind: The span's kind (relationship to parent). Note that is meaningful even if there is no parent. - links: The span's links, to be used in sampling decisions + links: Links span to other spans Returns: The newly-created span. @@ -481,7 +481,7 @@ def start_as_current_span( parent: The span's parent. Defaults to the current span. kind: The span's kind (relationship to parent). Note that is meaningful even if there is no parent. - links: The span's links, to be used in sampling decisions + links: Links span to other spans Yields: The newly-created span. @@ -525,7 +525,7 @@ def create_span( parent: The span's parent. Defaults to the current span. kind: The span's kind (relationship to parent). Note that is meaningful even if there is no parent. - links: The span's links, to be used in sampling decisions + links: Links span to other spans Returns: The newly-created span. diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py index 94dffdc4bd9..429eb101d5e 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py @@ -130,7 +130,7 @@ def __init__( resource: None = None, # TODO attributes: types.Attributes = None, # TODO events: Sequence[trace_api.Event] = None, # TODO - links: Sequence[trace_api.Link] = None, + links: Sequence[trace_api.Link] = (), kind: trace_api.SpanKind = trace_api.SpanKind.INTERNAL, span_processor: SpanProcessor = SpanProcessor(), ) -> None: From b641ba4b3fee1920dea1874faeef624f19b0fc69 Mon Sep 17 00:00:00 2001 From: Alex Boten Date: Tue, 12 Nov 2019 11:06:48 -0800 Subject: [PATCH 10/12] adding attributes to span creation - adding `attributes` as optional parameter to `create_span`, `start_as_current_span`, `start_span` - adding `attributes` as optional parameter to Sampler's `should_sample` to allow them to be considered during sampling decision Signed-off-by: Alex Boten --- .../src/opentelemetry/trace/__init__.py | 6 +++ .../src/opentelemetry/trace/sampling.py | 5 ++- .../src/opentelemetry/sdk/trace/__init__.py | 21 ++++++++-- opentelemetry-sdk/tests/trace/test_trace.py | 40 +++++++++++++++++++ 4 files changed, 67 insertions(+), 5 deletions(-) diff --git a/opentelemetry-api/src/opentelemetry/trace/__init__.py b/opentelemetry-api/src/opentelemetry/trace/__init__.py index 91e2c1d9da0..3c265935608 100644 --- a/opentelemetry-api/src/opentelemetry/trace/__init__.py +++ b/opentelemetry-api/src/opentelemetry/trace/__init__.py @@ -402,6 +402,7 @@ def start_span( name: str, parent: ParentSpan = CURRENT_SPAN, kind: SpanKind = SpanKind.INTERNAL, + attributes: typing.Optional[types.Attributes] = None, links: typing.Sequence[Link] = (), ) -> "Span": """Starts a span. @@ -431,6 +432,7 @@ def start_span( parent: The span's parent. Defaults to the current span. kind: The span's kind (relationship to parent). Note that is meaningful even if there is no parent. + attributes: The span's attributes. links: Links span to other spans Returns: @@ -445,6 +447,7 @@ def start_as_current_span( name: str, parent: ParentSpan = CURRENT_SPAN, kind: SpanKind = SpanKind.INTERNAL, + attributes: typing.Optional[types.Attributes] = None, links: typing.Sequence[Link] = (), ) -> typing.Iterator["Span"]: """Context manager for creating a new span and set it @@ -481,6 +484,7 @@ def start_as_current_span( parent: The span's parent. Defaults to the current span. kind: The span's kind (relationship to parent). Note that is meaningful even if there is no parent. + attributes: The span's attributes. links: Links span to other spans Yields: @@ -495,6 +499,7 @@ def create_span( name: str, parent: ParentSpan = CURRENT_SPAN, kind: SpanKind = SpanKind.INTERNAL, + attributes: typing.Optional[types.Attributes] = None, links: typing.Sequence[Link] = (), ) -> "Span": """Creates a span. @@ -525,6 +530,7 @@ def create_span( parent: The span's parent. Defaults to the current span. kind: The span's kind (relationship to parent). Note that is meaningful even if there is no parent. + attributes: The span's attributes. links: Links span to other spans Returns: diff --git a/opentelemetry-api/src/opentelemetry/trace/sampling.py b/opentelemetry-api/src/opentelemetry/trace/sampling.py index f16e80495bf..967f4fd46b9 100644 --- a/opentelemetry-api/src/opentelemetry/trace/sampling.py +++ b/opentelemetry-api/src/opentelemetry/trace/sampling.py @@ -17,7 +17,7 @@ # pylint: disable=unused-import from opentelemetry.trace import Link, SpanContext -from opentelemetry.util.types import AttributeValue +from opentelemetry.util.types import Attributes, AttributeValue class Decision: @@ -53,6 +53,7 @@ def should_sample( trace_id: int, span_id: int, name: str, + attributes: Optional[Attributes] = None, links: Sequence["Link"] = (), ) -> "Decision": pass @@ -70,6 +71,7 @@ def should_sample( trace_id: int, span_id: int, name: str, + attributes: Optional[Attributes] = None, links: Sequence["Link"] = (), ) -> "Decision": return self._decision @@ -107,6 +109,7 @@ def should_sample( trace_id: int, span_id: int, name: str, + attributes: Optional[Attributes] = None, # TODO links: Sequence["Link"] = (), ) -> "Decision": if parent_context is not None: diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py index 429eb101d5e..5407eebe6f8 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py @@ -319,11 +319,12 @@ def start_span( name: str, parent: trace_api.ParentSpan = trace_api.Tracer.CURRENT_SPAN, kind: trace_api.SpanKind = trace_api.SpanKind.INTERNAL, + attributes: Optional[types.Attributes] = None, links: Sequence[trace_api.Link] = (), ) -> "Span": """See `opentelemetry.trace.Tracer.start_span`.""" - span = self.create_span(name, parent, kind, links) + span = self.create_span(name, parent, kind, attributes, links) span.start() return span @@ -332,11 +333,12 @@ def start_as_current_span( name: str, parent: trace_api.ParentSpan = trace_api.Tracer.CURRENT_SPAN, kind: trace_api.SpanKind = trace_api.SpanKind.INTERNAL, + attributes: Optional[types.Attributes] = None, links: Sequence[trace_api.Link] = (), ) -> Iterator[trace_api.Span]: """See `opentelemetry.trace.Tracer.start_as_current_span`.""" - span = self.start_span(name, parent, kind, links) + span = self.start_span(name, parent, kind, attributes, links) return self.use_span(span, end_on_exit=True) def create_span( @@ -344,6 +346,7 @@ def create_span( name: str, parent: trace_api.ParentSpan = trace_api.Tracer.CURRENT_SPAN, kind: trace_api.SpanKind = trace_api.SpanKind.INTERNAL, + attributes: Optional[types.Attributes] = None, links: Sequence[trace_api.Link] = (), ) -> "trace_api.Span": """See `opentelemetry.trace.Tracer.create_span`. @@ -387,16 +390,26 @@ def create_span( # The sampler may also add attributes to the newly-created span, e.g. # to include information about the sampling decision. sampling_decision = self.sampler.should_sample( - parent_context, context.trace_id, context.span_id, name, links, + parent_context, + context.trace_id, + context.span_id, + name, + attributes, + links, ) if sampling_decision.sampled: + if attributes is None: + attributes = sampling_decision.attributes + else: + # apply sampling decision attributes after initial attributes + attributes = {**attributes, **sampling_decision.attributes} return Span( name=name, context=context, parent=parent, sampler=self.sampler, - attributes=sampling_decision.attributes, + attributes=attributes, span_processor=self._active_span_processor, kind=kind, links=links, diff --git a/opentelemetry-sdk/tests/trace/test_trace.py b/opentelemetry-sdk/tests/trace/test_trace.py index 3a9f3e98ed8..0d36c003e0f 100644 --- a/opentelemetry-sdk/tests/trace/test_trace.py +++ b/opentelemetry-sdk/tests/trace/test_trace.py @@ -248,6 +248,46 @@ def test_attributes(self): self.assertEqual(root.attributes["misc.pi"], 3.14) self.assertEqual(root.attributes["attr-key"], "attr-value2") + attributes = { + "attr-key": "val", + "attr-key2": "val2", + "attr-in-both": "span-attr", + } + with self.tracer.start_as_current_span( + "root2", attributes=attributes + ) as root: + self.assertEqual(len(root.attributes), 3) + self.assertEqual(root.attributes["attr-key"], "val") + self.assertEqual(root.attributes["attr-key2"], "val2") + self.assertEqual(root.attributes["attr-in-both"], "span-attr") + + decision_attributes = { + "sampler-attr": "sample-val", + "attr-in-both": "decision-attr", + } + self.tracer.sampler = sampling.StaticSampler( + sampling.Decision(sampled=True, attributes=decision_attributes) + ) + + with self.tracer.start_as_current_span("root2") as root: + self.assertEqual(len(root.attributes), 2) + self.assertEqual(root.attributes["sampler-attr"], "sample-val") + self.assertEqual(root.attributes["attr-in-both"], "decision-attr") + + attributes = { + "attr-key": "val", + "attr-key2": "val2", + "attr-in-both": "span-attr", + } + with self.tracer.start_as_current_span( + "root2", attributes=attributes + ) as root: + self.assertEqual(len(root.attributes), 4) + self.assertEqual(root.attributes["attr-key"], "val") + self.assertEqual(root.attributes["attr-key2"], "val2") + self.assertEqual(root.attributes["sampler-attr"], "sample-val") + self.assertEqual(root.attributes["attr-in-both"], "decision-attr") + def test_events(self): self.assertIsNone(self.tracer.get_current_span()) From d95151db0511163aac2c59f83aa0ab970f145fa4 Mon Sep 17 00:00:00 2001 From: Alex Boten Date: Tue, 12 Nov 2019 12:21:54 -0800 Subject: [PATCH 11/12] Fixing for for python 3.4 Signed-off-by: Alex Boten --- opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py index 5407eebe6f8..9ac9d81bc2c 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py @@ -400,16 +400,17 @@ def create_span( if sampling_decision.sampled: if attributes is None: - attributes = sampling_decision.attributes + span_attributes = sampling_decision.attributes else: # apply sampling decision attributes after initial attributes - attributes = {**attributes, **sampling_decision.attributes} + span_attributes = attributes.copy() + span_attributes.update(sampling_decision.attributes) return Span( name=name, context=context, parent=parent, sampler=self.sampler, - attributes=attributes, + attributes=span_attributes, span_processor=self._active_span_processor, kind=kind, links=links, From 4e0c328e5b8689238777003f22747b1b7cae2eea Mon Sep 17 00:00:00 2001 From: Alex Boten Date: Fri, 15 Nov 2019 09:45:17 -0800 Subject: [PATCH 12/12] clean up Signed-off-by: Alex Boten --- .../src/opentelemetry/ext/opentracing_shim/__init__.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/ext/opentelemetry-ext-opentracing-shim/src/opentelemetry/ext/opentracing_shim/__init__.py b/ext/opentelemetry-ext-opentracing-shim/src/opentelemetry/ext/opentracing_shim/__init__.py index b9081d457ca..eefb85466ae 100644 --- a/ext/opentelemetry-ext-opentracing-shim/src/opentelemetry/ext/opentracing_shim/__init__.py +++ b/ext/opentelemetry-ext-opentracing-shim/src/opentelemetry/ext/opentracing_shim/__init__.py @@ -233,9 +233,8 @@ def start_span( # use a `None` parent, which triggers the creation of a new trace. parent = child_of.unwrap() if child_of else None - links = () + links = [] if references: - links = [] for ref in references: links.append(trace_api.Link(ref.referenced_context.unwrap()))