Avoid setting an empty (null) TraceProvider in trace::Provider::SetTracerProvider

[grrtrr]

Problem Description

By default, the global TracerProvider returns shared pointer to a NoopTracerProvider from trace::Provider::GetTracerProvider(), which has this comment:

// api/include/opentelemetry/trace/provider.h
  /**
   * Returns the singleton TracerProvider.
   *
   * By default, a no-op TracerProvider is returned. This will never return a
   * nullptr TracerProvider.
   */
  static nostd::shared_ptr<TracerProvider> GetTracerProvider() noexcept

The SetTracerProvider allows to set the global singleton instance to a null/empty TracerProvider:

// Similar for empty shared_ptr / unique_ptr:
opentelemetry::trace::Provider::SetTracerProvider(nullptr);

Some of the examples do that to clean up.

While this is fine at the end of a program, when one is sure that there are no more calls to trace::Provider::GetTracerProvider(), this can easily break.

The following segfaults:

opentelemetry::trace::Provider::SetTracerProvider(nullptr);

// ... Many lines below (or in some other file):
auto provider = opentelemetry::trace::Provider::GetTracerProvider();
auto tracer = provider->GetTracer("main");  // <== SEGMENTATION FAULT

Suggestion

Instead of accepting an empty shared_ptr in SetTracerProvider, replace with NoopTracerProvider:

// api/include/opentelemetry/trace/provider.h
  static void SetTracerProvider(const nostd::shared_ptr<TracerProvider> &tp) noexcept
  {
    std::lock_guard<common::SpinLockMutex> guard(GetLock());
    if (!tp) {
        // Ensure that GetTracerProvider never returns a null/empty TracerProvider:
        GetProvider() = std::make_shared<NoopTracerProvider>();
    } else {
        GetProvider() = tp;
    }
  }

[perhapsmaple]

I've encountered this issue a couple of times. It's something that can be easily overlooked but can cause a lot of frustration when it happens. I'll be happy to contribute a patch.

[Reneg973]

I don't think that the design here is correct. It allows code in Cpp#1 to set a provider and somewhere in Cpp#2 another provider could be set, replacing the first one.
Should Provider really be a singleton? Means there is just one tracer provider overall? Then it should be allowed to just override the tracer once.
Or is it allowed to have different tracers? Then the static functions are not correct.

Edit: ... checked the tests:
According to them it's just allowed to set one tracer provider globally.
Proposal, take ownership of the tracer provider by adding an owner parameter, e.g.:

  static bool SetTracerProvider(const nostd::shared_ptr<TracerProvider> &tp, void* owner) noexcept
  {
    std::lock_guard<common::SpinLockMutex> guard(GetLock());
    static void* owner_ = nullptr;
    if (owner_ == nullptr ? owner != nullptr : owner_ == owner)
    {
      GetProvider() = tp ? tp : new NoopTracerProvider();
      owner_ = owner;
      return true;
    }
    return false;
  }

Owner can be this, or a function pointer of the caller or even a pointer to the tracer provider itself.