diff --git a/api/envoy/config/filter/http/transformation/v2/transformation_filter.proto b/api/envoy/config/filter/http/transformation/v2/transformation_filter.proto index 067e2bad..04cb495c 100644 --- a/api/envoy/config/filter/http/transformation/v2/transformation_filter.proto +++ b/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 diff --git a/changelog/v1.30.6-patch3/opentelemetry-set-operation-patch.yaml b/changelog/v1.30.6-patch3/opentelemetry-set-operation-patch.yaml new file mode 100644 index 00000000..20328699 --- /dev/null +++ b/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. diff --git a/source/extensions/filters/http/transformation/inja_transformer.cc b/source/extensions/filters/http/transformation/inja_transformer.cc index b5c0947e..d1a4e557 100644 --- a/source/extensions/filters/http/transformation/inja_transformer.cc +++ b/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 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()) { diff --git a/source/extensions/filters/http/transformation/inja_transformer.h b/source/extensions/filters/http/transformation/inja_transformer.h index 20e4a89d..e5234391 100644 --- a/source/extensions/filters/http/transformation/inja_transformer.h +++ b/source/extensions/filters/http/transformation/inja_transformer.h @@ -155,6 +155,7 @@ class InjaTransformer : public Transformer, Logger::Loggable bool escape_characters_{}; absl::optional body_template_; + absl::optional 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 diff --git a/test/extensions/filters/http/transformation/BUILD b/test/extensions/filters/http/transformation/BUILD index 344cc91a..27e810ea 100644 --- a/test/extensions/filters/http/transformation/BUILD +++ b/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", ], ) diff --git a/test/extensions/filters/http/transformation/inja_transformer_test.cc b/test/extensions/filters/http/transformation/inja_transformer_test.cc index c3d5b6b9..8eb8745b 100644 --- a/test/extensions/filters/http/transformation/inja_transformer_test.cc +++ b/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 callbacks; + + InjaTransformer transformer(transformation, rng_, google::protobuf::BoolValue(), tls_); + std::unique_ptr mock_span = std::make_unique(); + const std::unique_ptr mock_decorator = std::make_unique>(); + 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 callbacks; + + InjaTransformer transformer(transformation, rng_, google::protobuf::BoolValue(), tls_); + std::unique_ptr mock_span = std::make_unique(); + const std::unique_ptr mock_decorator = std::make_unique>(); + 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 callbacks; + + InjaTransformer transformer(transformation, rng_, google::protobuf::BoolValue(), tls_); + std::unique_ptr mock_span = std::make_unique(); + const std::unique_ptr mock_decorator = std::make_unique>(); + 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 callbacks; + + InjaTransformer transformer(transformation, rng_, google::protobuf::BoolValue(), tls_); + const std::unique_ptr mock_decorator = std::make_unique>(); + 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\")}}");