Treat log's key-values as attributes in log bridge

[KodrAus]

Changes

This PR supports log's newly stabilized structured logging API in the opentelemetry-appender-log crate.

As of the 0.4.21 release, log has stable support for structured logging. Its model is to decouple the serialization framework choice from producers and consumers, so I've opted to use serde here to convert log's structured values into AnyValues. The infrastructure isn't specific to log, so if it either already exists somewhere in the SDK that I can use, or would be worth moving into the SDK under some serde feature I'd be happy to do that too.

One open question I've got is whether I need to deal with deduplication when constructing the attributes list, or whether that's dealt with elsewhere.

Merge requirement checklist

• CONTRIBUTING guidelines followed
• Unit tests added/updated (if applicable)
• Appropriate CHANGELOG.md files updated for non-trivial, user-facing changes
• Changes in public API reviewed (if applicable)

[codecov]

Codecov Report

Attention: Patch coverage is 90.57592% with 54 lines in your changes are missing coverage. Please review.

Project coverage is 70.1%. Comparing base (f203b03) to head (7de7f09).
Report is 21 commits behind head on main.

❗ Current head 7de7f09 differs from pull request most recent head 7cfca6d. Consider uploading reports for the commit 7cfca6d to get more accurate results

Files	Patch %	Lines
opentelemetry-appender-log/src/lib.rs	90.5%	54 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##            main   #1628     +/-   ##
=======================================
+ Coverage   69.3%   70.1%   +0.7%     
=======================================
  Files        136     136             
  Lines      19637   20024    +387     
=======================================
+ Hits       13610   14037    +427     
+ Misses      6027    5987     -40     

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

[cijothomas]

One open question I've got is whether I need to deal with deduplication when constructing the attributes list, or whether that's dealt with elsewhere.

As of today, its not done anywhere in the API, SDK, Exporter. There is a good chance of a feature flag/option to make it happen. (unsure where,, could be just in otlp exporter only)

[cijothomas]

I've opted to use serde here to convert log's structured values into AnyValues.

mm.. This may require some discussion.. The log-bridge only depends on opentelemetry (which is otel's API crate) and it does not (and should not) deal with concerns such as serialization etc. That is left for the exporters (which are based off of the opentelemetry-sdk crate).

[KodrAus]

The serialization via serde is really just an internal conversion from log's value type into AnyValues. log doesn't define its own complete data-model, instead deferring to a serialization framework like serde to provide one for it. From there, an exporter will receive AnyValues which it can choose to serialize however it wants.

If we didn't want to force a dependency on serde here (which is perfectly reasonable) we can still convert primitive types coming from log into AnyValues via its own simplified visitor API, but won't be able to convert complex types like arrays or objects without needing to format them as strings. I could make the serde dependency optional for anyone that wants to opt-in to complete support.

[lalitb]

If we didn't want to force a dependency on serde here (which is perfectly reasonable) we can still convert primitive types coming from log into AnyValues via its own simplified visitor API, but won't be able to convert complex types like arrays or objects without needing to format them as strings. I could make the serde dependency optional for anyone that wants to opt-in to complete support.

+1 on that. As of now we only support primitive types with tokio-tracing appender, so should be good to have similar support with log-appender, and then using serde optionally for complete support.

[KodrAus]

Ok, I can make that change 👍 Any preference on what that feature should be called? log_attributes_full? The tracing appender may end up with an equivalent feature once they finish integrating valuable, unless they make that dependency required in tracing.

[TommyCpp]

In the other create(opentelemetry-proto) we have the feature with_serde to annotate proto generated files with serde attributes. Maybe we can borrow it here

[KodrAus]

Ok, I've made the serde dependency optional, gated by a with_serde feature. This test case gives you a reasonable idea of how values are converted with and without the feature enabled.

[KodrAus]

I can improve the test coverage here once we’re happy with the overall shape of things. Is there anywhere I’d need to plug this new feature into CI?

[lalitb]

changes look good. Some minor comments

[hdost]

Some relevant comments here open-telemetry/opentelemetry-specification#3938

[KodrAus]

Thanks for the review @lalitb. I’ll make those changes, fill out the test coverage, and leave some docs on how to enable key-value support, and how various kinds of values are represented.

[KodrAus]

This should be ready for another review now. I've added some crate-level docs to opentelemetry-appender-log. I tried to follow any conventions I saw in other docs, but please point out anywhere I should be doing things differently.

[TommyCpp]

LGTM. Thanks for the change!

[KodrAus]

The build failure here looks unrelated; is kicking off a fresh build likely to resolve it?

[cijothomas]

💯 love this doc!

[cijothomas]

Thanks! Could you add a changelog.md file too?