From 6218a24a472ae11af4b31399cd09b8ad7135bd99 Mon Sep 17 00:00:00 2001 From: Doug Barker Date: Sat, 7 Dec 2024 05:06:53 -0700 Subject: [PATCH] [EXPORTER] add instrumentation scope attributes to otlp proto messages for traces and metrics (#3185) --- CHANGELOG.md | 3 ++ .../otlp/otlp_populate_attribute_utils.h | 6 +++ exporters/otlp/src/otlp_metric_utils.cc | 6 ++- .../otlp/src/otlp_populate_attribute_utils.cc | 12 ++++++ exporters/otlp/src/otlp_recordable.cc | 1 + exporters/otlp/src/otlp_recordable_utils.cc | 9 ++-- .../otlp/test/otlp_file_exporter_test.cc | 35 ++++++++++++---- .../test/otlp_file_metric_exporter_test.cc | 18 ++++++-- .../test/otlp_metrics_serialization_test.cc | 38 +++++++++++++++++ exporters/otlp/test/otlp_recordable_test.cc | 42 +++++++++++++++++-- 10 files changed, 149 insertions(+), 21 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2025c36b59..22176df917 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_populate_attribute_utils.h b/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_populate_attribute_utils.h index 499aa3685f..441d1d3c58 100644 --- a/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_populate_attribute_utils.h +++ b/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; diff --git a/exporters/otlp/src/otlp_metric_utils.cc b/exporters/otlp/src/otlp_metric_utils.cc index c8502d65c5..521a7e2f10 100644 --- a/exporters/otlp/src/otlp_metric_utils.cc +++ b/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_) { diff --git a/exporters/otlp/src/otlp_populate_attribute_utils.cc b/exporters/otlp/src/otlp_populate_attribute_utils.cc index 490e4c771b..1388b73823 100644 --- a/exporters/otlp/src/otlp_populate_attribute_utils.cc +++ b/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 diff --git a/exporters/otlp/src/otlp_recordable.cc b/exporters/otlp/src/otlp_recordable.cc index 356472fb48..0cf3dbeb08 100644 --- a/exporters/otlp/src/otlp_recordable.cc +++ b/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; } diff --git a/exporters/otlp/src/otlp_recordable_utils.cc b/exporters/otlp/src/otlp_recordable_utils.cc index df29cca838..1be27204c7 100644 --- a/exporters/otlp/src/otlp_recordable_utils.cc +++ b/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()); } diff --git a/exporters/otlp/test/otlp_file_exporter_test.cc b/exporters/otlp/test/otlp_file_exporter_test.cc index 575339c621..63c4c76de9 100644 --- a/exporters/otlp/test/otlp_file_exporter_test.cc +++ b/exporters/otlp/test/otlp_file_exporter_test.cc @@ -95,7 +95,7 @@ class OtlpFileExporterTestPeer : public ::testing::Test resource_attributes["vec_uint64_value"] = std::vector{7, 8}; resource_attributes["vec_double_value"] = std::vector{3.2, 3.3}; resource_attributes["vec_string_value"] = std::vector{"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(); - 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()); + EXPECT_EQ("scope_value", scope_attribute["value"]["stringValue"].get()); +#endif + EXPECT_EQ("resource_url", resource_span["schemaUrl"].get()); + EXPECT_EQ("scope_url", scope_span["schemaUrl"].get()); + EXPECT_EQ("scope_name", scope["name"].get()); + EXPECT_EQ("scope_version", scope["version"].get()); + EXPECT_EQ(report_trace_id, span["traceId"].get()); } else { diff --git a/exporters/otlp/test/otlp_file_metric_exporter_test.cc b/exporters/otlp/test/otlp_file_metric_exporter_test.cc index f79e2280b6..86cce25869 100644 --- a/exporters/otlp/test/otlp_file_metric_exporter_test.cc +++ b/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::PointAttributes{{"a1", "b1"}}, sum_point_data}, {opentelemetry::sdk::metrics::PointAttributes{{"a2", "b2"}}, sum_point_data2}}}; + data.scope_metric_data_ = std::vector{ {scope.get(), std::vector{metric_data}}}; @@ -111,6 +115,7 @@ 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); @@ -118,8 +123,15 @@ class OtlpFileMetricExporterTestPeer : public ::testing::Test 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()); EXPECT_EQ("library_name", scope["name"].get()); EXPECT_EQ("1.5.0", scope["version"].get()); + EXPECT_EQ("scope_url", scope_metrics["schemaUrl"].get()); + ASSERT_EQ(1, scope["attributes"].size()); + const auto scope_attribute = scope["attributes"].front(); + EXPECT_EQ("scope_key", scope_attribute["key"].get()); + EXPECT_EQ("scope_value", scope_attribute["value"]["stringValue"].get()); auto metric = *scope_metrics["metrics"].begin(); EXPECT_EQ("metrics_library_name", metric["name"].get()); diff --git a/exporters/otlp/test/otlp_metrics_serialization_test.cc b/exporters/otlp/test/otlp_metrics_serialization_test.cc index 9f266e5e5f..55ab80d0ab 100644 --- a/exporters/otlp/test/otlp_metrics_serialization_test.cc +++ b/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 diff --git a/exporters/otlp/test/otlp_recordable_test.cc b/exporters/otlp/test/otlp_recordable_test.cc index 9e148e60ee..0a58e52243 100644 --- a/exporters/otlp/test/otlp_recordable_test.cc +++ b/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(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(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") {