Add Exception to LogRecordData

[juliuskoval]

Towards #4433

Changes

Added Exception property to LogRecordData struct.

Merge requirement checklist

• CONTRIBUTING guidelines followed (license requirements, nullable enabled, static analysis, etc.)
• Unit tests added/updated
• Appropriate CHANGELOG.md files updated for non-trivial changes
• Changes in public API reviewed (if applicable)

[juliuskoval]

I made LogRecordAttributeList.RecordException obsolete instead of removing it outright, because I am using the method here. My worry is that if the method was removed, I could update my package, but then if somebody upgraded OpenTelemetry.Api without upgrading my package, they would get runtime errors. I understand that there can be breaking changes to the logs bridge API since it is still experimental, but I would appreciate it if you could wait before removing the method for a version or two, since this could affect users for whom the logs bridge API is just an unimportant implementation detail given that the API is meant to be used by library authors.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 88.50%. Comparing base (14ef30b) to head (4d2c13c).
⚠️ Report is 27 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6980      +/-   ##
==========================================
- Coverage   88.65%   88.50%   -0.15%     
==========================================
  Files         263      263              
  Lines       12397    12399       +2     
==========================================
- Hits        10990    10974      -16     
- Misses       1407     1425      +18     

Flag	Coverage Δ
unittests-Project-Experimental	88.21% <100.00%> (-0.37%)	⬇️
unittests-Project-Stable	88.39% <100.00%> (-0.06%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
...c/OpenTelemetry.Api/Logs/LogRecordAttributeList.cs	70.79% <ø> (ø)
src/OpenTelemetry.Api/Logs/LogRecordData.cs	100.00% <100.00%> (ø)
src/OpenTelemetry/Logs/LoggerSdk.cs	100.00% <100.00%> (ø)

... and 5 files with indirect coverage changes

[martincostello]

Nit: sorting.

[martincostello]

If code used both, would they get duplicated attributes?

[martincostello]

Here, and in the existing properties, the need to set to null seems redundant as the properties are already nullable.

[rajkumar-rangaraj]

Questioning whether this change is needed at this time.

The Logs Bridge API spec (https://opentelemetry.io/docs/specs/otel/logs/bridge-api/#emit-a-logrecord) says the API
MAY accept an Exception parameter, it's optional, not required. We already support exception logging via
LogRecordAttributeList.RecordException, which correctly emits the exception.type, exception.message, and
exception.stacktrace semantic convention attributes.

Adding a second path (LogRecordData.Exception) introduces a few concerns:

1. Duplication risk: As @martincostello noted
   (Add Exception to LogRecordData #6980 (comment)), if a caller sets
   data.Exception and calls attributes.RecordException() during migration, the OTLP serializer will emit duplicate
   exception attributes - it appends from logRecord.Exception first, then iterates the attribute list without
   deduplication.
2. Two ways to do the same thing: Having both RecordException (obsoleted but functional) and
   LogRecordData.Exception simultaneously increases API surface confusion, especially for bridge library authors who
   need to pick one and understand the interaction.
3. No deduplication guard: If we do want to support this, we should at minimum add logic to suppress
   RecordException-generated attributes when LogRecordData.Exception is present (or vice versa) before shipping.

My recommendation is to avoid this change and rely on RecordException

[juliuskoval]

I don't disagree, but the existence of LogRecordAttributeList.RecordException was one of the complaints made here by @Kielek regarding non-compliance with the spec. #4433 (comment)
So that was my motivation, but I understand the point about duplication.

[rajkumar-rangaraj]

@Kielek, as you may be aware, OpenTelemetry .NET has a few areas that slightly deviate from the current spec because the stable release happened before the spec was fully stabilized. This API is one such case.

The current release is already well adopted by customers, and introducing a second API for the same purpose at this point would create unnecessary confusion for users and bridge/library authors.

If we want to fully address this, I believe the right solution would be to replace the existing API rather than add another path that does the same thing. However, that would require a major version bump, and I do not think we are in a position to do that right now.

My recommendation is to keep the current API as-is. Thoughts?

[Kielek]

If we have good reason (major bump needed) I think that we can make some differences from the otel specification, but we for sure have documentation written for all such cases. It will be great to have also issues to track such stuff. Maybe there will be good moment to release it (completely not related, but we could revisit support for netstandard2.0/2.1)?

In such cases, it will be easier to go through existing issues instead of making full assessment from scratch.

---

I would like to check this with @pellared who was working on Golang Logs stabilization. He is on Kubecon this week, so it needs to wait couple more days.

[rajkumar-rangaraj]

@Kielek, I agree that if we keep intentional deviations from the spec, we should document and track them clearly.

That said, I also want to call out that LogRecordData.Exception is not a must-have from a spec compliance perspective. The Logs Bridge spec allows accepting an exception, but it does not require this exact API shape. We already have an existing supported path via RecordException, and that path is already adopted by customers.

That is why I am hesitant to add a second API for the same purpose in the current release line. It increases API surface and creates potential customer confusion without closing a mandatory compliance gap.

Happy to wait for your discussion with @pellared as well.

[github-actions]

This PR was marked stale due to lack of activity and will be closed in 7 days. Commenting or pushing will instruct the bot to automatically remove the label. This bot runs once per day.