Skip to content

Conversation

@marcalff
Copy link
Member

@marcalff marcalff commented May 8, 2024

This is a follow up on PR #2553, to simplify bool is_shutdown_ in general.

Changes

Please provide a brief description of the changes here.

  • Replace every use of a spin lock + boolean by an std::atomic<bool>
  • Replace a spin lock by a mutex, when serializing to a file in the ostream metric exporter
  • Implemented the missing call to sout_.flush() in OStreamMetricExporter::ForceFlush()

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

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

@marcalff marcalff changed the title [EXPORTERS] General cleanup for is_shutdown_ flags in exporters. [EXPORTER] General cleanup for is_shutdown_ flags in exporters. May 8, 2024
@marcalff marcalff marked this pull request as ready for review May 8, 2024 15:58
@marcalff marcalff requested a review from a team May 8, 2024 15:58
@marcalff marcalff added the pr:please-review This PR is ready for review label May 8, 2024
Copy link
Member

@lalitb lalitb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the cleanup.

@codecov
Copy link

codecov bot commented May 8, 2024

Codecov Report

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

Project coverage is 87.54%. Comparing base (497eaf4) to head (60ce176).
Report is 60 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #2663      +/-   ##
==========================================
+ Coverage   87.12%   87.54%   +0.42%     
==========================================
  Files         200      188      -12     
  Lines        6109     5840     -269     
==========================================
- Hits         5322     5112     -210     
+ Misses        787      728      -59     
Files Coverage Δ
...lemetry/exporters/memory/in_memory_span_exporter.h 89.48% <100.00%> (-1.43%) ⬇️
...de/opentelemetry/exporters/ostream/span_exporter.h 100.00% <ø> (ø)
exporters/ostream/src/log_record_exporter.cc 97.06% <ø> (-0.08%) ⬇️
exporters/ostream/src/span_exporter.cc 87.88% <ø> (-0.24%) ⬇️
exporters/ostream/src/metric_exporter.cc 90.91% <33.34%> (-0.89%) ⬇️

... and 55 files with indirect coverage changes

@marcalff marcalff merged commit ce14bf6 into open-telemetry:main May 8, 2024
malkia added a commit to malkia/opentelemetry-cpp that referenced this pull request May 9, 2024
[EXPORTER] General cleanup for is_shutdown_ flags in exporters. (open-telemetry#2663)
@marcalff marcalff deleted the fix_spinlock_cleanup branch June 3, 2024 21:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr:please-review This PR is ready for review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants