Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Opentelemetry set operation patch #366

Merged
merged 3 commits into from
Sep 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
139 changes: 139 additions & 0 deletions bazel/foreign_cc/0005-otel-set-operation.patch
Original file line number Diff line number Diff line change
@@ -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<Logger::Id::tracing>, 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<Logger::Id::tracing>, 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<Tracing::MockConfig> config;
+ NiceMock<StreamInfo::MockStreamInfo> stream_info;
+ Tracing::TestTraceContextImpl trace_context{};
+ NiceMock<Server::Configuration::MockTracerFactoryContext> context;
+ NiceMock<Upstream::MockClusterManager> 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<Driver>(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<Span*>(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<Span*>(tracing_span.get())->name(), new_operation_name);
+}
+
+} // namespace OpenTelemetry
+} // namespace Tracers
+} // namespace Extensions
+} // namespace Envoy
1 change: 1 addition & 0 deletions bazel/repositories.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Original file line number Diff line number Diff line change
@@ -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.
Loading