[EXPORTER] add instrumentation scope attributes to otlp proto messages for traces and metrics (open-telemetry#3185)

Author: dbarker

CHANGELOG.md

@@ -149,6 +149,9 @@ Increment the:
 * [bazel] Update opentelemetry-proto in MODULE.bazel
   [#3163](https://github.com/open-telemetry/opentelemetry-cpp/pull/3163)
 
+* [EXPORTER] Fix scope attributes missing from otlp traces metrics
+  [#3185](https://github.com/open-telemetry/opentelemetry-cpp/pull/3185)
+
 Important changes:
 
 * [API] Jaeger Propagator should not be deprecated

exporters/otlp/include/opentelemetry/exporters/otlp/otlp_populate_attribute_utils.h

@@ -6,6 +6,7 @@
 #include "opentelemetry/common/attribute_value.h"
 #include "opentelemetry/nostd/string_view.h"
 #include "opentelemetry/sdk/common/attribute_utils.h"
+#include "opentelemetry/sdk/instrumentationscope/instrumentation_scope.h"
 #include "opentelemetry/sdk/resource/resource.h"
 #include "opentelemetry/version.h"
 
@@ -20,6 +21,7 @@ namespace v1
 {
 class AnyValue;
 class KeyValue;
+class InstrumentationScope;
 }  // namespace v1
 }  // namespace common
 
@@ -49,6 +51,10 @@ class OtlpPopulateAttributeUtils
   static void PopulateAttribute(opentelemetry::proto::resource::v1::Resource *proto,
                                 const opentelemetry::sdk::resource::Resource &resource) noexcept;
 
+  static void PopulateAttribute(opentelemetry::proto::common::v1::InstrumentationScope *proto,
+                                const opentelemetry::sdk::instrumentationscope::InstrumentationScope
+                                    &instrumentation_scope) noexcept;
+
   static void PopulateAnyValue(opentelemetry::proto::common::v1::AnyValue *proto_value,
                                const opentelemetry::common::AttributeValue &value) noexcept;
 

exporters/otlp/src/otlp_metric_utils.cc

@@ -242,6 +242,8 @@ void OtlpMetricUtils::PopulateResourceMetrics(
   OtlpPopulateAttributeUtils::PopulateAttribute(resource_metrics->mutable_resource(),
                                                 *(data.resource_));
 
+  resource_metrics->set_schema_url(data.resource_->GetSchemaURL());
+
   for (auto &scope_metrics : data.scope_metric_data_)
   {
     if (scope_metrics.scope_ == nullptr)
@@ -252,7 +254,9 @@ void OtlpMetricUtils::PopulateResourceMetrics(
     proto::common::v1::InstrumentationScope *scope = scope_lib_metrics->mutable_scope();
     scope->set_name(scope_metrics.scope_->GetName());
     scope->set_version(scope_metrics.scope_->GetVersion());
-    resource_metrics->set_schema_url(scope_metrics.scope_->GetSchemaURL());
+    scope_lib_metrics->set_schema_url(scope_metrics.scope_->GetSchemaURL());
+
+    OtlpPopulateAttributeUtils::PopulateAttribute(scope, *scope_metrics.scope_);
 
     for (auto &metric_data : scope_metrics.metric_data_)
     {

exporters/otlp/src/otlp_populate_attribute_utils.cc

@@ -15,6 +15,7 @@
 #include "opentelemetry/nostd/utility.h"
 #include "opentelemetry/nostd/variant.h"
 #include "opentelemetry/sdk/common/attribute_utils.h"
+#include "opentelemetry/sdk/instrumentationscope/instrumentation_scope.h"
 #include "opentelemetry/sdk/resource/resource.h"
 #include "opentelemetry/version.h"
 
@@ -315,6 +316,17 @@ void OtlpPopulateAttributeUtils::PopulateAttribute(
   }
 }
 
+void OtlpPopulateAttributeUtils::PopulateAttribute(
+    opentelemetry::proto::common::v1::InstrumentationScope *proto,
+    const opentelemetry::sdk::instrumentationscope::InstrumentationScope
+        &instrumentation_scope) noexcept
+{
+  for (const auto &kv : instrumentation_scope.GetAttributes())
+  {
+    OtlpPopulateAttributeUtils::PopulateAttribute(proto->add_attributes(), kv.first, kv.second);
+  }
+}
+
 }  // namespace otlp
 }  // namespace exporter
 OPENTELEMETRY_END_NAMESPACE

exporters/otlp/src/otlp_recordable.cc

@@ -107,6 +107,7 @@ proto::common::v1::InstrumentationScope OtlpRecordable::GetProtoInstrumentationS
   {
     instrumentation_scope.set_name(instrumentation_scope_->GetName());
     instrumentation_scope.set_version(instrumentation_scope_->GetVersion());
+    OtlpPopulateAttributeUtils::PopulateAttribute(&instrumentation_scope, *instrumentation_scope_);
   }
   return instrumentation_scope;
 }

exporters/otlp/src/otlp_recordable_utils.cc

@@ -107,6 +107,9 @@ void OtlpRecordableUtils::PopulateRequest(
         proto::common::v1::InstrumentationScope instrumentation_scope_proto;
         instrumentation_scope_proto.set_name(input_scope_spans.first->GetName());
         instrumentation_scope_proto.set_version(input_scope_spans.first->GetVersion());
+        OtlpPopulateAttributeUtils::PopulateAttribute(&instrumentation_scope_proto,
+                                                      *input_scope_spans.first);
+
         *scope_spans->mutable_scope() = instrumentation_scope_proto;
         scope_spans->set_schema_url(input_scope_spans.first->GetSchemaURL());
       }
@@ -170,11 +173,7 @@ void OtlpRecordableUtils::PopulateRequest(
             proto_scope->set_name(input_scope_log.first->GetName());
             proto_scope->set_version(input_scope_log.first->GetVersion());
 
-            for (auto &scope_attribute : input_scope_log.first->GetAttributes())
-            {
-              OtlpPopulateAttributeUtils::PopulateAttribute(
-                  proto_scope->add_attributes(), scope_attribute.first, scope_attribute.second);
-            }
+            OtlpPopulateAttributeUtils::PopulateAttribute(proto_scope, *input_scope_log.first);
           }
           output_scope_log->set_schema_url(input_scope_log.first->GetSchemaURL());
         }

exporters/otlp/test/otlp_file_exporter_test.cc

@@ -95,7 +95,7 @@ class OtlpFileExporterTestPeer : public ::testing::Test
     resource_attributes["vec_uint64_value"]          = std::vector<uint64_t>{7, 8};
     resource_attributes["vec_double_value"]          = std::vector<double>{3.2, 3.3};
     resource_attributes["vec_string_value"]          = std::vector<std::string>{"vector", "string"};
-    auto resource = resource::Resource::Create(resource_attributes);
+    auto resource = resource::Resource::Create(resource_attributes, "resource_url");
 
     auto processor_opts                  = sdk::trace::BatchSpanProcessorOptions();
     processor_opts.max_export_batch_size = 5;
@@ -109,9 +109,17 @@ class OtlpFileExporterTestPeer : public ::testing::Test
 
     std::string report_trace_id;
 
+#if OPENTELEMETRY_ABI_VERSION_NO >= 2
+    auto tracer = provider->GetTracer("scope_name", "scope_version", "scope_url",
+                                      {{ "scope_key",
+                                         "scope_value" }});
+#else
+    auto tracer = provider->GetTracer("scope_name", "scope_version", "scope_url");
+#endif
+
+    auto parent_span = tracer->StartSpan("Test parent span");
+
     char trace_id_hex[2 * trace_api::TraceId::kSize] = {0};
-    auto tracer                                      = provider->GetTracer("test");
-    auto parent_span                                 = tracer->StartSpan("Test parent span");
 
     trace_api::StartSpanOptions child_span_opts = {};
     child_span_opts.parent                      = parent_span->GetContext();
@@ -136,11 +144,22 @@ class OtlpFileExporterTestPeer : public ::testing::Test
       auto check_json = nlohmann::json::parse(check_json_text, nullptr, false);
       if (!check_json.is_discarded())
       {
-        auto resource_span     = *check_json["resourceSpans"].begin();
-        auto scope_span        = *resource_span["scopeSpans"].begin();
-        auto span              = *scope_span["spans"].begin();
-        auto received_trace_id = span["traceId"].get<std::string>();
-        EXPECT_EQ(received_trace_id, report_trace_id);
+        auto resource_span = *check_json["resourceSpans"].begin();
+        auto scope_span    = *resource_span["scopeSpans"].begin();
+        auto scope         = scope_span["scope"];
+        auto span          = *scope_span["spans"].begin();
+
+#if OPENTELEMETRY_ABI_VERSION_NO >= 2
+        ASSERT_EQ(1, scope["attributes"].size());
+        const auto scope_attribute = scope["attributes"].front();
+        EXPECT_EQ("scope_key", scope_attribute["key"].get<std::string>());
+        EXPECT_EQ("scope_value", scope_attribute["value"]["stringValue"].get<std::string>());
+#endif
+        EXPECT_EQ("resource_url", resource_span["schemaUrl"].get<std::string>());
+        EXPECT_EQ("scope_url", scope_span["schemaUrl"].get<std::string>());
+        EXPECT_EQ("scope_name", scope["name"].get<std::string>());
+        EXPECT_EQ("scope_version", scope["version"].get<std::string>());
+        EXPECT_EQ(report_trace_id, span["traceId"].get<std::string>());
       }
       else
       {

exporters/otlp/test/otlp_file_metric_exporter_test.cc

@@ -85,11 +85,14 @@ class OtlpFileMetricExporterTestPeer : public ::testing::Test
     opentelemetry::sdk::metrics::SumPointData sum_point_data2{};
     sum_point_data2.value_ = 20.0;
     opentelemetry::sdk::metrics::ResourceMetrics data;
+
     auto resource = opentelemetry::sdk::resource::Resource::Create(
-        opentelemetry::sdk::resource::ResourceAttributes{});
+        opentelemetry::sdk::resource::ResourceAttributes{}, "resource_url");
     data.resource_ = &resource;
-    auto scope     = opentelemetry::sdk::instrumentationscope::InstrumentationScope::Create(
-        "library_name", "1.5.0");
+
+    auto scope = opentelemetry::sdk::instrumentationscope::InstrumentationScope::Create(
+        "library_name", "1.5.0", "scope_url", {{"scope_key", "scope_value"}});
+
     opentelemetry::sdk::metrics::MetricData metric_data{
         opentelemetry::sdk::metrics::InstrumentDescriptor{
             "metrics_library_name", "metrics_description", "metrics_unit",
@@ -100,6 +103,7 @@ class OtlpFileMetricExporterTestPeer : public ::testing::Test
         std::vector<opentelemetry::sdk::metrics::PointDataAttributes>{
             {opentelemetry::sdk::metrics::PointAttributes{{"a1", "b1"}}, sum_point_data},
             {opentelemetry::sdk::metrics::PointAttributes{{"a2", "b2"}}, sum_point_data2}}};
+
     data.scope_metric_data_ = std::vector<opentelemetry::sdk::metrics::ScopeMetrics>{
         {scope.get(), std::vector<opentelemetry::sdk::metrics::MetricData>{metric_data}}};
 
@@ -111,15 +115,23 @@ class OtlpFileMetricExporterTestPeer : public ::testing::Test
     output.flush();
     output.sync();
     auto check_json_text = output.str();
+
     if (!check_json_text.empty())
     {
       auto check_json = nlohmann::json::parse(check_json_text, nullptr, false);
 
       auto resource_metrics = *check_json["resourceMetrics"].begin();
       auto scope_metrics    = *resource_metrics["scopeMetrics"].begin();
       auto scope            = scope_metrics["scope"];
+
+      EXPECT_EQ("resource_url", resource_metrics["schemaUrl"].get<std::string>());
       EXPECT_EQ("library_name", scope["name"].get<std::string>());
       EXPECT_EQ("1.5.0", scope["version"].get<std::string>());
+      EXPECT_EQ("scope_url", scope_metrics["schemaUrl"].get<std::string>());
+      ASSERT_EQ(1, scope["attributes"].size());
+      const auto scope_attribute = scope["attributes"].front();
+      EXPECT_EQ("scope_key", scope_attribute["key"].get<std::string>());
+      EXPECT_EQ("scope_value", scope_attribute["value"]["stringValue"].get<std::string>());
 
       auto metric = *scope_metrics["metrics"].begin();
       EXPECT_EQ("metrics_library_name", metric["name"].get<std::string>());

exporters/otlp/test/otlp_metrics_serialization_test.cc

@@ -13,14 +13,17 @@
 #include "opentelemetry/common/timestamp.h"
 #include "opentelemetry/exporters/otlp/otlp_metric_utils.h"
 #include "opentelemetry/exporters/otlp/otlp_preferred_temporality.h"
+#include "opentelemetry/sdk/instrumentationscope/instrumentation_scope.h"
 #include "opentelemetry/sdk/metrics/data/metric_data.h"
 #include "opentelemetry/sdk/metrics/data/point_data.h"
 #include "opentelemetry/sdk/metrics/export/metric_producer.h"
 #include "opentelemetry/sdk/metrics/instruments.h"
+#include "opentelemetry/sdk/resource/resource.h"
 #include "opentelemetry/version.h"
 
 // clang-format off
 #include "opentelemetry/exporters/otlp/protobuf_include_prefix.h" // IWYU pragma: keep
+#include "opentelemetry/proto/collector/metrics/v1/metrics_service.pb.h"
 #include "opentelemetry/proto/common/v1/common.pb.h"
 #include "opentelemetry/proto/metrics/v1/metrics.pb.h"
 #include "opentelemetry/exporters/otlp/protobuf_include_suffix.h" // IWYU pragma: keep
@@ -302,6 +305,41 @@ TEST(OtlpMetricSerializationTest, ObservableUpDownCounter)
   EXPECT_EQ(1, 1);
 }
 
+TEST(OtlpMetricSerializationTest, PopulateExportMetricsServiceRequest)
+{
+  const auto resource =
+      resource::Resource::Create({{"service.name", "test_service_name"}}, "resource_schema_url");
+  const auto scope = opentelemetry::sdk::instrumentationscope::InstrumentationScope::Create(
+      "scope_name", "scope_version", "scope_schema_url", {{"scope_key", "scope_value"}});
+
+  metrics_sdk::ScopeMetrics scope_metrics{scope.get(), CreateSumAggregationData()};
+  metrics_sdk::ResourceMetrics resource_metrics{&resource, scope_metrics};
+
+  proto::collector::metrics::v1::ExportMetricsServiceRequest request_proto;
+  otlp_exporter::OtlpMetricUtils::PopulateRequest(resource_metrics, &request_proto);
+
+  ASSERT_EQ(1, request_proto.resource_metrics_size());
+  const auto &resource_metrics_proto = request_proto.resource_metrics(0);
+  EXPECT_EQ("resource_schema_url", resource_metrics_proto.schema_url());
+
+  ASSERT_EQ(1, resource_metrics_proto.scope_metrics_size());
+  const auto &scope_metrics_proto = resource_metrics_proto.scope_metrics(0);
+  EXPECT_EQ("scope_schema_url", scope_metrics_proto.schema_url());
+
+  ASSERT_EQ(1, scope_metrics_proto.metrics_size());
+  const auto &metric_proto = scope_metrics_proto.metrics(0);
+  EXPECT_EQ("Counter", metric_proto.name());
+
+  const auto &scope_proto = scope_metrics_proto.scope();
+  EXPECT_EQ("scope_name", scope_proto.name());
+  EXPECT_EQ("scope_version", scope_proto.version());
+
+  ASSERT_EQ(1, scope_proto.attributes_size());
+  const auto &scope_attributes_proto = scope_proto.attributes(0);
+  EXPECT_EQ("scope_key", scope_attributes_proto.key());
+  EXPECT_EQ("scope_value", scope_attributes_proto.value().string_value());
+}
+
 }  // namespace otlp
 }  // namespace exporter
 OPENTELEMETRY_END_NAMESPACE

