[Exporter.Geneva] Allow to export only stack trace in logs

[prezaei]

Component

OpenTelemetry.Exporter.Geneva

Package Version

Package Name	Version
OpenTelemetry.Api	1.10.0
OpenTelemetry	1.10.0

Runtime Version

net9.0

Description

I am looking at the Geneva Exporter and there seems to be bug:

Exception.ToString() is not the stack trace.... it has a ton more than that...
Can we please fix and limit to the stack trace?

Please consider using Exception.StackTrace here which is also cheaper to generate.

Steps to Reproduce

Log and exception and you see it is not the stack trace that gets exported.

Expected Result

Just the stack trace to be exported for this property.

Actual Result

Full Exception Message.

Additional Context

No response

[github-actions]

Tagging component owner(s).

@cijothomas @CodeBlanch @rajkumar-rangaraj

[prezaei]

@utpilla as FYI. The code is here: https://github.com/open-telemetry/opentelemetry-dotnet-contrib/blame/1ca05685cbad63d3fa813b9cab49be341048e69e/src/OpenTelemetry.Exporter.Geneva/Internal/Tld/TldLogExporter.cs#L259

[cijothomas]

This is not a bug, it is intentional to use ToString() here. OTel SDK upstream uses ToString() for representing exceptions, so if use OTLP Exporter, that is what you'd get.
It is entirely possible for an opt-in feature flag in GenevaExporter to let users control what representation of Exception is preferred.

I don't recall full details, but I don't think StackTraces takes care of inner_exceptions. Also ToString() takes care of avoiding the async state machine stuff to get cleaner representation. But I can totally see some users preferring StackTrace.

[cijothomas]

https://github.com/open-telemetry/opentelemetry-dotnet-contrib/blame/1ca05685cbad63d3fa813b9cab49be341048e69e/src/OpenTelemetry.Exporter.Geneva/Internal/Tld/TldLogExporter.cs#L259

The same is the case when Msgpack encoding is used.

[prezaei]

https://github.com/open-telemetry/opentelemetry-dotnet-contrib/blame/1ca05685cbad63d3fa813b9cab49be341048e69e/src/OpenTelemetry.Exporter.Geneva/Internal/Tld/TldLogExporter.cs#L259

The same is the case when Msgpack encoding is used.

Why do you think this is intentional? This is called Exception Stack and it is the ToString on Exception.

There is a separate property for exception message here.

IMHO, this should follow the spec and just be the stack trace and not a repeat of the message + stack trace.

[cijothomas]

Why do you think this is intentional? This is called Exception Stack and it is the ToString on Exception.

See the previous discussions on this topic in OTel SDK itself: open-telemetry/opentelemetry-dotnet#1569
"StackTrace" name is what confuses many! The spec wording is "A stacktrace as a string in the natural representation for the language runtime. The representation is to be determined and documented by each language SIG."

OTel .NET SDK has chosen ex.ToString() as the representation in open-telemetry/opentelemetry-dotnet#5143 (To be precise, it was done in 1.0 itself, but this is where it was documented)

IMHO, this should follow the spec and just be the stack trace and not a repeat of the message + stack trace.

As shared above, this is already following the spec. Totally open to adding an option to let users decide the representation suitable for them. It should be done upstream in OTel SDK rather than GenevaExporter alone, otherwise users using OTLP Exporter will not be able to take advantage of this flexibility.

[prezaei]

Why do you think this is intentional? This is called Exception Stack and it is the ToString on Exception.

See the previous discussions on this topic in OTel SDK itself: open-telemetry/opentelemetry-dotnet#1569

"StackTrace" name is what confuses many! The spec wording is "A stacktrace as a string in the natural representation for the language runtime. The representation is to be determined and documented by each language SIG."

OTel .NET SDK has chosen ex.ToString() as the representation in open-telemetry/opentelemetry-dotnet#5143 (To be precise, it was done in 1.0 itself, but this is where it was documented)

IMHO, this should follow the spec and just be the stack trace and not a repeat of the message + stack trace.

As shared above, this is already following the spec. Totally open to adding an option to let users decide the representation suitable for them. It should be done upstream in OTel SDK rather than GenevaExporter alone, otherwise users using OTLP Exporter will not be able to take advantage of this flexibility.

Yes, let's please add an option for StackTrace to actually mean stacktrace... and not some other random string that differs from exception to exception. Thanks

[cijothomas]

@prezaei I and @rajkumar-rangaraj can help with adding an opt-in feature to GenevaExporter to use Exception.StackTrace instead of Exception.ToString().

However, be aware that this will just work for GenevaExporter and will be inconsistent with other systems. I am aware of the below three other places that made the decision to use ToString().

1. If one uses OTLP Exporter, it'll use ToString() only and changing this requires upstream change in OTel SDK itself.

2. ToString() for Exception is also baked into .NET Runtime itself for Traces . .NET Exposes API to override the tag values with custom implementation, but not exposed via OTel. (users can still do it)
   Note that this was a conscious choice in .NET Runtime - the original proposal shows StackTrace, but the final approved one uses ToString().

3. Default LoggerProviders like ConsoleProvider from .NET runtime library also uses ToString().
   (They don't call it stacktrace anyway, so no confusion like what we have for Otel!)

[prezaei]

@prezaei I and @rajkumar-rangaraj can help with adding an opt-in feature to GenevaExporter to use Exception.StackTrace instead of Exception.ToString().

However, be aware that this will just work for GenevaExporter and will be inconsistent with other systems. I am aware of the below three other places that made the decision to use ToString().

1. If one uses OTLP Exporter, it'll use ToString() only and changing this requires upstream change in OTel SDK itself.
2. ToString() for Exception is also baked into .NET Runtime itself for Traces . .NET Exposes API to override the tag values with custom implementation, but not exposed via OTel. (users can still do it)
   Note that this was a conscious choice in .NET Runtime - the original proposal shows StackTrace, but the final approved one uses ToString().
3. Default LoggerProviders like ConsoleProvider from .NET runtime library also uses ToString().
   (They don't call it stacktrace anyway, so no confusion like what we have for Otel!)

Yes please. Let's add the config to the Geneva one please.
As for ToString use in traces, that makes perfect sense as they are not structured or meant to be structured. They are also not called Stack. Whereas here, we have added a new property called env_exp_stack which clearly indicated it is a stack trace. This is even from the docs:

[cijothomas]

As for ToString use in traces, that makes perfect sense as they are not structured or meant to be structured.

Why? Exception conventions for Logs and Spans (Traces) are defined in the exact same way: https://github.com/open-telemetry/semantic-conventions/tree/main/docs/exceptions
If it makes sense for Traces, then it makes sense for Logs too.

#2422 - I made a quick PR to add StackTrace support. I'll wait for code owners @CodeBlanch and @rajkumar-rangaraj to review. It does add a new public api, but seems very safe to me to ship it as stable.

[cijothomas]

@CodeBlanch Could you mark this as non-bug, and feature request?