Skip to content

Commit

Permalink
Replaced Tracer.use_span() with opentelemetry.trace.use_span()
Browse files Browse the repository at this point in the history
  • Loading branch information
owais committed Mar 4, 2021
1 parent aa31e73 commit 096d69d
Show file tree
Hide file tree
Showing 32 changed files with 56 additions and 91 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ on:
- 'release/*'
pull_request:
env:
CORE_REPO_SHA: 834462941942229e308117220e1fa1fad7cb06e3
CORE_REPO_SHA: ddff32ac77e2e22b3193b71f1e71a590a99d1eda

jobs:
build:
Expand Down
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).

## [Unreleased](https://github.com/open-telemetry/opentelemetry-python-contrib/compare/v0.18b0...HEAD)
- Updated instrumentations to use `opentelemetry.trace.use_span` instead of `Tracer.use_span()`
([#364](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/364))

### Changed
- Rename `IdsGenerator` to `IdGenerator`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
DatabaseApiIntegration,
)
from opentelemetry.trace import SpanKind
from opentelemetry.trace.status import Status, StatusCode


# pylint: disable=abstract-method
Expand Down Expand Up @@ -117,13 +116,7 @@ async def traced_execution(
name, kind=SpanKind.CLIENT
) as span:
self._populate_span(span, cursor, *args)
try:
result = await query_method(*args, **kwargs)
return result
except Exception as ex: # pylint: disable=broad-except
if span.is_recording():
span.set_status(Status(StatusCode.ERROR, str(ex)))
raise ex
return await query_method(*args, **kwargs)


def get_traced_cursor_proxy(cursor, db_api_integration, *args, **kwargs):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -289,8 +289,6 @@ def test_span_not_recording(self):
mock_span = mock.Mock()
mock_span.is_recording.return_value = False
mock_tracer.start_span.return_value = mock_span
mock_tracer.use_span.return_value.__enter__ = mock_span
mock_tracer.use_span.return_value.__exit__ = True
db_integration = AiopgIntegration(
mock_tracer, "testcomponent", connection_attributes
)
Expand Down Expand Up @@ -322,7 +320,7 @@ def test_span_failed(self):
self.assertIs(
span.status.status_code, trace_api.status.StatusCode.ERROR
)
self.assertEqual(span.status.description, "Test Exception")
self.assertEqual(span.status.description, "Exception: Test Exception")

def test_executemany(self):
db_integration = AiopgIntegration(self.tracer, "testcomponent")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,8 +82,6 @@ def test_not_recording(self):
mock_span = Mock()
mock_span.is_recording.return_value = False
mock_tracer.start_span.return_value = mock_span
mock_tracer.use_span.return_value.__enter__ = mock_span
mock_tracer.use_span.return_value.__exit__ = True
with patch("opentelemetry.trace.get_tracer") as tracer:
tracer.return_value = mock_tracer
ec2 = boto.ec2.connect_to_region("us-west-2")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,8 +81,6 @@ def test_not_recording(self):
mock_span = Mock()
mock_span.is_recording.return_value = False
mock_tracer.start_span.return_value = mock_span
mock_tracer.use_span.return_value.__enter__ = mock_span
mock_tracer.use_span.return_value.__exit__ = True
with patch("opentelemetry.trace.get_tracer") as tracer:
tracer.return_value = mock_tracer
ec2 = self.session.create_client("ec2", region_name="us-west-2")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -137,8 +137,8 @@ def _trace_prerun(self, *args, **kwargs):
operation_name, context=tracectx, kind=trace.SpanKind.CONSUMER
)

activation = self._tracer.use_span(span, end_on_exit=True)
activation.__enter__()
activation = trace.use_span(span, end_on_exit=True)
activation.__enter__() # pylint: disable=E1101
utils.attach_span(task, task_id, (span, activation))

@staticmethod
Expand Down Expand Up @@ -186,8 +186,9 @@ def _trace_before_publish(self, *args, **kwargs):
span.set_attribute(_TASK_NAME_KEY, task.name)
utils.set_attributes_from_context(span, kwargs)

