From a1edacdc8937a0702d7a47a2bc75ef01456ad3bf Mon Sep 17 00:00:00 2001 From: Ashish Banerjee Date: Thu, 19 Sep 2024 21:21:02 +0000 Subject: [PATCH 1/2] Add setOperation patch --- .../foreign_cc/0005-otel-set-operation.patch | 139 ++++++++++++++++++ bazel/repositories.bzl | 1 + 2 files changed, 140 insertions(+) create mode 100644 bazel/foreign_cc/0005-otel-set-operation.patch diff --git a/bazel/foreign_cc/0005-otel-set-operation.patch b/bazel/foreign_cc/0005-otel-set-operation.patch new file mode 100644 index 00000000..e21fa961 --- /dev/null +++ b/bazel/foreign_cc/0005-otel-set-operation.patch @@ -0,0 +1,139 @@ +diff --git a/source/extensions/tracers/opentelemetry/tracer.cc b/source/extensions/tracers/opentelemetry/tracer.cc +index 544c37ef96..df64692a37 100644 +--- source/extensions/tracers/opentelemetry/tracer.cc ++++ source/extensions/tracers/opentelemetry/tracer.cc +@@ -83,6 +83,8 @@ void Span::finishSpan() { + } + } + ++void Span::setOperation(absl::string_view operation) { span_.set_name(operation); }; ++ + void Span::injectContext(Tracing::TraceContext& trace_context, + const Upstream::HostDescriptionConstSharedPtr&) { + std::string trace_id_hex = absl::BytesToHexString(span_.trace_id()); +diff --git a/source/extensions/tracers/opentelemetry/tracer.h b/source/extensions/tracers/opentelemetry/tracer.h +index b63e54d44a69..2c9bb216b411 100644 +--- source/extensions/tracers/opentelemetry/tracer.h ++++ source/extensions/tracers/opentelemetry/tracer.h +@@ -85,7 +85,7 @@ class Span : Logger::Loggable, public Tracing::Span { + Tracer& parent_tracer, OTelSpanKind span_kind); + + // Tracing::Span functions +- void setOperation(absl::string_view /*operation*/) override{}; ++ void setOperation(absl::string_view /*operation*/) override; + void setTag(absl::string_view /*name*/, absl::string_view /*value*/) override; + void log(SystemTime /*timestamp*/, const std::string& /*event*/) override{}; + void finishSpan() override; +@@ -121,6 +121,11 @@ class Span : Logger::Loggable, public Tracing::Span { + + OTelSpanKind spankind() const { return span_.kind(); } + ++ /** ++ * @return the operation name set on the span ++ */ ++ std::string name() const { return span_.name(); } ++ + /** + * Sets the span's id. + */ +diff --git a/test/extensions/tracers/opentelemetry/BUILD b/test/extensions/tracers/opentelemetry/BUILD +index f96040354c42..c336cc9119d3 100644 +--- test/extensions/tracers/opentelemetry/BUILD ++++ test/extensions/tracers/opentelemetry/BUILD +@@ -101,3 +101,19 @@ envoy_extension_cc_test( + "//test/test_common:utility_lib", + ], + ) ++ ++envoy_extension_cc_test( ++ name = "operation_name_test", ++ srcs = ["operation_name_test.cc"], ++ extension_names = ["envoy.tracers.opentelemetry"], ++ deps = [ ++ "//source/extensions/tracers/opentelemetry:config", ++ "//source/extensions/tracers/opentelemetry:opentelemetry_tracer_lib", ++ "//test/mocks/server:tracer_factory_context_mocks", ++ "//test/mocks/stream_info:stream_info_mocks", ++ "//test/mocks/thread_local:thread_local_mocks", ++ "//test/mocks/tracing:tracing_mocks", ++ "//test/mocks/upstream:cluster_manager_mocks", ++ "//test/test_common:utility_lib", ++ ], ++) +diff --git a/test/extensions/tracers/opentelemetry/operation_name_test.cc b/test/extensions/tracers/opentelemetry/operation_name_test.cc +new file mode 100644 +index 000000000000..0d96159e4053 +--- /dev/null ++++ test/extensions/tracers/opentelemetry/operation_name_test.cc +@@ -0,0 +1,71 @@ ++#include "source/extensions/tracers/opentelemetry/config.h" ++#include "source/extensions/tracers/opentelemetry/opentelemetry_tracer_impl.h" ++#include "source/extensions/tracers/opentelemetry/tracer.h" ++ ++#include "test/mocks/server/tracer_factory_context.h" ++#include "test/mocks/stream_info/mocks.h" ++#include "test/mocks/thread_local/mocks.h" ++#include "test/mocks/tracing/mocks.h" ++#include "test/mocks/upstream/cluster_manager.h" ++#include "test/test_common/utility.h" ++ ++#include "gtest/gtest.h" ++ ++namespace Envoy { ++namespace Extensions { ++namespace Tracers { ++namespace OpenTelemetry { ++ ++class OpenTelemetryTracerOperationNameTest : public testing::Test { ++public: ++ OpenTelemetryTracerOperationNameTest(); ++ ++protected: ++ NiceMock config; ++ NiceMock stream_info; ++ Tracing::TestTraceContextImpl trace_context{}; ++ NiceMock context; ++ NiceMock cluster_manager_; ++}; ++ ++OpenTelemetryTracerOperationNameTest::OpenTelemetryTracerOperationNameTest() { ++ cluster_manager_.initializeClusters({"fake-cluster"}, {}); ++ cluster_manager_.thread_local_cluster_.cluster_.info_->name_ = "fake-cluster"; ++ cluster_manager_.initializeThreadLocalClusters({"fake-cluster"}); ++} ++ ++TEST_F(OpenTelemetryTracerOperationNameTest, OperationName) { ++ // Checks: ++ // ++ // - The span returned by `Tracer::startSpan` has as its name the ++ // operation name passed to `Tracer::startSpan` ++ // - `Span::setOperation` sets the name of the span ++ ++ const std::string yaml_string = R"EOF( ++ grpc_service: ++ envoy_grpc: ++ cluster_name: fake-cluster ++ timeout: 0.250s ++ service_name: test-service-name ++ )EOF"; ++ ++ envoy::config::trace::v3::OpenTelemetryConfig opentelemetry_config; ++ TestUtility::loadFromYaml(yaml_string, opentelemetry_config); ++ ++ auto driver = std::make_unique(opentelemetry_config, context); ++ ++ const std::string operation_name = "initial_operation_name"; ++ Tracing::SpanPtr tracing_span = driver->startSpan( ++ config, trace_context, stream_info, operation_name, {Tracing::Reason::Sampling, true}); ++ ++ EXPECT_EQ(dynamic_cast(tracing_span.get())->name(), operation_name); ++ ++ const std::string new_operation_name = "the_new_operation_name"; ++ tracing_span->setOperation(new_operation_name); ++ EXPECT_EQ(dynamic_cast(tracing_span.get())->name(), new_operation_name); ++} ++ ++} // namespace OpenTelemetry ++} // namespace Tracers ++} // namespace Extensions ++} // namespace Envoy diff --git a/bazel/repositories.bzl b/bazel/repositories.bzl index f5def418..71325e0b 100644 --- a/bazel/repositories.bzl +++ b/bazel/repositories.bzl @@ -71,6 +71,7 @@ def envoy_gloo_dependencies(): "@envoy_gloo//bazel/foreign_cc:0002-ratelimit-filter-state-hits-addend.patch", "@envoy_gloo//bazel/foreign_cc:0003-deallocate-slots-on-worker-threads.patch", # https://github.com/envoyproxy/envoy/pull/33395 "@envoy_gloo//bazel/foreign_cc:0004-local-rate-limit-bucket-backport.patch", + "@envoy_gloo//bazel/foreign_cc:0005-otel-set-operation.patch", ]) _repository_impl("json", build_file = "@envoy_gloo//bazel/external:json.BUILD") _repository_impl("inja", build_file = "@envoy_gloo//bazel/external:inja.BUILD") From 74fb9ce7d7a64618d26ae3b9c811067b603967b3 Mon Sep 17 00:00:00 2001 From: Ashish Banerjee Date: Thu, 26 Sep 2024 11:50:20 -0400 Subject: [PATCH 2/2] Add changelog --- .../v1.30.6-patch2/opentelemetry-set-operation-patch.yaml | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 changelog/v1.30.6-patch2/opentelemetry-set-operation-patch.yaml diff --git a/changelog/v1.30.6-patch2/opentelemetry-set-operation-patch.yaml b/changelog/v1.30.6-patch2/opentelemetry-set-operation-patch.yaml new file mode 100644 index 00000000..184b8b1d --- /dev/null +++ b/changelog/v1.30.6-patch2/opentelemetry-set-operation-patch.yaml @@ -0,0 +1,6 @@ +changelog: +- type: FIX + issueLink: https://github.com/solo-io/gloo/issues/9848 + resolvesIssue: false + description: >- + Backport a patch that allows setting the span name for OpenTelemetry traces.