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 code.stacktrace attribute #435

Merged
merged 5 commits into from
Nov 9, 2023

Conversation

AlexanderWert
Copy link
Member

Changes

There are use cases when it's desirable to collect a stacktrace that is not necessarily attached to an exception (e.g. to identify the exact stack of a span).

This PR proposes a new attribute code.stacktrace for the above-mentioned use cases.

Merge requirement checklist

@AlexanderWert AlexanderWert requested review from a team October 23, 2023 08:20
Copy link
Member

@trask trask left a comment

Choose a reason for hiding this comment

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

Would we deprecate exception.stacktrace in favor of this generalization?

@AlexanderWert
Copy link
Member Author

Would we deprecate exception.stacktrace in favor of this generalization?

@trask I wasn't intending to imply changes to exception.stacktrace with that, but I'm open to it if we want to. code.stacktrace would cover other non-exception-related use cases. Also as you can see in the example value in this case the exception message is not present here.

But, you are right that code.stacktrace is a more general attribute that theoretically could be reused / referenced in the exception semantic conventions.

@pyohannes
Copy link
Contributor

Would we deprecate exception.stacktrace in favor of this generalization?

Re-using a generic stacktrace field might be another option (hypothetical until there's progress on #339).

@trask
Copy link
Member

trask commented Oct 24, 2023

Also as you can see in the example value in this case the exception message is not present here.

👍

Copy link
Member

@jack-berg jack-berg left a comment

Choose a reason for hiding this comment

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

I have also had use cases for including a stack trace outside of an exception and came to the same conclusion that the attribute should be named code.stacktrace.

Thanks for beating me too it 😁!

docs/general/attributes.md Outdated Show resolved Hide resolved
Signed-off-by: Alexander Wert <alexander.wert@elastic.co>
@Oberon00
Copy link
Member

Oberon00 commented Oct 29, 2023

I didn't read everything, but please note that exception.stacktrace is not a plain stacktrace but the default detailed textual exception representation (including, usually, exception type, message, caused-by, etc). There was a long discussion about the name (IIRC I was strongly against exceptions.stacktrace since in many languages there is also the possibility to get a textual representation of just the exception stacktrace), but there is no other common name for that concept.

@AlexanderWert
Copy link
Member Author

I didn't read everything, but please note that exception.stacktrace is not a plain stacktrace but the default detailed textual exception representation (including, usually, exception type, message, caused-by, etc). There was a long discussion about the name (IIRC I was strongly against exceptions.stacktrace since in many languages there is also the possibility to get a textual representation of just the exception stacktrace), but there is no other common name for that concept.

Thanks @Oberon00 , that's also how I understood exception.stacktrace. For code.stacktrace the intention is to capture the textual representation of just the exception stacktrace. What do you think in general about the proposal in this PR?

@trask trask changed the title Add code.stacktrace atttribute Add code.stacktrace attribute Nov 8, 2023
@trask
Copy link
Member

trask commented Nov 8, 2023

should this have a change log entry?

Signed-off-by: Alexander Wert <alexander.wert@elastic.co>
@AlexanderWert AlexanderWert merged commit e3da931 into open-telemetry:main Nov 9, 2023
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

7 participants