-
Notifications
You must be signed in to change notification settings - Fork 637
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
Replaced Tracer.use_span() with opentelemetry.trace.use_span() #364
Changes from all commits
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 |
---|---|---|
|
@@ -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 | ||
) | ||
|
@@ -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") | ||
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. Instrumentation was duplicating start_as_current_span's exception handling and not formatting the description properly. After removing the duplicate code, had to update the assertion to expect description formatted by the context manager. Similar changes below. |
||
|
||
def test_executemany(self): | ||
db_integration = AiopgIntegration(self.tracer, "testcomponent") | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 | ||
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. pylint has false positives when using a generator as context manager so ignoring rule. |
||
utils.attach_span(task, task_id, (span, activation)) | ||
|
||
@staticmethod | ||
|
@@ -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") | ||
|
@@ -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 | ||
|
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.
The context manager above (
start_as_current_span
) handles exceptions and records them as status already so this was completely unnecessary/redundant. Some similar changes below.