exporters/otlp/test/otlp_recordable_test.cc

@@ -119,6 +119,28 @@ TEST(OtlpRecordable, SetInstrumentationLibraryWithSchemaURL)
   EXPECT_EQ(expected_schema_url, rec.GetInstrumentationLibrarySchemaURL());
 }
 
+TEST(OtlpRecordable, SetInstrumentationScopeWithAttributes)
+{
+  exporter::otlp::OtlpRecordable rec;
+
+  auto inst_lib = trace_sdk::InstrumentationScope::Create(
+      "test_scope_name", "test_version", "test_schema_url", {{"test_key", "test_value"}});
+
+  ASSERT_EQ(inst_lib->GetAttributes().size(), 1);
+
+  rec.SetInstrumentationScope(*inst_lib);
+
+  const auto proto_instr_libr = rec.GetProtoInstrumentationScope();
+  EXPECT_EQ("test_scope_name", proto_instr_libr.name());
+  EXPECT_EQ("test_version", proto_instr_libr.version());
+
+  ASSERT_EQ(proto_instr_libr.attributes_size(), 1);
+  const auto &proto_attributes = proto_instr_libr.attributes(0);
+  ASSERT_TRUE(proto_attributes.value().has_string_value());
+  EXPECT_EQ("test_key", proto_attributes.key());
+  EXPECT_EQ("test_value", proto_attributes.value().string_value());
+}
+
 TEST(OtlpRecordable, SetStartTime)
 {
   OtlpRecordable rec;
@@ -324,7 +346,8 @@ TEST(OtlpRecordable, PopulateRequest)
   auto rec1      = std::unique_ptr<sdk::trace::Recordable>(new OtlpRecordable);
   auto resource1 = resource::Resource::Create({{"service.name", "one"}});
   rec1->SetResource(resource1);
-  auto inst_lib1 = trace_sdk::InstrumentationScope::Create("one", "1");
+  auto inst_lib1 = trace_sdk::InstrumentationScope::Create("one", "1", "scope_schema",
+                                                           {{"scope_key", "scope_value"}});
   rec1->SetInstrumentationScope(*inst_lib1);
 
   auto rec2      = std::unique_ptr<sdk::trace::Recordable>(new OtlpRecordable);
@@ -350,12 +373,23 @@ TEST(OtlpRecordable, PopulateRequest)
   EXPECT_EQ(req.resource_spans().size(), 2);
   for (const auto &resource_spans : req.resource_spans())
   {
-    auto service_name     = resource_spans.resource().attributes(0).value().string_value();
-    auto scope_spans_size = resource_spans.scope_spans().size();
+    ASSERT_GT(resource_spans.resource().attributes_size(), 0);
+    const auto service_name     = resource_spans.resource().attributes(0).value().string_value();
+    const auto scope_spans_size = resource_spans.scope_spans().size();
     if (service_name == "one")
     {
+      ASSERT_GT(resource_spans.scope_spans_size(), 0);
+      const auto &scope_one = resource_spans.scope_spans(0).scope();
+
       EXPECT_EQ(scope_spans_size, 1);
-      EXPECT_EQ(resource_spans.scope_spans(0).scope().name(), "one");
+      EXPECT_EQ(scope_one.name(), "one");
+      EXPECT_EQ(scope_one.version(), "1");
+
+      ASSERT_EQ(scope_one.attributes_size(), 1);
+      const auto &scope_attribute = scope_one.attributes(0);
+
+      EXPECT_EQ(scope_attribute.key(), "scope_key");
+      EXPECT_EQ(scope_attribute.value().string_value(), "scope_value");
     }
     if (service_name == "two")
     {