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

Recommend Exception.StackTrace for C# #1939

Closed
wants to merge 1 commit into from

Conversation

bgrainger
Copy link

Changes

The StackTrace property "gets a string representation of the immediate frames on the call stack." It's a better way to get the stack trace than calling .ToString(), which will include the exception type and message (which should already be in the exception.type and exception.message attributes. It's supported on all .NET Frameworks and .NET Standard versions, so no reason not to use it.

The StackTrace property 'gets a string representation of the immediate frames on the call stack.'
@bgrainger bgrainger requested review from a team September 19, 2021 20:25
@linux-foundation-easycla

This comment has been minimized.

Copy link
Member

@Oberon00 Oberon00 left a comment

Choose a reason for hiding this comment

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

I think you are running into a common misunderstanding here: exception.stacktrace should not only contain the frames on the stack, but everything that is usually included when an "exception stacktrace" is printed in the language, which is exactly what ToString does. I fought to have the name changed to something like full_details instead of stacktrace (#697 (comment)), but stacktrace is also a common, though ambigous name for the concept of "human-readable exception details".

With this change, we would lose important details, like the cause of the exception (inner exception). Also, the stacktraces for AggregateExceptions are usually quite useless, while the full ToString is more helpful.

@Oberon00 Oberon00 added area:semantic-conventions Related to semantic conventions spec:trace Related to the specification/trace directory labels Sep 20, 2021
@jsuereth
Copy link
Contributor

I think there's a related question around logging and stacktrace here.

FWIW - Google Cloud Trace has the stack frame as well: https://cloud.google.com/trace/docs/reference/v2/rpc/google.devtools.cloudtrace.v2?hl=it#google.devtools.cloudtrace.v2.StackTrace

While @Oberon00 mentions an important use case (reading the error and following across causes), there's also the use case of looking at stack-traces for common "failure lines" and reporting this in the backend. Sending raw strings vs. structured data can make this hard as you need to re-parse the data per-language.

I'm wondering if maybe we should have a best of both worlds? e.g. reporting a structured stack trace and allowing "caused by" chains in the stack trace (or some kind of better error string)

@Oberon00
Copy link
Member

Sending raw strings vs. structured data can make this hard as you need to re-parse the data per-language.

I'm wondering if maybe we should have a best of both worlds? e.g. reporting a structured stack trace and allowing "caused by" chains in the stack trace (or some kind of better error string)

I agree with that and I think it was also consensus back then that we would want a structured form of exception reporting in the future. In particular, see #427 (comment). But this would be an entirely different issue, and a separate (set of) attribute(s). Related #955.

@bgrainger
Copy link
Author

@Oberon00 Thanks for the link to that comment thread. I think the attribute name and description (of exception.stacktrace) in this document are incredibly misleading (but I don't need to tell you that). Comparing to the recommendations for other languages, it seems that .ToString() is the desired data so I'll close this.

@bgrainger bgrainger closed this Sep 20, 2021
@Oberon00 Oberon00 added the area:error-reporting Related to error reporting label Sep 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:error-reporting Related to error reporting area:semantic-conventions Related to semantic conventions spec:trace Related to the specification/trace directory
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants