Set opentelemetry span name in transformation filter (#368)

* Add setOperation patch

* Add changelog

* Transformation initial implementation

Should work but does not seem to at the moment

* Implement unit test for setting span name

* Update changelog

* Remove extra entry from changelog

* Do not override route.decorator.operation

* Add test for null route in callbacks

* Remove redundant call to setOperation

This is the default value that is set from the HCM - we do not need to write it
a second time.

api/envoy/config/filter/http/transformation/v2/transformation_filter.proto

@@ -320,6 +320,16 @@ message TransformationTemplate {
  // for more information.
  string string_delimiter = 14;
 
+ message SpanTransformer {
+ // A template that sets the span name
+ InjaTemplate name = 1;
+
+ // TODO if we want to set attributes as well, add fields to modify them here.
+ }
+
+ // Use this field to modify the span of the trace.
+ SpanTransformer span_transformer = 15;
+
 }
 
 // Defines an [Inja template](https://github.com/pantor/inja) that will be

changelog/v1.30.6-patch3/opentelemetry-set-operation-patch.yaml

@@ -0,0 +1,6 @@
+changelog:
+- type: NEW_FEATURE
+ issueLink: https://github.com/solo-io/gloo/issues/9848
+ resolvesIssue: false
+ description: >-
+ Update the transformation filter to allow setting the tracer's span name.

source/extensions/filters/http/transformation/inja_transformer.cc

@@ -833,6 +833,14 @@ InjaTransformer::InjaTransformer(const TransformationTemplate &transformation,
  }
  }
 
+ if (transformation.has_span_transformer() && transformation.span_transformer().has_name()) {
+ try {
+ span_name_template_.emplace(instance_->parse(transformation.span_transformer().name().text()));
+ } catch (const std::exception &e) {
+ throw EnvoyException(
+ fmt::format("Failed to parse span name template {}", e.what()));
+ }
+ }
 }
 
 InjaTransformer::~InjaTransformer() {}
@@ -980,7 +988,6 @@ void InjaTransformer::transform(Http::RequestOrResponseHeaderMap &header_map,
  typed_tls_data.endpoint_metadata_ = endpoint_metadata;
  typed_tls_data.metadata_string_delimiter_ = metadata_string_delimiter_;
 
-
  // Body transform:
  absl::optional<Buffer::OwnedImpl> maybe_body;
 
@@ -1063,6 +1070,18 @@ void InjaTransformer::transform(Http::RequestOrResponseHeaderMap &header_map,
  }
  }
 
+ // Span transform:
+ if (span_name_template_.has_value()) {
+ // If route.decorator.operation is set, do not update the span name.
+ bool route_has_decorator_operation = callbacks.route()
+ && callbacks.route()->decorator()
+ && !callbacks.route()->decorator()->getOperation().empty();
+ if (!route_has_decorator_operation) {
+ std::string output = instance_->render(span_name_template_.value());
+ callbacks.activeSpan().setOperation(output);
+ }
+ }
+
  // replace body. we do it here so that headers and dynamic metadata have the
  // original body.
  if (maybe_body.has_value()) {

source/extensions/filters/http/transformation/inja_transformer.h

@@ -155,6 +155,7 @@ class InjaTransformer : public Transformer, Logger::Loggable<Logger::Id::filter>
  bool escape_characters_{};
 
  absl::optional<inja::Template> body_template_;
+ absl::optional<inja::Template> span_name_template_;
  bool merged_extractors_to_body_{};
  // merged_templates_ is a vector of tuples with the following fields:
  // 1. The json path to merge the template into

test/extensions/filters/http/transformation/BUILD

@@ -25,6 +25,7 @@ envoy_gloo_cc_test(
  "@envoy//test/mocks/http:http_mocks",
  "@envoy//test/mocks/server:server_mocks",
  "@envoy//test/mocks/upstream:upstream_mocks",
+ "@envoy//test/mocks/tracing:tracing_mocks",
  ],
 )
 

test/extensions/filters/http/transformation/inja_transformer_test.cc

@@ -7,6 +7,7 @@
 #include "test/mocks/http/mocks.h"
 #include "test/mocks/server/mocks.h"
 #include "test/mocks/thread_local/mocks.h"
+#include "test/mocks/tracing/mocks.h"
 #include "test/mocks/upstream/mocks.h"
 #include "test/test_common/environment.h"
 
@@ -1237,6 +1238,85 @@ TEST_F(InjaTransformerTest, ParseFromClusterMetadata) {
  EXPECT_EQ(body.toString(), "val");
 }
 
