-
Notifications
You must be signed in to change notification settings - Fork 610
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
Named tracers #301
Changes from 22 commits
d5c1da1
94c72f8
f1a23c1
d91e5dd
2f95b61
8014e52
fbf4a1e
716ead7
ac5dc8b
7e9dad7
009418a
e69f102
9163c9b
ab805d2
c987246
2aa258a
f05eaaf
a6a2670
e28713e
0cbdccb
9236606
725bb16
4de385e
7907133
b7c688c
e4db217
239a18d
b48e708
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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__) | ||
|
@@ -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__ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That would be another possibility. There should probably a unique mapping There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, one risk of copying |
||
) | ||
|
||
span = tracer.start_span( | ||
span_name, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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( | ||
|
@@ -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: | ||
|
There was a problem hiding this comment.
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:
Although I'm also a fan of adding these into the top level trace interface, as that's simpler: