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.

Sign up for GitHub

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

Jump to bottom

add optional parameter to record_exception method #1242

Merged
merged 9 commits into from
Nov 3, 2020
Merged

add optional parameter to record_exception method #1242

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
63f57b6
add optional parameter to record_exception method
shreyagupta30 Oct 14, 2020
45583d1
update sdk/trace/init.py and Changelog
shreyagupta30 Oct 14, 2020
845c257
fix imports error
shreyagupta30 Oct 14, 2020
8ddb574
fix imports error
shreyagupta30 Oct 14, 2020
e001ccb
fix paramter override in record_exception()
shreyagupta30 Oct 14, 2020
0262b64
Merge branch 'master' into record-exception
Oct 14, 2020
b8c5b85
Merge branch 'master' into record-exception
Oct 29, 2020
f4bbe2e
Merge branch 'master' into record-exception
lzchen Nov 2, 2020
ef33e45
Merge branch 'master' into record-exception
lzchen Nov 3, 2020
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
3 changes: 2 additions & 1 deletion opentelemetry-api/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@ Released 2020-11-02
## Version 0.14b0

Released 2020-10-13

- Add optional parameter to `record_exception` method
([#1242](https://github.com/open-telemetry/opentelemetry-python/pull/1242))
- Add support for `OTEL_PROPAGATORS`
([#1123](https://github.com/open-telemetry/opentelemetry-python/pull/1123))
- Store `int`s as `int`s in the global Configuration object
Expand Down
14 changes: 12 additions & 2 deletions opentelemetry-api/src/opentelemetry/trace/span.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,12 @@ def set_status(self, status: Status) -> None:
"""

@abc.abstractmethod
def record_exception(self, exception: Exception) -> None:
def record_exception(
self,
exception: Exception,
attributes: types.Attributes = None,
timestamp: typing.Optional[int] = None,
) -> None:
"""Records an exception as a span event."""

def __enter__(self) -> "Span":
Expand Down Expand Up @@ -263,7 +268,12 @@ def update_name(self, name: str) -> None:
def set_status(self, status: Status) -> None:
pass

def record_exception(self, exception: Exception) -> None:
def record_exception(
self,
exception: Exception,
attributes: types.Attributes = None,
timestamp: typing.Optional[int] = None,
) -> None:
pass


Expand Down
3 changes: 2 additions & 1 deletion opentelemetry-sdk/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,8 @@ Released 2020-11-02
## Version 0.14b0

Released 2020-10-13

- Add optional parameter to `record_exception` method
([#1242](https://github.com/open-telemetry/opentelemetry-python/pull/1242))
- Add timestamps to aggregators
([#1199](https://github.com/open-telemetry/opentelemetry-python/pull/1199))
- Add Global Error Handler
Expand Down
7 changes: 6 additions & 1 deletion opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -680,7 +680,12 @@ def __exit__(

super().__exit__(exc_type, exc_val, exc_tb)

def record_exception(self, exception: Exception) -> None:
def record_exception(
self,
exception: Exception,
attributes: types.Attributes = None,
timestamp: Optional[int] = None,
) -> None:
Copy link
Contributor

Choose a reason for hiding this comment

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

the additional parameters here (attributes, timestamp) should be used in the method below. As per the spec:

If attributes with the same name would be generated by the method already, the additional attributes take precedence

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@codeboten Can you please guide me on how to do this. I am not able to figure out what exactly needs to be done here.

Copy link
Contributor

Choose a reason for hiding this comment

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

@shreyagupta30 you'll want to pass in the timestamp argument into add_event, you can see what it looks like by looking at the method signature here:

opentelemetry-python/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py

Lines 589 to 593 in 937863f

def add_event(
self,
name: str,
attributes: types.Attributes = None,
timestamp: Optional[int] = None,

You'll also want to pass the attributes parameter in the add_event call below, ensuring that any attributes passed into this method overrides any of the default attributes, the way i'd approach it is probably to create a dict with the default attributes seen below, then loop through the attributes arg and set any values in the dict that way.

opentelemetry-python/opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py

Lines 697 to 701 in 937863f

attributes={
"exception.type": exception.__class__.__name__,
"exception.message": str(exception),
"exception.stacktrace": stacktrace,
},

Last but not least, adding a test like the one below to ensure the attributes are set as expected

opentelemetry-python/opentelemetry-sdk/tests/trace/test_trace.py

Lines 855 to 872 in 937863f

def test_record_exception(self):
span = trace._Span("name", mock.Mock(spec=trace_api.SpanContext))
try:
raise ValueError("invalid")
except ValueError as err:
span.record_exception(err)
exception_event = span.events[0]
self.assertEqual("exception", exception_event.name)
self.assertEqual(
"invalid", exception_event.attributes["exception.message"]
)
self.assertEqual(
"ValueError", exception_event.attributes["exception.type"]
)
self.assertIn(
"ValueError: invalid",
exception_event.attributes["exception.stacktrace"],
)

"""Records an exception as a span event."""
try:
stacktrace = traceback.format_exc()
Expand Down