Skip to content

[TEST] fix test failure with elasticsearch exporter on cxx20#3308

Merged
marcalff merged 2 commits intoopen-telemetry:mainfrom
dbarker:fix_elasticsearch_test_on_cxx20
Mar 24, 2025
Merged

[TEST] fix test failure with elasticsearch exporter on cxx20#3308
marcalff merged 2 commits intoopen-telemetry:mainfrom
dbarker:fix_elasticsearch_test_on_cxx20

Conversation

@dbarker
Copy link
Member

@dbarker dbarker commented Mar 21, 2025

Fixes #3278

Changes

Removes the conditional that switches to std::format on cxx20 in es_log_recordable.cc.

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

@dbarker dbarker requested a review from a team as a code owner March 21, 2025 16:52
@netlify
Copy link

netlify bot commented Mar 21, 2025 •
edited
Loading

✅ Deploy Preview for opentelemetry-cpp-api-docs canceled.

Name Link
🔨 Latest commit bc67a28
🔍 Latest deploy log https://app.netlify.com/sites/opentelemetry-cpp-api-docs/deploys/67e110fa2f268200080607ab

@dbarker
Copy link
Member Author

dbarker commented Mar 21, 2025 •
edited
Loading

Steps to reproduce the elasticsearch exporter test failure in a linux container.

  1. build and run a dev container setting CXX_STANDARD=20.
cd <path-to-opentelemetry-cpp>
docker build --build-arg CXX_STANDARD=20 --build-arg USER_UID="$(id -u)" --build-arg USER_GID="$(id -g)"  -t opentelemetry-cpp-dev:cxx20 -f ./.devcontainer/Dockerfile.dev .
docker run -it -v "$PWD:/workspaces/opentelemetry-cpp" opentelemetry-cpp-dev:cxx20 bash
  1. build the elasticsearch exporter and run tests
cmake -S . -B ~/build -DWITH_ELASTICSEARCH=ON
cmake --build ~/build 
ctest --test-dir ~/build

@dbarker dbarker changed the title fix test failure with elasticsearch exporter on cxx20 [TEST] fix test failure with elasticsearch exporter on cxx20 Mar 21, 2025
@codecov
Copy link

codecov bot commented Mar 21, 2025 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 89.52%. Comparing base (2c9b68f) to head (bc67a28).
Report is 1 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #3308      +/-   ##
==========================================
+ Coverage   89.50%   89.52%   +0.02%     
==========================================
  Files         210      210              
  Lines        6522     6522              
==========================================
+ Hits         5837     5838       +1     
+ Misses        685      684       -1

see 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

marcalff
marcalff approved these changes Mar 22, 2025
View reviewed changes
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 fix.

@marcalff marcalff added the ok-to-merge The PR is ok to merge (has two approves or raised by a maintainer/approver and has one approve) label Mar 24, 2025
@marcalff marcalff merged commit 2566fb6 into open-telemetry:main Mar 24, 2025
58 checks passed
@dbarker dbarker mentioned this pull request Mar 26, 2025
Closed
6 tasks
@dbarker dbarker deleted the fix_elasticsearch_test_on_cxx20 branch March 31, 2025 16:14
@BrewTestBot BrewTestBot mentioned this pull request Apr 1, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@owent owent owent approved these changes

@marcalff marcalff marcalff approved these changes

Assignees

No one assigned

Labels

ok-to-merge The PR is ok to merge (has two approves or raised by a maintainer/approver and has one approve)

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

ElasticsearchLogRecordableTests.BasicTests fails on Ubuntu 24.04 with CXX_STANDARD=20

3 participants

@dbarker @owent @marcalff