Code flagged noexcept should not raise exceptions

[marcalff]

Currently, many APIs are flagged as noexcept.

For example:

class OPENTELEMETRY_EXPORT TracerProvider
{
public:
  virtual nostd::shared_ptr<Tracer> GetTracer(
      nostd::string_view name,
      nostd::string_view version,
      nostd::string_view schema_url,
      const common::KeyValueIterable *attributes) noexcept = 0;

When implemented in the SDK:

nostd::shared_ptr<trace_api::Tracer> TracerProvider::GetTracer(
    nostd::string_view name,
    nostd::string_view version,
    nostd::string_view schema_url,
    const opentelemetry::common::KeyValueIterable *attributes) noexcept
{
...
  auto tracer = std::shared_ptr<Tracer>(new Tracer(context_, std::move(scope)));
  tracers_.push_back(tracer);
  return nostd::shared_ptr<trace_api::Tracer>{tracer};
}

The call to new Tracer() can fail with bad_alloc, so an exception is still raised, breaking the noexcept contract.

Proposal:

nostd::shared_ptr<trace_api::Tracer> TracerProvider::GetTracerImpl(
    nostd::string_view name,
    nostd::string_view version,
    nostd::string_view schema_url,
    const opentelemetry::common::KeyValueIterable *attributes) {
}

nostd::shared_ptr<trace_api::Tracer> TracerProvider::GetTracer(
    nostd::string_view name,
    nostd::string_view version,
    nostd::string_view schema_url,
    const opentelemetry::common::KeyValueIterable *attributes) noexcept
{
  try {
    // invoke GetTracerImpl
  }
  catch {
    return a pre allocated Noop tracer instead.
  }
}

Forcing down the noexcept to the entire code base is not realistic, entry points to the SDK surface needs to handle failures explicitly instead, to give room to the SDK implementation to fail internally with exceptions if needed.

[dbarker]

The challenge seems to be the wide application of noexcept in the SDK.

The error handling policy in the spec is a bit more nuanced and allows relaxing the noexcept constraint in order to fail fast during initialization.

OpenTelemetry implementations MUST NOT throw unhandled exceptions at runtime.

1. API methods MUST NOT throw unhandled exceptions when used incorrectly by end users. The API and SDK SHOULD provide safe defaults for missing or invalid arguments. For instance, a name like empty may be used if the user passes in null as the span name argument during Span construction.
2. The API or SDK MAY fail fast and cause the application to fail on initialization, e.g. because of a bad user config or environment, but MUST NOT cause the application to fail later at runtime, e.g. due to dynamic config settings received from the Collector.
3. The SDK MUST NOT throw unhandled exceptions for errors in their own operations. For example, an exporter should not throw an exception when it cannot reach the endpoint to which it sends telemetry data.

I think we can take advantage of this to provide a more realistic error handling contract for the SDK.

Currently the SDK signal providers (LoggerProvider, TracerProvider, MeterProvider) and instrumentation scoped objects (Logger, Tracer, Meter) all have noexcept constructors. The providers Get<Logger,Meter,Tracer> methods that construct the instrumentation scoped objects also are marked noexcept.

Relaxing these and removing the noexcept may be something to explore.

[dbarker]

Notes based on SIG meeting discussion (8/10):

• The current constructors for TracerProvider, LoggerProvider, MeterProvider, Tracer, Meter, Logger are all noexcept and will terminate if any exception escapes.
• These objects allocated memory and may throw std::bad_alloc.
• These objects are constructed during initialization phase
  • Providers are constructed during SDK configuration
  • Tracer/Meter/Logger are constructed when calling *Provider::GetTracer/Meter/Logger for the first time in the instrumented library.
• If a Provider constructor throws during the initialization phase the exception should escape without termination so the caller can handle it (often this will be the configuration library which catches exceptions and logs errors currently).
• If a Tracer/Logger/Meter constructor throws the exception should escape without termination so the corresponding GetTracer/Logger/Meter method can handle it and return a noop Tracer/Meter/Logger to the caller.

Next steps:

• Remove noexcept from the constructors mentioned above.
• Add exception handling around the construction of Tracer/Meter/Logger in the provider and return noop objects when an exception is caught.

@marcalff Please edit if I missed anything.

[22elix3r]

I’d like to take this. I’ll follow the post-SIG direction: drop noexcept from the SDK provider/instrumentation constructors, keep GetTracer/GetLogger/GetMeter non-throwing, and return a valid noop object if construction of a new instrumentation object fails.