Merge branch 'main' into Metrics_DLL

open-telemetry · Oct 23, 2023 · 4a08f95 · 4a08f95
2 parents 96b1361 + f16deb0
commit 4a08f95
Show file tree

Hide file tree

Showing 16 changed files with 383 additions and 37 deletions.
diff --git a/.github/workflows/project_management_comment.yml b/.github/workflows/project_management_comment.yml
@@ -12,7 +12,7 @@ jobs:
       issues: write
     steps:
       - name: Add comment
-        uses: peter-evans/create-or-update-comment@c6c9a1a66007646a28c153e2a8580a5bad27bcfa
+        uses: peter-evans/create-or-update-comment@23ff15729ef2fc348714a3bb66d2f655ca9066f2
         with:
           issue-number: ${{ github.event.issue.number }}
           body: |

diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -17,6 +17,29 @@ Increment the:
 
 * [BUILD] Remove WITH_REMOVE_METER_PREVIEW, use WITH_ABI_VERSION_2 instead
   [#2370](https://github.com/open-telemetry/opentelemetry-cpp/pull/2370)
+* [BUILD] Make WITH_OTLP_HTTP_SSL_PREVIEW mainstream
+  [#2378](https://github.com/open-telemetry/opentelemetry-cpp/pull/2378)
+* [API] Add InstrumentationScope attributes in TracerProvider::GetTracer()
+  [#2371](https://github.com/open-telemetry/opentelemetry-cpp/pull/2371)
+
+Important changes:
+
+* [API] Add InstrumentationScope attributes in TracerProvider::GetTracer()
+  [#2371](https://github.com/open-telemetry/opentelemetry-cpp/pull/2371)
+  * TracerProvider::GetTracer() now accepts InstrumentationScope attributes.
+  * Because this is an `ABI` breaking change, the fix is only available
+    with the `CMake` option `WITH_ABI_VERSION_2=ON`.
+  * When building with `CMake` option `WITH_ABI_VERSION_1=ON` (by default)
+    the `ABI` is unchanged, and the fix is not available.
+
+* [BUILD] Make WITH_OTLP_HTTP_SSL_PREVIEW mainstream
+  [#2378](https://github.com/open-telemetry/opentelemetry-cpp/pull/2378)
+  * The experimental `CMake` option `WITH_OTLP_HTTP_SSL_PREVIEW`
+    is now promoted to stable. The default is changed to `ON`.
+  * The experimental `CMake` option `WITH_OTLP_HTTP_SSL_TLS_PREVIEW`
+    is now promoted to stable. The default is changed to `ON`.
+  * These build options are scheduled to be removed by the next release,
+    building without SSL/TLS will no longer be possible.
 
 Breaking changes:
 

diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -275,12 +275,12 @@ endif()
 
 option(WITH_ASYNC_EXPORT_PREVIEW "Whether to enable async export" OFF)
 
-# EXPERIMENTAL
-option(WITH_OTLP_HTTP_SSL_PREVIEW "Whether to enable otlp http ssl export" OFF)
+# STABLE
+option(WITH_OTLP_HTTP_SSL_PREVIEW "Whether to enable otlp http ssl export" ON)
 
-# EXPERIMENTAL
+# STABLE
 option(WITH_OTLP_HTTP_SSL_TLS_PREVIEW
-       "Whether to enable otlp http ssl tls min/max/cipher options" OFF)
+       "Whether to enable otlp http ssl tls min/max/cipher options" ON)
 
 # Exemplar specs status is experimental, so behind feature flag by default
 option(WITH_METRICS_EXEMPLAR_PREVIEW

diff --git a/api/include/opentelemetry/trace/noop.h b/api/include/opentelemetry/trace/noop.h
@@ -108,12 +108,23 @@ class OPENTELEMETRY_EXPORT NoopTracerProvider final : public trace::TracerProvid
       : tracer_{nostd::shared_ptr<trace::NoopTracer>(new trace::NoopTracer)}
   {}
 
-  nostd::shared_ptr<trace::Tracer> GetTracer(nostd::string_view /* library_name */,
-                                             nostd::string_view /* library_version */,
+#if OPENTELEMETRY_ABI_VERSION_NO >= 2
+  nostd::shared_ptr<trace::Tracer> GetTracer(
+      nostd::string_view /* name */,
+      nostd::string_view /* version */,
+      nostd::string_view /* schema_url */,
+      const common::KeyValueIterable * /* attributes */) noexcept override
+  {
+    return tracer_;
+  }
+#else
+  nostd::shared_ptr<trace::Tracer> GetTracer(nostd::string_view /* name */,
+                                             nostd::string_view /* version */,
                                              nostd::string_view /* schema_url */) noexcept override
   {
     return tracer_;
   }
+#endif
 
 private:
   nostd::shared_ptr<trace::Tracer> tracer_;

diff --git a/api/include/opentelemetry/trace/tracer_provider.h b/api/include/opentelemetry/trace/tracer_provider.h
@@ -3,6 +3,8 @@
 
 #pragma once
 
+#include "opentelemetry/common/key_value_iterable.h"
+#include "opentelemetry/common/key_value_iterable_view.h"
 #include "opentelemetry/nostd/shared_ptr.h"
 #include "opentelemetry/nostd/string_view.h"
 #include "opentelemetry/version.h"
@@ -20,15 +22,106 @@ class OPENTELEMETRY_EXPORT TracerProvider
 {
 public:
   virtual ~TracerProvider() = default;
+
+#if OPENTELEMETRY_ABI_VERSION_NO >= 2
+
+  /**
+   * Gets or creates a named Tracer instance (ABI).
+   *
+   * @since ABI_VERSION 2
+   *
+   * @param[in] name Tracer instrumentation scope
+   * @param[in] version Instrumentation scope version
+   * @param[in] schema_url Instrumentation scope schema URL
+   * @param[in] attributes Instrumentation scope attributes (optional, may be nullptr)
+   */
+  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;
+
+  /**
+   * Gets or creates a named Tracer instance (API helper).
+   *
+   * @since ABI_VERSION 2
+   *
+   * @param[in] name Tracer instrumentation scope
+   * @param[in] version Instrumentation scope version, optional
+   * @param[in] schema_url Instrumentation scope schema URL, optional
+   */
+  nostd::shared_ptr<Tracer> GetTracer(nostd::string_view name,
+                                      nostd::string_view version    = "",
+                                      nostd::string_view schema_url = "")
+  {
+    return GetTracer(name, version, schema_url, nullptr);
+  }
+
+  /**
+   * Gets or creates a named Tracer instance (API helper).
+   *
+   * @since ABI_VERSION 2
+   *
+   * @param[in] name Tracer instrumentation scope
+   * @param[in] version Instrumentation scope version
+   * @param[in] schema_url Instrumentation scope schema URL
+   * @param[in] attributes Instrumentation scope attributes
+   */
+  nostd::shared_ptr<Tracer> GetTracer(
+      nostd::string_view name,
+      nostd::string_view version,
+      nostd::string_view schema_url,
+      std::initializer_list<std::pair<nostd::string_view, common::AttributeValue>> attributes)
+  {
+    /* Build a container from std::initializer_list. */
+    nostd::span<const std::pair<nostd::string_view, common::AttributeValue>> attributes_span{
+        attributes.begin(), attributes.end()};
+
+    /* Build a view on the container. */
+    common::KeyValueIterableView<
+        nostd::span<const std::pair<nostd::string_view, common::AttributeValue>>>
+        iterable_attributes{attributes_span};
+
+    /* Add attributes using the view. */
+    return GetTracer(name, version, schema_url, &iterable_attributes);
+  }
+
+  /**
+   * Gets or creates a named Tracer instance (API helper).
+   *
+   * @since ABI_VERSION 2
+   *
+   * @param[in] name Tracer instrumentation scope
+   * @param[in] version Instrumentation scope version
+   * @param[in] schema_url Instrumentation scope schema URL
+   * @param[in] attributes Instrumentation scope attributes container
+   */
+  template <class T,
+            nostd::enable_if_t<common::detail::is_key_value_iterable<T>::value> * = nullptr>
+  nostd::shared_ptr<Tracer> GetTracer(nostd::string_view name,
+                                      nostd::string_view version,
+                                      nostd::string_view schema_url,
+                                      const T &attributes)
+  {
+    /* Build a view on the container. */
+    common::KeyValueIterableView<T> iterable_attributes(attributes);
+
+    /* Add attributes using the view. */
+    return GetTracer(name, version, schema_url, &iterable_attributes);
+  }
+
+#else
+
   /**
    * Gets or creates a named tracer instance.
    *
    * Optionally a version can be passed to create a named and versioned tracer
    * instance.
    */
-  virtual nostd::shared_ptr<Tracer> GetTracer(nostd::string_view library_name,
-                                              nostd::string_view library_version = "",
-                                              nostd::string_view schema_url      = "") noexcept = 0;
+  virtual nostd::shared_ptr<Tracer> GetTracer(nostd::string_view name,
+                                              nostd::string_view version    = "",
+                                              nostd::string_view schema_url = "") noexcept = 0;
+#endif
 };
 }  // namespace trace
 OPENTELEMETRY_END_NAMESPACE
diff --git a/api/test/singleton/singleton_test.cc b/api/test/singleton/singleton_test.cc
@@ -264,13 +264,25 @@ class MyTracerProvider : public trace::TracerProvider
     return result;
   }
 
-  nostd::shared_ptr<trace::Tracer> GetTracer(nostd::string_view /* library_name */,
-                                             nostd::string_view /* library_version */,
+#if OPENTELEMETRY_ABI_VERSION_NO >= 2
+  nostd::shared_ptr<trace::Tracer> GetTracer(
+      nostd::string_view /* name */,
+      nostd::string_view /* version */,
+      nostd::string_view /* schema_url */,
+      const common::KeyValueIterable * /* attributes */) noexcept override
+  {
+    nostd::shared_ptr<trace::Tracer> result(new MyTracer());
+    return result;
+  }
+#else
+  nostd::shared_ptr<trace::Tracer> GetTracer(nostd::string_view /* name */,
+                                             nostd::string_view /* version */,
                                              nostd::string_view /* schema_url */) noexcept override
   {
     nostd::shared_ptr<trace::Tracer> result(new MyTracer());
     return result;
   }
+#endif
 };
 
 void setup_otel()

diff --git a/api/test/trace/provider_test.cc b/api/test/trace/provider_test.cc
@@ -3,6 +3,7 @@
 
 #include "opentelemetry/trace/provider.h"
 #include "opentelemetry/nostd/shared_ptr.h"
+#include "opentelemetry/trace/tracer_provider.h"
 
 #include <gtest/gtest.h>
 
@@ -14,12 +15,24 @@ namespace nostd = opentelemetry::nostd;
 
 class TestProvider : public TracerProvider
 {
-  nostd::shared_ptr<Tracer> GetTracer(nostd::string_view /* library_name */,
-                                      nostd::string_view /* library_version */,
+
+#if OPENTELEMETRY_ABI_VERSION_NO >= 2
+  nostd::shared_ptr<Tracer> GetTracer(
+      nostd::string_view /* name */,
+      nostd::string_view /* version */,
+      nostd::string_view /* schema_url */,
+      const opentelemetry::common::KeyValueIterable * /* attributes */) noexcept override
+  {
+    return nostd::shared_ptr<Tracer>(nullptr);
+  }
+#else
+  nostd::shared_ptr<Tracer> GetTracer(nostd::string_view /* name */,
+                                      nostd::string_view /* version */,
                                       nostd::string_view /* schema_url */) noexcept override
   {
     return nostd::shared_ptr<Tracer>(nullptr);
   }
+#endif
 };
 
 TEST(Provider, GetTracerProviderDefault)

diff --git a/sdk/include/opentelemetry/sdk/metrics/aggregation/default_aggregation.h b/sdk/include/opentelemetry/sdk/metrics/aggregation/default_aggregation.h
@@ -35,7 +35,8 @@ class DefaultAggregation
       case AggregationType::kSum:
         return (instrument_descriptor.value_type_ == InstrumentValueType::kLong)
                    ? std::move(std::unique_ptr<Aggregation>(new LongSumAggregation(is_monotonic)))
-                   : std::move(std::unique_ptr<Aggregation>(new DoubleSumAggregation(true)));
+                   : std::move(
+                         std::unique_ptr<Aggregation>(new DoubleSumAggregation(is_monotonic)));
         break;
       case AggregationType::kHistogram: {
         if (instrument_descriptor.value_type_ == InstrumentValueType::kLong)

diff --git a/sdk/include/opentelemetry/sdk/metrics/async_instruments.h b/sdk/include/opentelemetry/sdk/metrics/async_instruments.h
@@ -25,6 +25,7 @@ class ObservableInstrument : public opentelemetry::metrics::ObservableInstrument
   ObservableInstrument(InstrumentDescriptor instrument_descriptor,
                        std::unique_ptr<AsyncWritableMetricStorage> storage,
                        std::shared_ptr<ObservableRegistry> observable_registry);
+  ~ObservableInstrument() override;
 
   void AddCallback(opentelemetry::metrics::ObservableCallbackPtr callback,
                    void *state) noexcept override;

diff --git a/sdk/include/opentelemetry/sdk/metrics/state/observable_registry.h b/sdk/include/opentelemetry/sdk/metrics/state/observable_registry.h
@@ -35,6 +35,8 @@ class ObservableRegistry
                       void *state,
                       opentelemetry::metrics::ObservableInstrument *instrument);
 
+  void CleanupCallback(opentelemetry::metrics::ObservableInstrument *instrument);
+
   void Observe(opentelemetry::common::SystemTimestamp collection_ts);
 
 private:

diff --git a/sdk/include/opentelemetry/sdk/trace/tracer_provider.h b/sdk/include/opentelemetry/sdk/trace/tracer_provider.h
@@ -63,10 +63,23 @@ class TracerProvider final : public opentelemetry::trace::TracerProvider
 
   ~TracerProvider() override;
 
+  /*
+    Make sure GetTracer() helpers from the API are seen in overload resolution.
+  */
+  using opentelemetry::trace::TracerProvider::GetTracer;
+
+#if OPENTELEMETRY_ABI_VERSION_NO >= 2
+  opentelemetry::nostd::shared_ptr<opentelemetry::trace::Tracer> GetTracer(
+      nostd::string_view name,
+      nostd::string_view version,
+      nostd::string_view schema_url,
+      const opentelemetry::common::KeyValueIterable *attributes) noexcept override;
+#else
   opentelemetry::nostd::shared_ptr<opentelemetry::trace::Tracer> GetTracer(
-      nostd::string_view library_name,
-      nostd::string_view library_version = "",
-      nostd::string_view schema_url      = "") noexcept override;
+      nostd::string_view name,
+      nostd::string_view version    = "",
+      nostd::string_view schema_url = "") noexcept override;
+#endif
 
   /**
    * Attaches a span processor to list of configured processors for this tracer provider.

diff --git a/sdk/src/metrics/async_instruments.cc b/sdk/src/metrics/async_instruments.cc
@@ -21,6 +21,11 @@ ObservableInstrument::ObservableInstrument(InstrumentDescriptor instrument_descr
 
 {}
 
+ObservableInstrument::~ObservableInstrument()
+{
+  observable_registry_->CleanupCallback(this);
+}
+
 void ObservableInstrument::AddCallback(opentelemetry::metrics::ObservableCallbackPtr callback,
                                        void *state) noexcept
 {

diff --git a/sdk/src/metrics/state/observable_registry.cc b/sdk/src/metrics/state/observable_registry.cc
@@ -38,6 +38,16 @@ void ObservableRegistry::RemoveCallback(opentelemetry::metrics::ObservableCallba
   callbacks_.erase(new_end, callbacks_.end());
 }
 
+void ObservableRegistry::CleanupCallback(opentelemetry::metrics::ObservableInstrument *instrument)
+{
+  std::lock_guard<std::mutex> lock_guard{callbacks_m_};
+  auto iter = std::remove_if(callbacks_.begin(), callbacks_.end(),
+                             [instrument](const std::unique_ptr<ObservableCallbackRecord> &record) {
+                               return record->instrument == instrument;
+                             });
+  callbacks_.erase(iter, callbacks_.end());
+}
+
 void ObservableRegistry::Observe(opentelemetry::common::SystemTimestamp collection_ts)
 {
   std::lock_guard<std::mutex> lock_guard{callbacks_m_};