activation = self._tracer.use_span(span, end_on_exit=True)
activation.__enter__()
activation = trace.use_span(span, end_on_exit=True)
activation.__enter__() # pylint: disable=E1101

utils.attach_span(task, task_id, (span, activation), is_publish=True)

headers = kwargs.get("headers")
Expand All @@ -208,7 +209,7 @@ def _trace_after_publish(*args, **kwargs):
logger.warning("no existing span found for task_id=%s", task_id)
return

activation.__exit__(None, None, None)
activation.__exit__(None, None, None) # pylint: disable=E1101
utils.detach_span(task, task_id, is_publish=True)

@staticmethod
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -371,13 +371,7 @@ def traced_execution(
name, kind=SpanKind.CLIENT
) as span:
self._populate_span(span, cursor, *args)
try:
result = query_method(*args, **kwargs)
return result
except Exception as ex: # pylint: disable=broad-except
if span.is_recording():
span.set_status(Status(StatusCode.ERROR, str(ex)))
raise ex
return query_method(*args, **kwargs)


def get_traced_cursor_proxy(cursor, db_api_integration, *args, **kwargs):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -146,8 +146,6 @@ def test_span_not_recording(self):
mock_span = mock.Mock()
mock_span.is_recording.return_value = False
mock_tracer.start_span.return_value = mock_span
mock_tracer.use_span.return_value.__enter__ = mock_span
mock_tracer.use_span.return_value.__exit__ = True
db_integration = dbapi.DatabaseApiIntegration(
mock_tracer, "testcomponent", connection_attributes
)
Expand Down Expand Up @@ -179,7 +177,7 @@ def test_span_failed(self):
self.assertIs(
span.status.status_code, trace_api.status.StatusCode.ERROR
)
self.assertEqual(span.status.description, "Test Exception")
self.assertEqual(span.status.description, "Exception: Test Exception")

def test_executemany(self):
db_integration = dbapi.DatabaseApiIntegration(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
collect_request_attributes,
)
from opentelemetry.propagate import extract
from opentelemetry.trace import SpanKind, get_tracer
from opentelemetry.trace import SpanKind, get_tracer, use_span
from opentelemetry.util.http import get_excluded_urls, get_traced_request_attrs

try:
Expand Down Expand Up @@ -118,8 +118,8 @@ def process_request(self, request):
for key, value in attributes.items():
span.set_attribute(key, value)

activation = tracer.use_span(span, end_on_exit=True)
activation.__enter__()
activation = use_span(span, end_on_exit=True)
activation.__enter__() # pylint: disable=E1101

request.META[self._environ_activation_key] = activation
request.META[self._environ_span_key] = span
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -146,8 +146,6 @@ def test_not_recording(self):
mock_span = Mock()
mock_span.is_recording.return_value = False
mock_tracer.start_span.return_value = mock_span
mock_tracer.use_span.return_value.__enter__ = mock_span
mock_tracer.use_span.return_value.__exit__ = True
with patch("opentelemetry.trace.get_tracer") as tracer:
tracer.return_value = mock_tracer
Client().get("/traced/")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -136,19 +136,15 @@ def wrapper(wrapped, _, args, kwargs):
attributes["elasticsearch.params"] = str(params)
for key, value in attributes.items():
span.set_attribute(key, value)
try:
rv = wrapped(*args, **kwargs)
if isinstance(rv, dict) and span.is_recording():
for member in _ATTRIBUTES_FROM_RESULT:
if member in rv:
span.set_attribute(
"elasticsearch.{0}".format(member),
str(rv[member]),
)
return rv
except Exception as ex: # pylint: disable=broad-except
if span.is_recording():
span.set_status(Status(StatusCode.ERROR, str(ex)))
raise ex

rv = wrapped(*args, **kwargs)
if isinstance(rv, dict) and span.is_recording():
for member in _ATTRIBUTES_FROM_RESULT:
if member in rv:
span.set_attribute(
"elasticsearch.{0}".format(member),
str(rv[member]),
)
return rv

return wrapper
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
from elasticsearch_dsl import Search

