opentelemetry-dotnet
opentelemetry-dotnet copied to clipboard
open-telemetry
Record `Exception.Data` when calling `RecordException`
Feature Request
Has there been an consideration of recording values from an exception's Data property as tags on the event when calling activity?.RecordException(ex)?
Describe the solution you'd like:
I would like to add key/value data to certain exceptions beyond just a message. It would be useful to record this extra data when the exceptions are caught.
Describe alternatives you've considered.
The extra data can obviously be recorded manually, but it seems natural to include this functionality in the RecordException method.
The RecordException will be following the spec here : https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/semantic_conventions/exceptions.md (its currently experimental, and will be subject to breaking changes). We can revisit this once the spec gets to stable.
I think this should be reopened. Data attributes are required to be supported by RecordException according to the spec.
Here is the specification for RecordException: https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/api.md#record-exception (it is directly linked from the spec in https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/semantic_conventions/exceptions.md)
It says:
If RecordException is provided, the method MUST accept an optional parameter to provide any additional event attributes (this SHOULD be done in the same way as for the AddEvent method).
Given that it is a MUST, it seems that currently RecordException does not follow the spec by not having this parameter.
Given that it is a MUST, it seems that currently RecordException does not follow the spec by not having this parameter.
@pbiggar Thanks for raising this. I've opened a separate issue #3369. This issue is slightly different in that it is proposing a change in behavior to the existing RecordExpception method to have it record information from the Exception.Data property.
It would be useful if the entire exception object was available somewhere, so that it could be retrieved in a processor. Reducing an exception to its type, message, and stacktrace string makes it impossible to recreate the exception or get at values from other properties. For example ArgumentOutOfRangeException has an ActualValue property. Such values wouldn't be available from the Data dictionary.
Additionally, the stack trace string captured in exception.stacktrace is insufficient to create an actual System.Diagnostics.StackTrace instance, if one wants to do anything more advanced with the frames, their IL offsets, etc. One needs the original exception - as in new StackTrace(ex).
LogRecord do provide access to Exception (https://github.com/open-telemetry/opentelemetry-dotnet/blob/main/src/OpenTelemetry/Logs/LogRecord.cs#L262), but Traces do not. There is an open issue about supporing this in runtime repo, will find it and link from here.
There is an open issue about supporing this in runtime repo, will find it and link from here.
@cijothomas I'd be interested in that issue, if you're still able to track it down.
I'm wondering if I could maybe make a PR with @mattjohnsonpint's suggestion above.
Thanks @cijothomas. Do you think there's any possibility of working around this in the OpenTelemetry API until that issue in the dotnet runtime gets resolved? That issue has been open for a couple of years now.
There are a couple of potential workarounds that I could think of, but I'm not sure how these sit with the general design philosophy of the OpenTelemetry APIs:
- RecordException currently stores a couple of different properties of the Exception in tags. Is there any reason not to put a reference to the Exception itself in a tag at the same time?
- When initialising OpenTelemetry, you could give SDK users the option to configure an
OnRecordException(Activity activity, Exception ex, in TagList tags)callback that gets invoked each time RecordException is called... that would give folks the opportunity to extend the RecordException method to do whatever they wanted (e.g. storing a reference to the Exception in a Tag, if they wanted) - An
Activity.GetExceptions()method that returned any Exceptions recorded for the Activity
Do any of those sound feasible?
@jamescrosswell The RecordException is following the OpenTelemetry specification on how to store Exceptions.
Its best to improve Activity class itself to have first class support for Exception (as opposed to storing in tags), which is what the runtime issue is tracking.
(As a workaround, could you see if you can use ILogger to report exceptions, assuming you control the code which currently calls RecordException?. The LogRecord keeps Exception instance directly, allowing more flexibility at the processor/exporter level.
As a workaround, could you see if you can use ILogger to report exceptions, assuming you control the code which currently calls RecordException?
Unfortunately not @cijothomas. I'm working on the Sentry OpenTelemetry Tracing integration. Sentry SDK users could be using all kinds of different solutions for logging.
This issue goes beyond the OpenTelemetry .NET API though right? Even if the Exception was available on the Activity class in the dotnet runtime, that wouldn't solve the problem for other OpenTelemetry APIs (Python, Go etc.).
What would have to happen for the OpenTelemetry specification to change?
What would have to happen for the OpenTelemetry specification to change?
Need to open an issue in the opentelemetry-specification repo describing the issue and the suggested improvement.
I found an existing issue in the opentelemetry-specification repo.
It looks like for Java, something like this was implemented already?
- https://github.com/open-telemetry/opentelemetry-java/pull/4162