Appender-Tracing - extend spl treatment of message when recording str

[cijothomas]

TODO: Tests in next PR, if this is merged.

Follow up from https://github.com/open-telemetry/opentelemetry-rust/pull/2001/files

[cijothomas]

@mladedav Could you also review this?

[codecov]

Codecov Report

Attention: Patch coverage is 83.33333% with 1 line in your changes missing coverage. Please review.

Project coverage is 79.3%. Comparing base (5ce50ae) to head (da6e16b).
Report is 2 commits behind head on main.

Files with missing lines	Patch %	Lines
opentelemetry-appender-tracing/src/layer.rs	83.3%	1 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##            main   #2689     +/-   ##
=======================================
- Coverage   79.3%   79.3%   -0.1%     
=======================================
  Files        123     123             
  Lines      22602   22601      -1     
=======================================
- Hits       17927   17925      -2     
- Misses      4675    4676      +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[lalitb]

This would be a breaking change if user is expecting the message to go an attribute?

[lalitb]

And what should be the expected behaviour if both message attribute and default body field is provided

[cijothomas]

This would be a breaking change if user is expecting the message to go an attribute?

There was no contract documented about this, so while this change behavior, I don't think it should be a major issue. (We were already special casing "message" for the record_debug, so missing this may also be considered a bug)

[cijothomas]

And what should be the expected behavior if both message attribute and default body field is provided

We can add tests to make sure this is covered, but the behavior is really controlled by tracing itself. We can make some decision and document it in the event of conflict, as there can only be one body.

[cijothomas]

https://github.com/open-telemetry/opentelemetry-rust/blob/main/opentelemetry-appender-tracing/src/layer.rs#L121 Fixing these TODOs are also going to change the behavior, so is fixing error to be reported specially. It's questionable to do that post 1.0 stable (but maybe okay), but I don't see an issue doing it at this stage. Let me know your thoughts.

[lalitb]

We were already special casing "message" for the record_debug, so missing this may also be considered a bug

Yes, I didn't realize that. With special casing, user can't have both explicit and implicit string message attributes, as they would be overwritten. But this PR doesn't add any new bug :)

[cijothomas]

We need to document the final behavior before 1.0.
#2150 will also be related to this...

[mladedav]

Sorry for the late response but yeah, it seems fine.

[cijothomas]

Sorry for the late response but yeah, it seems fine.

Thanks!