+TEST_F(InjaTransformerTest, SetSpanNameNullRoute) {
+ std::string transformer_span_name = "TRANSFORMER_SPAN_NAME";
+ TransformationTemplate transformation;
+ transformation.mutable_span_transformer()->mutable_name()->set_text(transformer_span_name);
+
+ Http::TestRequestHeaderMapImpl headers{};
+ Buffer::OwnedImpl body("");
+ NiceMock<Http::MockStreamDecoderFilterCallbacks> callbacks;
+
+ InjaTransformer transformer(transformation, rng_, google::protobuf::BoolValue(), tls_);
+ std::unique_ptr<Tracing::MockSpan> mock_span = std::make_unique<Tracing::MockSpan>();
+ const std::unique_ptr<Router::MockDecorator> mock_decorator = std::make_unique<NiceMock<Router::MockDecorator>>();
+ ON_CALL(callbacks, route).WillByDefault(Return(nullptr));
+ EXPECT_CALL(callbacks, activeSpan).WillOnce(ReturnRef(*mock_span));
+ EXPECT_CALL(*mock_span, setOperation(transformer_span_name)).Times(1);
+
+ transformer.transform(headers, &headers, body, callbacks);
+}
+
+TEST_F(InjaTransformerTest, SetSpanNameNullRouteDecorator) {
+ std::string transformer_span_name = "TRANSFORMER_SPAN_NAME";
+ TransformationTemplate transformation;
+ transformation.mutable_span_transformer()->mutable_name()->set_text(transformer_span_name);
+
+ Http::TestRequestHeaderMapImpl headers{};
+ Buffer::OwnedImpl body("");
+ NiceMock<Http::MockStreamDecoderFilterCallbacks> callbacks;
+
+ InjaTransformer transformer(transformation, rng_, google::protobuf::BoolValue(), tls_);
+ std::unique_ptr<Tracing::MockSpan> mock_span = std::make_unique<Tracing::MockSpan>();
+ const std::unique_ptr<Router::MockDecorator> mock_decorator = std::make_unique<NiceMock<Router::MockDecorator>>();
+ ON_CALL(*callbacks.route_, decorator).WillByDefault(Return(nullptr));
+ EXPECT_CALL(callbacks, activeSpan).WillOnce(ReturnRef(*mock_span));
+ EXPECT_CALL(*mock_span, setOperation(transformer_span_name)).Times(1);
+
+ transformer.transform(headers, &headers, body, callbacks);
+}
+
+TEST_F(InjaTransformerTest, SetSpanNameEmptyRouteDecorator) {
+ std::string transformer_span_name = "TRANSFORMER_SPAN_NAME";
+ TransformationTemplate transformation;
+ transformation.mutable_span_transformer()->mutable_name()->set_text(transformer_span_name);
+
+ Http::TestRequestHeaderMapImpl headers{};
+ Buffer::OwnedImpl body("");
+ NiceMock<Http::MockStreamDecoderFilterCallbacks> callbacks;
+
+ InjaTransformer transformer(transformation, rng_, google::protobuf::BoolValue(), tls_);
+ std::unique_ptr<Tracing::MockSpan> mock_span = std::make_unique<Tracing::MockSpan>();
+ const std::unique_ptr<Router::MockDecorator> mock_decorator = std::make_unique<NiceMock<Router::MockDecorator>>();
+ EXPECT_CALL(*callbacks.route_, decorator).WillRepeatedly(Return(mock_decorator.get()));
+ ON_CALL(*mock_decorator, getOperation()).WillByDefault(ReturnRef(""));
+ EXPECT_CALL(callbacks, activeSpan).WillOnce(ReturnRef(*mock_span));
+ EXPECT_CALL(*mock_span, setOperation(transformer_span_name)).Times(1);
+
+ transformer.transform(headers, &headers, body, callbacks);
+}
+
+TEST_F(InjaTransformerTest, SetSpanNameNonEmptyRouteDecorator) {
+ // Ensure that if route->decorator->operation is set, that it overrides the
+ // transformer value
+ std::string transformer_span_name = "TRANSFORMER_SPAN_NAME";
+ std::string decorator_span_Name = "DECORATOR_SPAN_NAME";
+ TransformationTemplate transformation;
+ transformation.mutable_span_transformer()->mutable_name()->set_text(transformer_span_name);
+
+ Http::TestRequestHeaderMapImpl headers{};
+ Buffer::OwnedImpl body("");
+ NiceMock<Http::MockStreamDecoderFilterCallbacks> callbacks;
+
+ InjaTransformer transformer(transformation, rng_, google::protobuf::BoolValue(), tls_);
+ const std::unique_ptr<Router::MockDecorator> mock_decorator = std::make_unique<NiceMock<Router::MockDecorator>>();
+ EXPECT_CALL(*callbacks.route_, decorator).WillRepeatedly(Return(mock_decorator.get()));
+ ON_CALL(*mock_decorator, getOperation()).WillByDefault(ReturnRef(decorator_span_Name));
+ EXPECT_CALL(callbacks, activeSpan).Times(0);
+
+ transformer.transform(headers, &headers, body, callbacks);
+}
+
 const std::string NESTED_KEY =
  R"EOF(
 {
@@ -1246,7 +1326,7 @@ const std::string NESTED_KEY =
 }
 )EOF";
 
-TEST_F(InjaTransformerTest, ParseFromDynamicmeMetadata) {
+TEST_F(InjaTransformerTest, ParseFromDynamicMetadata) {
  Http::TestRequestHeaderMapImpl headers{{":method", "GET"}, {":path", "/foo"}};
  TransformationTemplate transformation;
  transformation.mutable_body()->set_text("{{dynamic_metadata(\"key:value\")}}");
@@ -1280,7 +1360,7 @@ const std::string NESTED_LIST =
 }
 )EOF";
 
-TEST_F(InjaTransformerTest, ParseFromDynamicmeMetadataList) {
+TEST_F(InjaTransformerTest, ParseFromDynamicMetadataList) {
  Http::TestRequestHeaderMapImpl headers{{":method", "GET"}, {":path", "/foo"}};
  TransformationTemplate transformation;
  transformation.mutable_body()->set_text("{{dynamic_metadata(\"key:value\")}}");