opentelemetry-cpp icon indicating copy to clipboard operation
opentelemetry-cpp copied to clipboard

[API/SDK] Provider cleanup

Open marcalff opened this issue 1 year ago • 4 comments

Fixes #2506

Changes

Please provide a brief description of the changes here.

  • Removed methods related to Flush() and Close() from the API class opentelemetry::trace::Tracer, starting with ABI version 2.
    • These methods are not intended for the API, and got exposed as an oversight
    • These methods belong to the SDK instead
  • Changed all XXXProviderFactory::Create() methods to return an SDK object instead of an API object.
    • Returning an SDK object is necessary for the application code to perform a proper initialisation, and proper shutdown of the SDK
    • In particular, returning an SDK object is required for the caller to invoke Flush() and Shutdown().
  • Removed static_cast<SDK*>(API object) from examples, and use cleaner code, to initialize SDK providers.

For significant contributions please make sure you have completed the following items:

  • [X] CHANGELOG.md updated for non-trivial changes
  • [ ] Unit tests have been added
  • [X] Changes in public API reviewed

marcalff avatar May 09 '24 14:05 marcalff

Codecov Report

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

Project coverage is 87.68%. Comparing base (497eaf4) to head (79595f5). Report is 72 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #2664      +/-   ##
==========================================
+ Coverage   87.12%   87.68%   +0.56%     
==========================================
  Files         200      190      -10     
  Lines        6109     5851     -258     
==========================================
- Hits         5322     5130     -192     
+ Misses        787      721      -66     
Files Coverage Δ
api/include/opentelemetry/trace/noop.h 93.34% <ø> (ø)
api/include/opentelemetry/trace/tracer.h 100.00% <ø> (ø)
sdk/include/opentelemetry/sdk/trace/tracer.h 100.00% <ø> (ø)
sdk/src/logs/event_logger_provider_factory.cc 100.00% <ø> (ø)
sdk/src/trace/tracer_provider_factory.cc 44.83% <30.00%> (+0.39%) :arrow_up:

... and 70 files with indirect coverage changes

codecov[bot] avatar May 09 '24 14:05 codecov[bot]

Good catch :)

lalitb avatar May 09 '24 15:05 lalitb

Waiting for the build to finish to make sure DLL links, then this will be ready for review.

marcalff avatar May 09 '24 15:05 marcalff

Add help-wanted label:

The windows DLL build fails with undefined symbols, and I can not figure out why (I don't have windows locally to investigate).

marcalff avatar May 10 '24 14:05 marcalff