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

Set opentelemetry span name in transformation filter #368

Merged
merged 11 commits into from
Oct 1, 2024
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -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.
Original file line number Diff line number Diff line change
Expand Up @@ -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() {}
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions test/extensions/filters/http/transformation/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -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",
],
)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down Expand Up @@ -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>>();
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are there simpler ways to set up these mocks?

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(
{
Expand All @@ -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\")}}");
Expand Down Expand Up @@ -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\")}}");
Expand Down
Loading