Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Named tracers #301

Merged
merged 28 commits into from
Dec 14, 2019
Merged
Show file tree
Hide file tree
Changes from 22 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
d5c1da1
Named tracer.
Oberon00 Nov 22, 2019
94c72f8
Adapt all the things to named tracers.
Oberon00 Nov 22, 2019
f1a23c1
Fix examples, move add_span_processor to source.
Oberon00 Nov 22, 2019
d91e5dd
Fix examples for add_span_processor move.
Oberon00 Nov 22, 2019
2f95b61
Fix examples for good.
Oberon00 Nov 22, 2019
8014e52
Add unit tests.
Oberon00 Nov 22, 2019
fbf4a1e
Fix W3C tests, lint, docs.
Oberon00 Nov 22, 2019
716ead7
Increase test coverage.
Oberon00 Nov 22, 2019
ac5dc8b
Merge branch 'master' into named-tracers
Oberon00 Nov 25, 2019
7e9dad7
Adapt flask extension for named tracers.
Oberon00 Nov 25, 2019
009418a
Fix mismerge of da8b8d907c29fc358ca635114dac2967b6501563.
Oberon00 Nov 25, 2019
e69f102
Merge branch 'master' into named-tracers
Oberon00 Nov 26, 2019
9163c9b
Fix tracer -> tracer source in comments/doc.
Oberon00 Dec 5, 2019
ab805d2
Update opentelemetry-api/src/opentelemetry/trace/__init__.py
Oberon00 Dec 5, 2019
c987246
Merge branch 'master' into named-tracers
Oberon00 Dec 5, 2019
2aa258a
Adjust new tests to named tracers.
Oberon00 Dec 5, 2019
f05eaaf
Rewrap comments
c24t Dec 6, 2019
a6a2670
Fix stray examples
c24t Dec 6, 2019
e28713e
Merge remote-tracking branch 'upstream/master' into named-tracers
Oberon00 Dec 9, 2019
0cbdccb
Adapt new tests.
Oberon00 Dec 9, 2019
9236606
Fix docstrings.
Oberon00 Dec 9, 2019
725bb16
Rename creator_info to instrumentation_info.
Oberon00 Dec 9, 2019
4de385e
Merge branch 'master' into named-tracers-merge
c24t Dec 10, 2019
7907133
Merge branch 'master' into named-tracers
Oberon00 Dec 12, 2019
b7c688c
Fix tests.
Oberon00 Dec 13, 2019
e4db217
libname -> module name
Oberon00 Dec 13, 2019
239a18d
Lint.
Oberon00 Dec 13, 2019
b48e708
Fix docs build.
Oberon00 Dec 13, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -52,15 +52,15 @@ pip install -e ./ext/opentelemetry-ext-{integration}
```python
from opentelemetry import trace
from opentelemetry.context import Context
from opentelemetry.sdk.trace import Tracer
from opentelemetry.sdk.trace import TracerSource
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

another idea on naming: "tracing". Since it's conceptually similar to the logging module, we could use that interface convention:

tracing.get_tracer()

Although I'm also a fan of adding these into the top level trace interface, as that's simpler:

trace.get_tracer(__name__)

from opentelemetry.sdk.trace.export import ConsoleSpanExporter
from opentelemetry.sdk.trace.export import SimpleExportSpanProcessor

trace.set_preferred_tracer_implementation(lambda T: Tracer())
tracer = trace.tracer()
tracer.add_span_processor(
trace.set_preferred_tracer_source_implementation(lambda T: TracerSource())
trace.tracer_source().add_span_processor(
SimpleExportSpanProcessor(ConsoleSpanExporter())
)
tracer = trace.tracer_source().get_tracer("myapp")
with tracer.start_as_current_span('foo'):
with tracer.start_as_current_span('bar'):
with tracer.start_as_current_span('baz'):
Expand Down
14 changes: 10 additions & 4 deletions examples/basic_tracer/tracer.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@

from opentelemetry import trace
from opentelemetry.context import Context
from opentelemetry.sdk.trace import Tracer
from opentelemetry.sdk.trace import TracerSource
from opentelemetry.sdk.trace.export import (
BatchExportSpanProcessor,
ConsoleSpanExporter,
Expand All @@ -37,13 +37,19 @@

# The preferred tracer implementation must be set, as the opentelemetry-api
# defines the interface with a no-op implementation.
trace.set_preferred_tracer_implementation(lambda T: Tracer())
tracer = trace.tracer()
trace.set_preferred_tracer_source_implementation(lambda T: TracerSource())

# We tell OpenTelemetry who it is that is creating spans. In this case, we have
# no real name (no setup.py), so we make one up. If we had a version, we would
# also specify it here.
tracer = trace.tracer_source().get_tracer(
"opentelemetry-examples-basic-tracer"
)

# SpanExporter receives the spans and send them to the target location.
span_processor = BatchExportSpanProcessor(exporter)

tracer.add_span_processor(span_processor)
trace.tracer_source().add_span_processor(span_processor)
with tracer.start_as_current_span("foo"):
with tracer.start_as_current_span("bar"):
with tracer.start_as_current_span("baz"):
Expand Down
10 changes: 5 additions & 5 deletions examples/http/server.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
from opentelemetry import trace
from opentelemetry.ext import http_requests
from opentelemetry.ext.wsgi import OpenTelemetryMiddleware
from opentelemetry.sdk.trace import Tracer
from opentelemetry.sdk.trace import TracerSource
from opentelemetry.sdk.trace.export import (
BatchExportSpanProcessor,
ConsoleSpanExporter,
Expand All @@ -41,17 +41,17 @@

# The preferred tracer implementation must be set, as the opentelemetry-api
# defines the interface with a no-op implementation.
trace.set_preferred_tracer_implementation(lambda T: Tracer())
tracer = trace.tracer()
trace.set_preferred_tracer_source_implementation(lambda T: TracerSource())
tracer = trace.tracer_source().get_tracer("opentelemetry-example-http")

# SpanExporter receives the spans and send them to the target location.
span_processor = BatchExportSpanProcessor(exporter)
tracer.add_span_processor(span_processor)
trace.tracer_source().add_span_processor(span_processor)

# Integrations are the glue that binds the OpenTelemetry API and the
# frameworks and libraries that are used together, automatically creating
# Spans and propagating context as appropriate.
http_requests.enable(tracer)
http_requests.enable(trace.tracer_source())
app = flask.Flask(__name__)
app.wsgi_app = OpenTelemetryMiddleware(app.wsgi_app)

Expand Down
10 changes: 5 additions & 5 deletions examples/http/tracer_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@

from opentelemetry import trace
from opentelemetry.ext import http_requests
from opentelemetry.sdk.trace import Tracer
from opentelemetry.sdk.trace import TracerSource
from opentelemetry.sdk.trace.export import (
BatchExportSpanProcessor,
ConsoleSpanExporter,
Expand All @@ -39,15 +39,15 @@

# The preferred tracer implementation must be set, as the opentelemetry-api
# defines the interface with a no-op implementation.
trace.set_preferred_tracer_implementation(lambda T: Tracer())
tracer = trace.tracer()
trace.set_preferred_tracer_source_implementation(lambda T: TracerSource())
tracer_source = trace.tracer_source()

# SpanExporter receives the spans and send them to the target location.
span_processor = BatchExportSpanProcessor(exporter)
tracer.add_span_processor(span_processor)
tracer_source.add_span_processor(span_processor)

# Integrations are the glue that binds the OpenTelemetry API and the
# frameworks and libraries that are used together, automatically creating
# Spans and propagating context as appropriate.
http_requests.enable(tracer)
http_requests.enable(tracer_source)
response = requests.get(url="http://127.0.0.1:5000/")
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,14 @@
the requests library to perform downstream requests
"""
import flask
import pkg_resources
import requests

import opentelemetry.ext.http_requests
from opentelemetry import propagators, trace
from opentelemetry.ext.flask import instrument_app
from opentelemetry.sdk.context.propagation.b3_format import B3Format
from opentelemetry.sdk.trace import Tracer
from opentelemetry.sdk.trace import TracerSource


def configure_opentelemetry(flask_app: flask.Flask):
Expand All @@ -45,7 +46,7 @@ def configure_opentelemetry(flask_app: flask.Flask):
# the preferred implementation of these objects must be set,
# as the opentelemetry-api defines the interface with a no-op
# implementation.
trace.set_preferred_tracer_implementation(lambda _: Tracer())
trace.set_preferred_tracer_source_implementation(lambda _: TracerSource())
# Next, we need to configure how the values that are used by
# traces and metrics are propagated (such as what specific headers
# carry this value).
Expand All @@ -56,7 +57,7 @@ def configure_opentelemetry(flask_app: flask.Flask):
# Integrations are the glue that binds the OpenTelemetry API
# and the frameworks and libraries that are used together, automatically
# creating Spans and propagating context as appropriate.
opentelemetry.ext.http_requests.enable(trace.tracer())
opentelemetry.ext.http_requests.enable(trace.tracer_source())
instrument_app(flask_app)


Expand All @@ -67,7 +68,13 @@ def configure_opentelemetry(flask_app: flask.Flask):
def hello():
# emit a trace that measures how long the
# sleep takes
with trace.tracer().start_as_current_span("example-request"):
version = pkg_resources.get_distribution(
"opentelemetry-example-app"
).version
tracer = trace.tracer_source().get_tracer(
"opentelemetry-example-app", version
)
with tracer.start_as_current_span("example-request"):
requests.get("http://www.example.com")
return "hello"

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

import opentelemetry.ext.wsgi as otel_wsgi
from opentelemetry import propagators, trace
from opentelemetry.ext.flask.version import __version__
from opentelemetry.util import time_ns

logger = logging.getLogger(__name__)
Expand Down Expand Up @@ -60,7 +61,9 @@ def _before_flask_request():
otel_wsgi.get_header_from_environ, environ
)

tracer = trace.tracer()
tracer = trace.tracer_source().get_tracer(
"opentelemetry-ext-flask", __version__
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Out of curiosity: why use the package name instead of the dotted namespace? E.g. opentelemetry.ext.flask? The later is closer to the __file__ convention for logger names, and you can have multiple unique names per package.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That would be another possibility. There should probably a unique mapping full module name -> package anyway.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 on the suggestion to use the name. Most loggers are configured that way as well, and having parity of the logging namespace and tracer namespace could be very useful.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I changed it to the module name in e4db217. But note that once we have a registry pattern, we will be creating a tracer per module instead of per library with the most naive implementation at least.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, one risk of copying logging here is that users might reasonably assume these names are meant to be hierarchical too.

)

span = tracer.start_span(
span_name,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@

from opentelemetry import propagators
from opentelemetry.context import Context
from opentelemetry.ext.http_requests.version import __version__
from opentelemetry.trace import SpanKind


Expand All @@ -32,7 +33,7 @@
# if the SDK/tracer is already using `requests` they may, in theory, bypass our
# instrumentation when using `import from`, etc. (currently we only instrument
# a instance method so the probability for that is very low).
def enable(tracer):
def enable(tracer_source):
"""Enables tracing of all requests calls that go through
:code:`requests.session.Session.request` (this includes
:code:`requests.get`, etc.)."""
Expand All @@ -47,6 +48,10 @@ def enable(tracer):
# Guard against double instrumentation
disable()

tracer = tracer_source.get_tracer(
"opentelemetry-ext-http-requests", __version__
)

wrapped = Session.request

@functools.wraps(wrapped)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import unittest
from unittest import mock

import pkg_resources
import requests
import urllib3

Expand All @@ -28,7 +29,16 @@ class TestRequestsIntegration(unittest.TestCase):
# TODO: Copy & paste from test_wsgi_middleware
def setUp(self):
self.span_attrs = {}
self.tracer = trace.tracer()
self.tracer_source = trace.TracerSource()
self.tracer = trace.Tracer()
self.get_tracer_patcher = mock.patch.object(
self.tracer_source,
"get_tracer",
autospec=True,
spec_set=True,
return_value=self.tracer,
)
self.get_tracer = self.get_tracer_patcher.start()
self.span_context_manager = mock.MagicMock()
self.span = mock.create_autospec(trace.Span, spec_set=True)
self.span_context_manager.__enter__.return_value = self.span
Expand Down Expand Up @@ -59,18 +69,25 @@ def setspanattr(key, value):
)
self.send = self.send_patcher.start()

opentelemetry.ext.http_requests.enable(self.tracer)
opentelemetry.ext.http_requests.enable(self.tracer_source)
distver = pkg_resources.get_distribution(
"opentelemetry-ext-http-requests"
).version
self.get_tracer.assert_called_with(
"opentelemetry-ext-http-requests", distver
)

def tearDown(self):
opentelemetry.ext.http_requests.disable()
self.get_tracer_patcher.stop()
self.send_patcher.stop()
self.start_span_patcher.stop()

def test_basic(self):
url = "https://www.example.org/foo/bar?x=y#top"
requests.get(url=url)
self.assertEqual(1, len(self.send.call_args_list))
self.tracer.start_as_current_span.assert_called_with(
self.tracer.start_as_current_span.assert_called_with( # pylint:disable=no-member
"/foo/bar", kind=trace.SpanKind.CLIENT
)
self.span_context_manager.__enter__.assert_called_with()
Expand All @@ -96,11 +113,12 @@ def test_invalid_url(self):

with self.assertRaises(exception_type):
requests.post(url=url)
call_args = (
self.tracer.start_as_current_span.call_args # pylint:disable=no-member
)
self.assertTrue(
self.tracer.start_as_current_span.call_args[0][0].startswith(
"<Unparsable URL"
),
msg=self.tracer.start_as_current_span.call_args,
call_args[0][0].startswith("<Unparsable URL"),
msg=self.tracer.start_as_current_span.call_args, # pylint:disable=no-member
)
self.span_context_manager.__enter__.assert_called_with()
exitspan = self.span_context_manager.__exit__
Expand Down
6 changes: 3 additions & 3 deletions ext/opentelemetry-ext-jaeger/README.rst
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,11 @@ gRPC is still not supported by this implementation.

from opentelemetry import trace
from opentelemetry.ext import jaeger
from opentelemetry.sdk.trace import Tracer
from opentelemetry.sdk.trace import TracerSource
from opentelemetry.sdk.trace.export import BatchExportSpanProcessor

trace.set_preferred_tracer_implementation(lambda T: Tracer())
tracer = trace.tracer()
trace.set_preferred_tracer_source_implementation(lambda T: TracerSource())
tracer = trace.tracer_source().get_tracer("myapp")

# create a JaegerSpanExporter
jaeger_exporter = jaeger.JaegerSpanExporter(
Expand Down
10 changes: 5 additions & 5 deletions ext/opentelemetry-ext-jaeger/examples/jaeger_exporter_example.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,11 @@

from opentelemetry import trace
from opentelemetry.ext import jaeger
from opentelemetry.sdk.trace import Tracer
from opentelemetry.sdk.trace import TracerSource
from opentelemetry.sdk.trace.export import BatchExportSpanProcessor

trace.set_preferred_tracer_implementation(lambda T: Tracer())
tracer = trace.tracer()
trace.set_preferred_tracer_source_implementation(lambda T: TracerSource())
tracer = trace.tracer_source().get_tracer("myapp")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I worry if we put all the examples as get_tracer("myapp"), that might cause a lot of people to not understand the convention to use the package / module name in the tracer, and make integrations that are not part of this repo harder to turn on / off.

How about following Flask's example of passing in name? Another argument to just use module names, in my opinion.


# create a JaegerSpanExporter
jaeger_exporter = jaeger.JaegerSpanExporter(
Expand All @@ -25,8 +25,8 @@
# create a BatchExportSpanProcessor and add the exporter to it
span_processor = BatchExportSpanProcessor(jaeger_exporter)

# add to the tracer
tracer.add_span_processor(span_processor)
# add to the tracer factory
trace.tracer_source().add_span_processor(span_processor)

# create some spans for testing
with tracer.start_as_current_span("foo") as foo:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,13 +29,15 @@
import time

from opentelemetry import trace
from opentelemetry.sdk.trace import Tracer
from opentelemetry.sdk.trace import TracerSource
from opentelemetry.ext.opentracing_shim import create_tracer

# Tell OpenTelemetry which Tracer implementation to use.
trace.set_preferred_tracer_implementation(lambda T: Tracer())
trace.set_preferred_tracer_source_implementation(lambda T: TracerSource())

# Create an OpenTelemetry Tracer.
otel_tracer = trace.tracer()
otel_tracer = trace.tracer_source().get_tracer("myapp")

# Create an OpenTracing shim.
shim = create_tracer(otel_tracer)

Expand Down Expand Up @@ -86,28 +88,33 @@
import opentelemetry.trace as trace_api
from opentelemetry import propagators
from opentelemetry.ext.opentracing_shim import util
from opentelemetry.ext.opentracing_shim.version import __version__

logger = logging.getLogger(__name__)


def create_tracer(otel_tracer):
def create_tracer(otel_tracer_source):
"""Creates a :class:`TracerShim` object from the provided OpenTelemetry
:class:`opentelemetry.trace.Tracer`.
:class:`opentelemetry.trace.TracerSource`.

The returned :class:`TracerShim` is an implementation of
:class:`opentracing.Tracer` using OpenTelemetry under the hood.

Args:
otel_tracer: A :class:`opentelemetry.trace.Tracer` to be used for
constructing the :class:`TracerShim`. This tracer will be used
otel_tracer_source: A :class:`opentelemetry.trace.TracerSource` to be used for
constructing the :class:`TracerShim`. A tracer from this source will be used
to perform the actual tracing when user code is instrumented using
the OpenTracing API.

Returns:
The created :class:`TracerShim`.
"""

return TracerShim(otel_tracer)
return TracerShim(
otel_tracer_source.get_tracer(
"opentelemetry-ext-opentracing-shim", __version__
)
)


class SpanContextShim(opentracing.SpanContext):
Expand Down
Loading