Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[TEST] Fix sync problems in OTLP File exporter tests. #3031

Merged
merged 2 commits into from
Aug 16, 2024

Conversation

owent
Copy link
Member

@owent owent commented Aug 15, 2024

Fixes #3025

Changes

Please provide a brief description of the changes here.

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

@owent owent requested a review from a team August 15, 2024 15:11
@marcalff marcalff changed the title Fix sync problems in OTLP File exporters. [TEST] Fix sync problems in OTLP File exporters. Aug 15, 2024
@marcalff marcalff changed the title [TEST] Fix sync problems in OTLP File exporters. [TEST] Fix sync problems in OTLP File exporter tests. Aug 15, 2024
marcalff
marcalff previously approved these changes Aug 15, 2024
Copy link
Member

@marcalff marcalff left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for the test cleanup.

@marcalff marcalff self-requested a review August 15, 2024 15:24
Copy link
Member

@marcalff marcalff left a comment

Choose a reason for hiding this comment

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

The code change looks ok, but the test still fails under valgrind.

There must be another root cause to fix.

Copy link

codecov bot commented Aug 15, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 87.60%. Comparing base (497eaf4) to head (6405b50).
Report is 117 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #3031      +/-   ##
==========================================
+ Coverage   87.12%   87.60%   +0.49%     
==========================================
  Files         200      190      -10     
  Lines        6109     5870     -239     
==========================================
- Hits         5322     5142     -180     
+ Misses        787      728      -59     

see 123 files with indirect coverage changes

@marcalff marcalff self-requested a review August 15, 2024 15:27
@marcalff marcalff dismissed their stale review August 15, 2024 15:28

Need a second look, test still failing.

@owent
Copy link
Member Author

owent commented Aug 15, 2024

nlohmann::json::parse seems parse json text failed, still try to find out the reason.

@marcalff
Copy link
Member

nlohmann::json::parse seems parse json text failed, still try to find out the reason.

I have not found anything either.

Let's merge this, in hope that the next failure will bring more information about the root cause.

Copy link
Member

@marcalff marcalff left a comment

Choose a reason for hiding this comment

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

Ok to merge.

This will help narrow the root cause, so it can be better investigated.

@marcalff marcalff merged commit b890969 into open-telemetry:main Aug 16, 2024
52 checks passed
malkia added a commit to malkia/opentelemetry-cpp that referenced this pull request Aug 16, 2024
[TEST] Fix sync problems in OTLP File exporter tests. (open-telemetry#3031)
msiddhu added a commit to msiddhu/opentelemetry-cpp that referenced this pull request Aug 20, 2024
* [EXPORTER] Ignore exception when create thread in OTLP file exporter. (open-telemetry#3012)

* [BUILD] Update MODULE.bazel (open-telemetry#3015)

* [BUILD] Fix build without vcpkg on Windows when gRPC is disabled (open-telemetry#3016)

* [BUILD] Add abi_version_no bazel flag. (open-telemetry#3020)

* [Code health] Expand iwyu coverage to include unit tests. (open-telemetry#3022)

* [BUILD] Version opentelemetry_proto/proto_grpc shared libraries (open-telemetry#2992)

* [SEMANTIC CONVENTIONS] Upgrade semantic conventions to 1.27.0 (open-telemetry#3023)

* [SDK] Support empty histogram buckets (open-telemetry#3027)

* support empty buckets

* Update histogram_test.cc

* Update histogram_test.cc

* test for negative values

* fix count

* [TEST] Fix sync problems in OTLP File exporter tests. (open-telemetry#3031)

---------

Co-authored-by: WenTao Ou <owentou@tencent.com>
Co-authored-by: Carbo Kuo <BYVoid@users.noreply.github.com>
Co-authored-by: Manuel Bergler <m.bergler@tum.de>
Co-authored-by: Marc Alff <marc.alff@oracle.com>
Co-authored-by: Troels Hoffmeyer <Troels.d.hoffmeyer@gmail.com>
Co-authored-by: Lalit Kumar Bhasin <lalit_fin@yahoo.com>
@owent owent deleted the fixes_3025 branch September 26, 2024 02:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[TEST] test failure in otlp_file_exporter_test
2 participants