Does otel-cpp require exception now?

[owent]

After upgrading to 1.29.0, building with -fno-exceptions fails with the following errors:

[ 50%] Building CXX object sdk/src/configuration/CMakeFiles/opentelemetry_configuration_core.dir/document_node.cc.o
/home/runner/work/cmake-toolset/cmake-toolset/test/third_party/packages/opentelemetry-cpp-v1.29.0/sdk/src/configuration/document_node.cc: In member function ‘std::string opentelemetry::v1::sdk::configuration::DocumentNode::DoOneSubstitution(const std::string&) const’:
/home/runner/work/cmake-toolset/cmake-toolset/test/third_party/packages/opentelemetry-cpp-v1.29.0/sdk/src/configuration/document_node.cc:116:53: error: exception handling disabled, use ‘-fexceptions’ to enable
  116 |     throw InvalidSchemaException(Location(), message);
      |                                                     ^
/home/runner/work/cmake-toolset/cmake-toolset/test/third_party/packages/opentelemetry-cpp-v1.29.0/sdk/src/configuration/document_node.cc: In member function ‘int64_t opentelemetry::v1::sdk::configuration::DocumentNode::SignedIntegerFromString(const std::string&) const’:
/home/runner/work/cmake-toolset/cmake-toolset/test/third_party/packages/opentelemetry-cpp-v1.29.0/sdk/src/configuration/document_node.cc:269:3: error: ‘...’ handler must be the last handler for its try block [-fpermissive]
  269 |   catch (const std::invalid_argument &)
      |   ^~~~~
/home/runner/work/cmake-toolset/cmake-toolset/test/third_party/packages/opentelemetry-cpp-v1.29.0/sdk/src/configuration/document_node.cc: In member function ‘bool opentelemetry::v1::sdk::configuration::DocumentNode::BooleanFromString(const std::string&) const’:
/home/runner/work/cmake-toolset/cmake-toolset/test/third_party/packages/opentelemetry-cpp-v1.29.0/sdk/src/configuration/document_node.cc:238:1: warning: control reaches end of non-void function [-Wreturn-type]
  238 | }
      | ^
gmake[2]: *** [sdk/src/configuration/CMakeFiles/opentelemetry_configuration_core.dir/build.make:79: sdk/src/configuration/CMakeFiles/opentelemetry_configuration_core.dir/document_node.cc.o] Error 1
gmake[1]: *** [CMakeFiles/Makefile2:923: sdk/src/configuration/CMakeFiles/opentelemetry_configuration_core.dir/all] Error 2
gmake: *** [Makefile:136: all] Error 2
'/usr/local/bin/cmake' '--build' '.' '--verbose'
Change Dir: '/home/runner/work/cmake-toolset/cmake-toolset/test/build_jobs_dir/_deps/opentelemetry_cpp-v1.29.0/linux-x86_64-gnu-13'

It looks like much of the code in the configuration module isn't guarded by OPENTELEMETRY_HAVE_EXCEPTIONS. Are exceptions now a hard requirement, or can I raise to a PR fixing this?

If a fix is welcome, I can see a few possible approaches:

1. Replace all throw XXXException statements with std::abort() when exceptions are disabled.
2. Disable the configuration module entirely when exceptions are disabled.
3. Rework the configuration APIs that currently throw so they report failures to the caller (e.g., via a status/result type), and log the original exception message to the global logger.It may changes a lot of APIs.

Which approach would you prefer? I'm happy to put together a PR once we agree on the direction.

[marcalff]

Disable the configuration module entirely when exceptions are disabled.

The configuration code needs exceptions because of the yaml parser in general, and to avoid tedious coding patterns when having return codes for failures to propagate everywhere.

This code was optional earlier, so one could build otel-cpp without declarative configuration, but this code recently was moved within the configuration_core library, which is not protected by a feature flag: WITH_CONFIGURATION now only protects the dependency to rapidyaml alone.

To discuss, it looks like we need a feature flag for configuration_core to make it optional.

cc @dbarker

[dbarker]

Thanks for raising this. Of the three options I think we should rule out 1) std::abort and 2) disable configuration. The promising option is to rework the API slightly to remove exceptions and still support fail fast configuration. The SDK user should remain in control over the lifetime of their application and opentelemetry-cpp should not terminate the process by design.

It seems reasonable to convert the SDK builder interface (ConfiguredSdk, SdkBuilders, and all the built-in buiders + utils) to be exception free since nearly all of these methods that are throwing return a smart pointer. The patch should change throw sites to log an error and return a nullptr (with the corresponding nullptr handling throughout). The document node class is one that will require an API change to return a value or error (std::expected like return value).

[dbarker]

Sharing some notes from the SIG meeting discussion today.

• The no-exception build of configuration_core should be a goal for the next release.
• The next step is to investigate options to remove exceptions from DocumentNode and the impact on derived classes.
• We discussed what expectations are reasonable for an SDK built with -fno-exceptions as many STL throwing types and functions are used. If built without exceptions then the application will terminate on throw and the spec requirements for error handling can not be met.

@owent are you able to share more about your use case for building with -fno-exceptions and expected SDK behavior when calling methods that throw?

[owent]

Disable the configuration module entirely when exceptions are disabled.

The configuration code needs exceptions because of the yaml parser in general, and to avoid tedious coding patterns when having return codes for failures to propagate everywhere.

This code was optional earlier, so one could build otel-cpp without declarative configuration, but this code recently was moved within the configuration_core library, which is not protected by a feature flag: WITH_CONFIGURATION now only protects the dependency to rapidyaml alone.

To discuss, it looks like we need a feature flag for configuration_core to make it optional.

cc @dbarker

I checked https://github.com/biojppm/rapidyaml and rapidyaml also support building without exception.

[owent]

Sharing some notes from the SIG meeting discussion today.

• The no-exception build of configuration_core should be a goal for the next release.
• The next step is to investigate options to remove exceptions from DocumentNode and the impact on derived classes.
• We discussed what expectations are reasonable for an SDK built with -fno-exceptions as many STL throwing types and functions are used. If built without exceptions then the application will terminate on throw and the spec requirements for error handling can not be met.

@owent are you able to share more about your use case for building with -fno-exceptions and expected SDK behavior when calling methods that throw?

From my perspective, any code path that throws already means the configuration parsing has failed — so it seems acceptable to handle it by reporting the configuration as invalid and doing nothing further (i.e., treating the configuration as absent).