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

[BUILD] Opentelemetry C++20 extension causes Envoy build issue #2572

Closed
shiponcs opened this issue Mar 1, 2024 · 8 comments
Closed

[BUILD] Opentelemetry C++20 extension causes Envoy build issue #2572

shiponcs opened this issue Mar 1, 2024 · 8 comments
Assignees
Labels
bug Something isn't working triage/needs-information Indicates an issue needs more information in order to work on it.

Comments

@shiponcs
Copy link

shiponcs commented Mar 1, 2024

Title: [BUILD] Opentelemetry C++20 extension causes Envoy build issue

Description:
Envoy build failed because of Opentelemetry C++20 requirement.

$ bazel build envoy
ERROR: /source/source/extensions/tracers/opentelemetry/BUILD:14:19: Compiling source/extensions/tracers/opentelemetry/config.cc failed: (Exit 1): clang-14 failed: error executing command (from target //source/extensions/tracers/opentelemetry:config) /opt/llvm/bin/clang-14 -U_FORTIFY_SOURCE -fstack-protector -Wall -Wthread-safety -Wself-assign -Wunused-but-set-parameter -Wno-free-nonheap-object -fcolor-diagnostics -fno-omit-frame-pointer ... (remaining 239 arguments skipped)

Use --sandbox_debug to see verbose messages from the sandbox and retain the sandbox build root for debugging
In file included from source/extensions/tracers/opentelemetry/config.cc:8:
In file included from ./source/extensions/tracers/opentelemetry/opentelemetry_tracer_impl.h:17:
In file included from bazel-out/k8-fastbuild/bin/external/io_opentelemetry_cpp/api/_virtual_includes/api/opentelemetry/logs/provider.h:10:
In file included from bazel-out/k8-fastbuild/bin/external/io_opentelemetry_cpp/api/_virtual_includes/api/opentelemetry/logs/noop.h:11:
In file included from bazel-out/k8-fastbuild/bin/external/io_opentelemetry_cpp/api/_virtual_includes/api/opentelemetry/logs/event_logger.h:7:
bazel-out/k8-fastbuild/bin/external/io_opentelemetry_cpp/api/_virtual_includes/api/opentelemetry/logs/logger.h:261:5: error: use of the 'likely' attribute is a C++20 extension [-Werror,-Wc++20-attribute-extensions]
    OPENTELEMETRY_LIKELY_IF(Enabled(severity) == false) { return false; }
    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
bazel-out/k8-fastbuild/bin/external/io_opentelemetry_cpp/api/_virtual_includes/api/opentelemetry/common/macros.h:12:11: note: expanded from macro 'OPENTELEMETRY_LIKELY_IF'
        [[likely]]
        ~~^~~~~~~~
In file included from source/extensions/tracers/opentelemetry/config.cc:8:
In file included from ./source/extensions/tracers/opentelemetry/opentelemetry_tracer_impl.h:17:
In file included from bazel-out/k8-fastbuild/bin/external/io_opentelemetry_cpp/api/_virtual_includes/api/opentelemetry/logs/provider.h:10:
In file included from bazel-out/k8-fastbuild/bin/external/io_opentelemetry_cpp/api/_virtual_includes/api/opentelemetry/logs/noop.h:11:
In file included from bazel-out/k8-fastbuild/bin/external/io_opentelemetry_cpp/api/_virtual_includes/api/opentelemetry/logs/event_logger.h:7:
bazel-out/k8-fastbuild/bin/external/io_opentelemetry_cpp/api/_virtual_includes/api/opentelemetry/logs/logger.h:267:5: error: use of the 'likely' attribute is a C++20 extension [-Werror,-Wc++20-attribute-extensions]
    OPENTELEMETRY_LIKELY_IF(Enabled(severity) == false) { return false; }
    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
bazel-out/k8-fastbuild/bin/external/io_opentelemetry_cpp/api/_virtual_includes/api/opentelemetry/common/macros.h:12:11: note: expanded from macro 'OPENTELEMETRY_LIKELY_IF'
        [[likely]]
        ~~^~~~~~~~
2 errors generated.
Target //source/exe:envoy-static failed to build
Use --verbose_failures to see the command lines of failed build steps.
INFO: Elapsed time: 2625.963s, Critical Path: 76.06s
INFO: 14619 processes: 6232 internal, 1 local, 8385 processwrapper-sandbox, 1 worker.
FAILED: Build did NOT complete successfully
@shiponcs shiponcs added the bug Something isn't working label Mar 1, 2024
@github-actions github-actions bot added the needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. label Mar 1, 2024
@marcalff
Copy link
Member

marcalff commented Mar 1, 2024

Please clarify:

  • which compiler is used, including the exact compiler version
  • which --std=c++ standard version is used (C++11/14/17/20/23)

The relevant opentelemetry-cpp code is:

#if !defined(OPENTELEMETRY_LIKELY_IF) && defined(__cplusplus)
// GCC 9 has likely attribute but do not support declare it at the beginning of statement
#  if defined(__has_cpp_attribute) && (defined(__clang__) || !defined(__GNUC__) || __GNUC__ > 9)
#    if __has_cpp_attribute(likely)
#      define OPENTELEMETRY_LIKELY_IF(...) \
        if (__VA_ARGS__)                   \
        [[likely]]

#    endif
#  endif
#endif
#if !defined(OPENTELEMETRY_LIKELY_IF) && (defined(__clang__) || defined(__GNUC__))
#  define OPENTELEMETRY_LIKELY_IF(...) if (__builtin_expect(!!(__VA_ARGS__), true))
#endif
#ifndef OPENTELEMETRY_LIKELY_IF
#  define OPENTELEMETRY_LIKELY_IF(...) if (__VA_ARGS__)
#endif

In other words, opentelemetry-cpp will use [[likely]] if the compiler supports it.

Building with [-Werror,-Wc++20-attribute-extensions] is likely the cause of the build break here.

@owent
Copy link
Member

owent commented Mar 2, 2024

Just wondering why envoy do not allow -Wc++20-attribute-extensions here? Is it necessary for otel-cpp to close this warning for some specify compilers here?

@shiponcs
Copy link
Author

shiponcs commented Mar 4, 2024

@marcalff

I am building envoy-contrib with this option --config=rbe-toolchain-clang-libc++.

I have tried building -Wno-c++20-attribute-extensions that failed also.

@marcalff marcalff self-assigned this Mar 6, 2024
@marcalff
Copy link
Member

marcalff commented Mar 7, 2024

I am building envoy-contrib with this option --config=rbe-toolchain-clang-libc++.

Ok, but this does not tell us anything about the compiler version, and the C++ standard used.

@marcalff marcalff added triage/needs-information Indicates an issue needs more information in order to work on it. and removed needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Mar 7, 2024
marcalff added a commit to marcalff/opentelemetry-cpp that referenced this issue Mar 7, 2024
@shiponcs
Copy link
Author

I am building envoy-contrib with this option --config=rbe-toolchain-clang-libc++.

Ok, but this does not tell us anything about the compiler version, and the C++ standard used.

You're right. I have to ask Envoy team about this.

@shiponcs
Copy link
Author

FYI, Envoy has recently switched to C++ 20. So, we can build without hitting any error. envoyproxy/envoy#32660 (comment)

esigo pushed a commit that referenced this issue Mar 11, 2024
* Contributes to #2572

* Restored support for __builtin_expect()

* Typo
Copy link

This issue was marked as stale due to lack of activity.

@github-actions github-actions bot added the Stale label May 11, 2024
@shiponcs shiponcs reopened this May 13, 2024
@github-actions github-actions bot added the needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. label May 13, 2024
@marcalff
Copy link
Member

@shiponcs

I see this issue got reopened.

PR #2580 should resolve the issue with C++20, what are the remaining problems at this point ?

@github-actions github-actions bot removed the Stale label May 14, 2024
@marcalff marcalff removed the needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. label May 20, 2024
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/needs-information Indicates an issue needs more information in order to work on it.
Projects
None yet
Development

No branches or pull requests

3 participants