import opentelemetry.instrumentation.elasticsearch
from opentelemetry import trace
from opentelemetry.instrumentation.elasticsearch import (
ElasticsearchInstrumentor,
)
Expand Down Expand Up @@ -94,8 +95,6 @@ def test_span_not_recording(self, request_mock):
mock_span = mock.Mock()
mock_span.is_recording.return_value = False
mock_tracer.start_span.return_value = mock_span
mock_tracer.use_span.return_value.__enter__ = mock_span
mock_tracer.use_span.return_value.__exit__ = mock_span
with mock.patch("opentelemetry.trace.get_tracer") as tracer:
tracer.return_value = mock_tracer
Elasticsearch()
Expand Down Expand Up @@ -174,7 +173,9 @@ def _test_trace_error(self, code, exc):
span = spans[0]
self.assertFalse(span.status.is_ok)
self.assertEqual(span.status.status_code, code)
self.assertEqual(span.status.description, str(exc))
self.assertEqual(
span.status.description, "{}: {}".format(type(exc).__name__, exc)
)

def test_parent(self, request_mock):
request_mock.return_value = (1, {}, {})
Expand All @@ -201,7 +202,7 @@ def test_multithread(self, request_mock):
# 2. Trace something from thread-2, make thread-1 join before finishing.
# 3. Check the spans got different parents, and are in the expected order.
def target1(parent_span):
with self.tracer.use_span(parent_span):
with trace.use_span(parent_span):
es.get(index="test-index", doc_type="tweet", id=1)
ev.set()
ev.wait()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)

def __call__(self, env, start_response):
# pylint: disable=E1101
if _excluded_urls.url_disabled(env.get("PATH_INFO", "/")):
return super().__call__(env, start_response)

Expand All @@ -120,7 +121,7 @@ def __call__(self, env, start_response):
for key, value in attributes.items():
span.set_attribute(key, value)

activation = self._tracer.use_span(span, end_on_exit=True)
activation = trace.use_span(span, end_on_exit=True)
activation.__enter__()
env[_ENVIRON_SPAN_KEY] = span
env[_ENVIRON_ACTIVATION_KEY] = activation
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -199,8 +199,6 @@ def test_traced_not_recording(self):
mock_span = Mock()
mock_span.is_recording.return_value = False
mock_tracer.start_span.return_value = mock_span
mock_tracer.use_span.return_value.__enter__ = mock_span
mock_tracer.use_span.return_value.__exit__ = mock_span
with patch("opentelemetry.trace.get_tracer") as tracer:
tracer.return_value = mock_tracer
self.client().simulate_get(path="/hello?q=abc")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -138,8 +138,8 @@ def _before_request():
for key, value in attributes.items():
span.set_attribute(key, value)

activation = tracer.use_span(span, end_on_exit=True)
activation.__enter__()
activation = trace.use_span(span, end_on_exit=True)
activation.__enter__() # pylint: disable=E1101
flask_request_environ[_ENVIRON_ACTIVATION_KEY] = activation
flask_request_environ[_ENVIRON_SPAN_KEY] = span
flask_request_environ[_ENVIRON_TOKEN] = token
Expand All @@ -148,6 +148,7 @@ def _before_request():


def _teardown_request(exc):
# pylint: disable=E1101
if _excluded_urls.url_disabled(flask.request.url):
return

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -125,8 +125,6 @@ def test_not_recording(self):
mock_span = Mock()
mock_span.is_recording.return_value = False
mock_tracer.start_span.return_value = mock_span
mock_tracer.use_span.return_value.__enter__ = mock_span
mock_tracer.use_span.return_value.__exit__ = mock_span
with patch("opentelemetry.trace.get_tracer") as tracer:
tracer.return_value = mock_tracer
self.client.get("/hello/123")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,9 @@ def _set_remote_context(self, servicer_context):
else:
yield

def _start_span(self, handler_call_details, context):
def _start_span(
self, handler_call_details, context, set_status_on_exception=False
):

# standard attributes
attributes = {
Expand Down Expand Up @@ -234,6 +236,7 @@ def _start_span(self, handler_call_details, context):
name=handler_call_details.method,
kind=trace.SpanKind.SERVER,
attributes=attributes,
set_status_on_exception=set_status_on_exception,
)

def intercept_service(self, continuation, handler_call_details):
Expand All @@ -251,7 +254,9 @@ def telemetry_interceptor(request_or_iterator, context):

with self._set_remote_context(context):
with self._start_span(
handler_call_details, context
handler_call_details,
context,
set_status_on_exception=False,
) as span:
# wrap the context
context = _OpenTelemetryServicerContext(context, span)
Expand Down Expand Up @@ -283,7 +288,9 @@ def _intercept_server_stream(
):

with self._set_remote_context(context):
with self._start_span(handler_call_details, context) as span:
with self._start_span(
handler_call_details, context, set_status_on_exception=False
) as span:
context = _OpenTelemetryServicerContext(context, span)

try:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,6 @@ def test_render_not_recording(self):
mock_span = mock.Mock()
mock_span.is_recording.return_value = False
mock_tracer.start_span.return_value = mock_span
mock_tracer.use_span.return_value.__enter__ = mock_span
mock_tracer.use_span.return_value.__exit__ = mock_span
with mock.patch("opentelemetry.trace.get_tracer") as tracer:
tracer.return_value = mock_tracer
jinja2.environment.Template("Hello {{name}}!")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -121,8 +121,6 @@ def test_not_recording(self):
mock_span = mock.Mock()
mock_span.is_recording.return_value = False
mock_tracer.start_span.return_value = mock_span
mock_tracer.use_span.return_value.__enter__ = mock_span
mock_tracer.use_span.return_value.__exit__ = True
Psycopg2Instrumentor().instrument()
with mock.patch("opentelemetry.trace.get_tracer") as tracer:
tracer.return_value = mock_tracer
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,8 +84,6 @@ def test_set_not_recording(self):
mock_span = mock.Mock()
mock_span.is_recording.return_value = False
mock_tracer.start_span.return_value = mock_span
mock_tracer.use_span.return_value.__enter__ = mock_span
mock_tracer.use_span.return_value.__exit__ = True
with mock.patch("opentelemetry.trace.get_tracer") as tracer:
tracer.return_value = mock_tracer
client = self.make_client([b"STORED\r\n"])
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,8 +76,6 @@ def test_not_recording(self):
mock_span = mock.Mock()
mock_span.is_recording.return_value = False
mock_tracer.start_span.return_value = mock_span
mock_tracer.use_span.return_value.__enter__ = mock_span
mock_tracer.use_span.return_value.__exit__ = True
mock_event = MockEvent({})
command_tracer = CommandTracer(mock_tracer)
command_tracer.started(event=mock_event)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,8 +83,8 @@ def _before_traversal(event):
for key, value in attributes.items():
span.set_attribute(key, value)

activation = tracer.use_span(span, end_on_exit=True)
activation.__enter__()
activation = trace.use_span(span, end_on_exit=True)
activation.__enter__() # pylint: disable=E1101
request_environ[_ENVIRON_ACTIVATION_KEY] = activation
request_environ[_ENVIRON_SPAN_KEY] = span
request_environ[_ENVIRON_TOKEN] = token
Expand All @@ -105,6 +105,7 @@ def disabled_tween(request):

# make a request tracing function
def trace_tween(request):
# pylint: disable=E1101
if _excluded_urls.url_disabled(request.url):
request.environ[_ENVIRON_ENABLED_KEY] = False
# short-circuit when we don't want to trace anything
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -104,8 +104,6 @@ def test_not_recording(self):
mock_span = Mock()
mock_span.is_recording.return_value = False
mock_tracer.start_span.return_value = mock_span
mock_tracer.use_span.return_value.__enter__ = mock_span
mock_tracer.use_span.return_value.__exit__ = True
with patch("opentelemetry.trace.get_tracer"):
self.client.get("/hello/123")
span_list = self.memory_exporter.get_finished_spans()
Expand Down
Loading

0 comments on commit 096d69d

Please sign in to comment.