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.

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

add optional parameter to record_exception method #1242

Merged
merged 9 commits into from
Nov 3, 2020
Prev Previous commit
Next Next commit
fix imports error
shreyagupta30 committed Oct 14, 2020

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
commit 8ddb5749338019a36e1f08a57cd0f2f0db19a672
2 changes: 1 addition & 1 deletion opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py
Original file line number Diff line number Diff line change
@@ -661,7 +661,7 @@ def __exit__(
def record_exception(
self,
exception: Exception,
attributes: util.types.Attributes = None,
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:

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.

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

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."""