Skip to content
This repository has been archived by the owner on May 23, 2023. It is now read-only.

Commit

Permalink
Fix __exit__ method of Scope class (#120)
Browse files Browse the repository at this point in the history
Technically, a scope could contain None as a span,
but on exit it calls _on_error on a span with no check.

Currently, this issue affects opentracing_instrumentation library:
https://travis-ci.org/uber-common/opentracing-python-instrumentation/jobs/525712144#L530-L616

These changes:

 - Make the _on_error method static

 - Add a check for a span to the _on_error method
  • Loading branch information
Jamim authored and yurishkuro committed May 2, 2019
1 parent 946f763 commit 35a7bc9
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 7 deletions.
6 changes: 4 additions & 2 deletions opentracing/scope.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# Copyright (c) 2017 The OpenTracing Authors.
# Copyright (c) 2017-2019 The OpenTracing Authors.
#
# Permission is hereby granted, free of charge, to any person obtaining a copy
# of this software and associated documentation files (the "Software"), to deal
Expand All @@ -20,6 +20,8 @@

from __future__ import absolute_import

from .span import Span


class Scope(object):
"""A scope formalizes the activation and deactivation of a :class:`Span`,
Expand Down Expand Up @@ -78,5 +80,5 @@ def __exit__(self, exc_type, exc_val, exc_tb):
and added as a tag to the :class:`Span`.
:attr:`~operation.ext.tags.ERROR` will also be set to `True`.
"""
self.span._on_error(exc_type, exc_val, exc_tb)
Span._on_error(self.span, exc_type, exc_val, exc_tb)
self.close()
11 changes: 6 additions & 5 deletions opentracing/span.py
Original file line number Diff line number Diff line change
Expand Up @@ -215,15 +215,16 @@ def __exit__(self, exc_type, exc_val, exc_tb):
and added as a tag. :attr:`~operation.ext.tags.ERROR` will also be set
to `True`.
"""
self._on_error(exc_type, exc_val, exc_tb)
Span._on_error(self, exc_type, exc_val, exc_tb)
self.finish()

def _on_error(self, exc_type, exc_val, exc_tb):
if not exc_val:
@staticmethod
def _on_error(span, exc_type, exc_val, exc_tb):
if not span or not exc_val:
return

self.set_tag(tags.ERROR, True)
self.log_kv({
span.set_tag(tags.ERROR, True)
span.log_kv({
logs.EVENT: tags.ERROR,
logs.MESSAGE: str(exc_val),
logs.ERROR_OBJECT: exc_val,
Expand Down
9 changes: 9 additions & 0 deletions tests/test_scope.py
Original file line number Diff line number Diff line change
Expand Up @@ -74,3 +74,12 @@ def test_scope_error_report():
ValueError)
assert isinstance(log_kv_args.get(logs.STACK, None),
types.TracebackType)


def test_scope_exit_with_no_span():
# ensure `Scope.__exit__` doesn't fail with `AttributeError`
try:
with Scope(None, None):
raise ValueError
except ValueError:
pass

0 comments on commit 35a7bc9

Please sign in to comment.