From 2da74beb96257539dadaf049298a6b8e0f72437a Mon Sep 17 00:00:00 2001 From: Yash Tibrewal Date: Mon, 28 Aug 2023 17:13:01 -0700 Subject: [PATCH] [OTel] Remove authority attribute from server metrics (#34189) Based on updates at https://github.com/grpc/proposal/pull/380 --- src/cpp/ext/otel/otel_plugin.cc | 2 -- src/cpp/ext/otel/otel_plugin.h | 1 - src/cpp/ext/otel/otel_server_call_tracer.cc | 6 ++---- test/cpp/ext/otel/otel_plugin_test.cc | 24 ++++----------------- 4 files changed, 6 insertions(+), 27 deletions(-) diff --git a/src/cpp/ext/otel/otel_plugin.cc b/src/cpp/ext/otel/otel_plugin.cc index b07b830662a..2dea43697a5 100644 --- a/src/cpp/ext/otel/otel_plugin.cc +++ b/src/cpp/ext/otel/otel_plugin.cc @@ -57,8 +57,6 @@ absl::string_view OTelStatusKey() { return "grpc.status"; } absl::string_view OTelTargetKey() { return "grpc.target"; } -absl::string_view OTelAuthorityKey() { return "grpc.authority"; } - absl::string_view OTelClientAttemptStartedInstrumentName() { return "grpc.client.attempt.started"; } diff --git a/src/cpp/ext/otel/otel_plugin.h b/src/cpp/ext/otel/otel_plugin.h index f0af959e8d1..b0cd5a4c7a8 100644 --- a/src/cpp/ext/otel/otel_plugin.h +++ b/src/cpp/ext/otel/otel_plugin.h @@ -66,7 +66,6 @@ const struct OTelPluginState& OTelPluginState(); absl::string_view OTelMethodKey(); absl::string_view OTelStatusKey(); absl::string_view OTelTargetKey(); -absl::string_view OTelAuthorityKey(); // Metrics absl::string_view OTelClientAttemptStartedInstrumentName(); diff --git a/src/cpp/ext/otel/otel_server_call_tracer.cc b/src/cpp/ext/otel/otel_server_call_tracer.cc index 41728b23f92..aa7c45d362f 100644 --- a/src/cpp/ext/otel/otel_server_call_tracer.cc +++ b/src/cpp/ext/otel/otel_server_call_tracer.cc @@ -146,8 +146,7 @@ void OpenTelemetryServerCallTracer::RecordReceivedInitialMetadata( // TODO(yashykt): Figure out how to get this to work with absl::string_view if (OTelPluginState().server.call.started != nullptr) { OTelPluginState().server.call.started->Add( - 1, {{std::string(OTelMethodKey()), std::string(method_)}, - {std::string(OTelAuthorityKey()), authority_}}); + 1, {{std::string(OTelMethodKey()), std::string(method_)}}); } } @@ -164,8 +163,7 @@ void OpenTelemetryServerCallTracer::RecordEnd( {std::string(OTelMethodKey()), std::string(method_)}, {std::string(OTelStatusKey()), absl::StatusCodeToString( - static_cast(final_info->final_status))}, - {std::string(OTelAuthorityKey()), authority_}}; + static_cast(final_info->final_status))}}; if (OTelPluginState().server.call.duration != nullptr) { OTelPluginState().server.call.duration->Record( absl::ToDoubleSeconds(elapsed_time_), attributes, diff --git a/test/cpp/ext/otel/otel_plugin_test.cc b/test/cpp/ext/otel/otel_plugin_test.cc index 8030b5a7f86..1248b1e59bf 100644 --- a/test/cpp/ext/otel/otel_plugin_test.cc +++ b/test/cpp/ext/otel/otel_plugin_test.cc @@ -303,15 +303,11 @@ TEST_F(OTelPluginEnd2EndTest, ServerCallStarted) { ASSERT_NE(server_started_value, nullptr); ASSERT_EQ(*server_started_value, 1); const auto& attributes = data[kMetricName][0].attributes.GetAttributes(); - EXPECT_EQ(attributes.size(), 2); + EXPECT_EQ(attributes.size(), 1); const auto* method_value = absl::get_if(&attributes.at("grpc.method")); ASSERT_NE(method_value, nullptr); EXPECT_EQ(*method_value, kMethodName); - const auto* authority_value = - absl::get_if(&attributes.at("grpc.authority")); - ASSERT_NE(authority_value, nullptr); - EXPECT_EQ(*authority_value, server_address_); } TEST_F(OTelPluginEnd2EndTest, ServerCallDuration) { @@ -330,15 +326,11 @@ TEST_F(OTelPluginEnd2EndTest, ServerCallDuration) { ASSERT_NE(point_data, nullptr); ASSERT_EQ(point_data->count_, 1); const auto& attributes = data[kMetricName][0].attributes.GetAttributes(); - EXPECT_EQ(attributes.size(), 3); + EXPECT_EQ(attributes.size(), 2); const auto* method_value = absl::get_if(&attributes.at("grpc.method")); ASSERT_NE(method_value, nullptr); EXPECT_EQ(*method_value, kMethodName); - const auto* authority_value = - absl::get_if(&attributes.at("grpc.authority")); - ASSERT_NE(authority_value, nullptr); - EXPECT_EQ(*authority_value, server_address_); const auto* status_value = absl::get_if(&attributes.at("grpc.status")); ASSERT_NE(status_value, nullptr); @@ -363,15 +355,11 @@ TEST_F(OTelPluginEnd2EndTest, ServerCallSentTotalCompressedMessageSize) { ASSERT_NE(point_data, nullptr); EXPECT_EQ(point_data->count_, 1); const auto& attributes = data[kMetricName][0].attributes.GetAttributes(); - EXPECT_EQ(attributes.size(), 3); + EXPECT_EQ(attributes.size(), 2); const auto* method_value = absl::get_if(&attributes.at("grpc.method")); ASSERT_NE(method_value, nullptr); EXPECT_EQ(*method_value, kMethodName); - const auto* authority_value = - absl::get_if(&attributes.at("grpc.authority")); - ASSERT_NE(authority_value, nullptr); - EXPECT_EQ(*authority_value, server_address_); const auto* status_value = absl::get_if(&attributes.at("grpc.status")); ASSERT_NE(status_value, nullptr); @@ -396,15 +384,11 @@ TEST_F(OTelPluginEnd2EndTest, ServerCallRcvdTotalCompressedMessageSize) { ASSERT_NE(point_data, nullptr); ASSERT_EQ(point_data->count_, 1); const auto& attributes = data[kMetricName][0].attributes.GetAttributes(); - EXPECT_EQ(attributes.size(), 3); + EXPECT_EQ(attributes.size(), 2); const auto* method_value = absl::get_if(&attributes.at("grpc.method")); ASSERT_NE(method_value, nullptr); EXPECT_EQ(*method_value, kMethodName); - const auto* authority_value = - absl::get_if(&attributes.at("grpc.authority")); - ASSERT_NE(authority_value, nullptr); - EXPECT_EQ(*authority_value, server_address_); const auto* status_value = absl::get_if(&attributes.at("grpc.status")); ASSERT_NE(status_value, nullptr);