Unify builders across signals

[stormshield-fabs]

trafficstars

Fixes #1527 #2164

Changes

The consensus reached during the SIG meeting is that we'd like to unify the builder APIs across signals, limit duplication and keep our traits object-safe. Proposed changed :

• remove the Logger and Tracer builders that were only used to wrap the InstrumentationLibrary builder
• remove the deprecated versioned_ methods on providers
• reduce the public API to Provider::signal(name: &str) and Provider::library_signal(library: Arc<InstrumentationLibrary>)

Merge requirement checklist

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

[codecov[bot]]

Codecov Report

Attention: Patch coverage is 76.11111% with 43 lines in your changes missing coverage. Please review.

Project coverage is 79.6%. Comparing base (5628f66) to head (af8562b). Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
opentelemetry-zipkin/src/exporter/mod.rs	0.0%	10 Missing :warning:
opentelemetry-stdout/src/trace/exporter.rs	0.0%	4 Missing :warning:
opentelemetry-sdk/src/logs/log_emitter.rs	84.2%	3 Missing :warning:
opentelemetry/src/global/trace.rs	57.1%	3 Missing :warning:
opentelemetry-proto/src/transform/trace.rs	66.6%	2 Missing :warning:
opentelemetry-sdk/src/logs/log_processor.rs	88.8%	2 Missing :warning:
opentelemetry-sdk/src/metrics/instrument.rs	0.0%	2 Missing :warning:
opentelemetry-sdk/src/metrics/meter_provider.rs	93.5%	2 Missing :warning:
opentelemetry-sdk/src/trace/tracer.rs	66.6%	2 Missing :warning:
opentelemetry/src/global/metrics.rs	0.0%	2 Missing :warning:
... and 9 more

Additional details and impacted files

@@          Coverage Diff           @@
##            main   #2220    +/-   ##
======================================
  Coverage   79.5%   79.6%            
======================================
  Files        121     121            
  Lines      21116   20918   -198     
======================================
- Hits       16805   16666   -139     
+ Misses      4311    4252    -59     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[stormshield-fabs]

There were some concerns about having Arc in the library_signal APIs, but someone also mentioned we want to avoid unnecessary copies. We could hide the Arc inside the InstrumentationLibrary if needed.

[cijothomas]

There were some concerns about having Arc in the library_signal APIs, but someone also mentioned we want to avoid unnecessary copies. We could hide the Arc inside the InstrumentationLibrary if needed.

yes I suggest to hide Arc inside.

[cijothomas]

@stormshield-fabs Thanks a lot! I think the following are to be addressed before merge. We also need to write a migration guide in the PR description showing before/after, so users can easily (relatively) react to this.

Lets keep InstrumentationScope simple, and accept the perf cost now, given it is infrequent operation. https://github.com/open-telemetry/opentelemetry-rust/pull/2220#discussion_r1809335125

Rename library-signal to something like signal_with_scope/better names. https://github.com/open-telemetry/opentelemetry-rust/pull/2220#discussion_r1809701673

[stormshield-fabs]

I've addressed the pending comments and added changelog entries (feel free to edit them!)

[cijothomas]

@stormshield-fabs oops we just cause some merge conflicts! Could you resolve them. We can merge with one more approval. (Also, please update pr description to show before/after code snippets to make it easy for users to migrate)

[cijothomas]

@stormshield-fabs Can you resolve conflicts and address the nits, and we can merge!

As suggested offline, if you can add a migration guide in the PR description, similar to https://github.com/open-telemetry/opentelemetry-rust/pull/2221, that'd be awesome!

(The next release is a flood of breaking changes, so we need to write an overall migration guide, which will refer to the sub-migration-guides within each PR description.)

[lalitb]

@stormshield-fabs I took the liberty of fixing minor merge conflicts in this PR, as it was introduced by changes from my own PR. Hope that's fine :)