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

ElasticsearchLogRecordableTests.BasicTests fails on Ubuntu 24.04 with CXX_STANDARD=20 #3278

Open
dbarker opened this issue Feb 18, 2025 · 3 comments
Labels
bug Something isn't working triage/accepted Indicates an issue or PR is ready to be actively worked on.

Comments

@dbarker
Copy link
Contributor

dbarker commented Feb 18, 2025

Describe your environment
main at 3ca3c76
Ubuntu 24.04 based devcontainer

Steps to reproduce

  • Build with CXX_STANDARD=20
  • Run ./ci/do_ci.sh cmake.maintainer.async.test

What is the expected behavior?
test passes

What is the actual behavior?
ElasticsearchLogRecordableTests.BasicTests fails.


[----------] 1 test from ElasticsearchLogRecordableTests
[ RUN      ] ElasticsearchLogRecordableTests.BasicTests
/workspaces/opentelemetry-cpp/exporters/elasticsearch/test/es_log_record_exporter_test.cc:127: Failure
Expected equality of these values:
  actual
    Which is: {"@timestamp":"2024-11-20T00:52:24.999647774+00:00","boolean":false,"double":2.0,"ecs":{"version":"8.11.0"},"int":1,"log":{"level":"FATAL","logger":"scope_name"},"message":"Body of the log message","observedtimestamp":1732063944999647774,"stringlist":["string1","string2"]}
  expected
    Which is: {"@timestamp":"2024-11-20T00:52:24.999647Z","boolean":false,"double":2.0,"ecs":{"version":"8.11.0"},"int":1,"log":{"level":"FATAL","logger":"scope_name"},"message":"Body of the log message","observedtimestamp":1732063944999647774,"stringlist":["string1","string2"]}

[  FAILED  ] ElasticsearchLogRecordableTests.BasicTests (0 ms)
[----------] 1 test from ElasticsearchLogRecordableTests (0 ms total)

[----------] Global test environment tear-down
[==========] 1 test from 1 test suite ran. (0 ms total)
[  PASSED  ] 0 tests.
[  FAILED  ] 1 test, listed below:
[  FAILED  ] ElasticsearchLogRecordableTests.BasicTests

 1 FAILED TEST


@dbarker dbarker added the bug Something isn't working label Feb 18, 2025
@github-actions github-actions bot added the needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. label Feb 18, 2025
@lalitb lalitb added triage/accepted Indicates an issue or PR is ready to be actively worked on. and removed needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Feb 19, 2025
@sjinks
Copy link
Contributor

sjinks commented Feb 21, 2025

https://github.com/open-telemetry/opentelemetry-cpp/blob/main/exporters/elasticsearch/src/es_log_recordable.cc#L95

std::format() uses different precision

If the precision of the input cannot be exactly represented with seconds, then the format is a decimal floating-point number with a fixed format and a precision matching that of the precision of the input (or to a microseconds precision if the conversion to floating-point decimal seconds cannot be made within 18 fractional digits)

@dbarker
Copy link
Contributor Author

dbarker commented Feb 21, 2025

https://github.com/open-telemetry/opentelemetry-cpp/blob/main/exporters/elasticsearch/src/es_log_recordable.cc#L95

std::format() uses different precision

If the precision of the input cannot be exactly represented with seconds, then the format is a decimal floating-point number with a fixed format and a precision matching that of the precision of the input (or to a microseconds precision if the conversion to floating-point decimal seconds cannot be made within 18 fractional digits)

Good catch!

@sjinks
Copy link
Contributor

sjinks commented Feb 21, 2025

It makes sense to remove that conditional. As far as I know, there's no easy way to make std::format do proper rounding. We will either reinvent our own implementation of strftime() or have to use format() + substr(). But because format() is slower, the existing strftime() approach seems to be the best option.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working triage/accepted Indicates an issue or PR is ready to be actively worked on.
Projects
None yet
Development

No branches or pull requests

3 participants