diff --git a/docs/root/version_history/current.rst b/docs/root/version_history/current.rst index 35ebd80af2..ad7a5d551a 100644 --- a/docs/root/version_history/current.rst +++ b/docs/root/version_history/current.rst @@ -47,6 +47,7 @@ Bug Fixes * upstream: cluster slow start config add ``min_weight_percent`` field to avoid too big EDF deadline which cause slow start endpoints receiving no traffic, default 10%. This fix is releted to `issue#19526 `_. * upstream: fix stack overflow when a cluster with large number of idle connections is removed. * xray: fix the AWS X-Ray tracer extension to not sample the trace if ``sampled=`` keyword is not present in the header ``x-amzn-trace-id``. +* xray: fix the AWS X-Ray tracer extension to annotate a child span with ``type=subsegment`` to correctly relate subsegments to a parent segment. Previously a subsegment would be treated as an independent segment. Removed Config or Runtime ------------------------- diff --git a/source/extensions/tracers/xray/daemon.proto b/source/extensions/tracers/xray/daemon.proto index 2054bced76..02c853cb47 100644 --- a/source/extensions/tracers/xray/daemon.proto +++ b/source/extensions/tracers/xray/daemon.proto @@ -43,6 +43,9 @@ message Segment { } // Object containing one or more fields that X-Ray indexes for use with filter expressions. map annotations = 8; + // Set type to "subsegment" when sending a child span so X-Ray treats it as a subsegment. + // https://docs.aws.amazon.com/xray/latest/devguide/xray-api-segmentdocuments.html#api-segmentdocuments-subsegments + string type = 14; } message Header { diff --git a/source/extensions/tracers/xray/tracer.cc b/source/extensions/tracers/xray/tracer.cc index b6bfb4da42..3cc7f770ce 100644 --- a/source/extensions/tracers/xray/tracer.cc +++ b/source/extensions/tracers/xray/tracer.cc @@ -71,7 +71,9 @@ void Span::finishSpan() { s.set_error(clientError()); s.set_fault(serverError()); s.set_throttle(isThrottled()); - + if (type() == Subsegment) { + s.set_type(std::string(Subsegment)); + } auto* aws = s.mutable_aws()->mutable_fields(); for (const auto& field : aws_metadata_) { aws->insert({field.first, field.second}); @@ -108,13 +110,14 @@ void Span::injectContext(Tracing::TraceContext& trace_context) { Tracing::SpanPtr Span::spawnChild(const Tracing::Config& config, const std::string& operation_name, Envoy::SystemTime start_time) { auto child_span = std::make_unique(time_source_, random_, broker_); - child_span->setName(name()); + child_span->setName(operation_name); child_span->setOperation(operation_name); child_span->setDirection(Tracing::HttpTracerUtility::toString(config.operationName())); child_span->setStartTime(start_time); child_span->setParentId(id()); child_span->setTraceId(traceId()); child_span->setSampled(sampled()); + child_span->setType(Subsegment); return child_span; } diff --git a/source/extensions/tracers/xray/tracer.h b/source/extensions/tracers/xray/tracer.h index f53c4ca91a..81206cc2fa 100644 --- a/source/extensions/tracers/xray/tracer.h +++ b/source/extensions/tracers/xray/tracer.h @@ -26,6 +26,7 @@ namespace Tracers { namespace XRay { constexpr auto XRayTraceHeader = "x-amzn-trace-id"; +constexpr absl::string_view Subsegment = "subsegment"; class Span : public Tracing::Span, Logger::Loggable { public: @@ -101,6 +102,12 @@ class Span : public Tracing::Span, Logger::Loggable { parent_segment_id_ = std::string(parent_segment_id); } + /** + * Sets the type of the Span. In X-Ray, an independent subsegment has a type of "subsegment". + * https://docs.aws.amazon.com/xray/latest/devguide/xray-api-segmentdocuments.html#api-segmentdocuments-subsegments + */ + void setType(absl::string_view type) { type_ = std::string(type); } + /** * Sets the aws metadata field of the Span. */ @@ -156,6 +163,11 @@ class Span : public Tracing::Span, Logger::Loggable { */ const std::string& direction() const { return direction_; } + /** + * Gets this Span's type. + */ + const std::string& type() const { return type_; } + /** * Gets this Span's name. */ @@ -216,6 +228,7 @@ class Span : public Tracing::Span, Logger::Loggable { std::string parent_segment_id_; std::string name_; std::string origin_; + std::string type_; absl::flat_hash_map aws_metadata_; absl::flat_hash_map http_request_annotations_; absl::flat_hash_map http_response_annotations_; diff --git a/test/extensions/tracers/xray/tracer_test.cc b/test/extensions/tracers/xray/tracer_test.cc index f77d28e407..6fa8cfb430 100644 --- a/test/extensions/tracers/xray/tracer_test.cc +++ b/test/extensions/tracers/xray/tracer_test.cc @@ -97,6 +97,7 @@ TEST_F(XRayTracerTest, SerializeSpanTest) { EXPECT_FALSE(s.id().empty()); EXPECT_EQ(2, s.annotations().size()); EXPECT_TRUE(s.parent_id().empty()); + EXPECT_TRUE(s.type().empty()); EXPECT_FALSE(s.fault()); /*server error*/ EXPECT_FALSE(s.error()); /*client error*/ EXPECT_FALSE(s.throttle()); /*request throttled*/ @@ -142,6 +143,7 @@ TEST_F(XRayTracerTest, SerializeSpanTestServerError) { EXPECT_FALSE(s.trace_id().empty()); EXPECT_FALSE(s.id().empty()); EXPECT_TRUE(s.parent_id().empty()); + EXPECT_TRUE(s.type().empty()); EXPECT_TRUE(s.fault()); /*server error*/ EXPECT_FALSE(s.error()); /*client error*/ EXPECT_EQ(expected_status_code, @@ -175,6 +177,7 @@ TEST_F(XRayTracerTest, SerializeSpanTestClientError) { EXPECT_FALSE(s.trace_id().empty()); EXPECT_FALSE(s.id().empty()); EXPECT_TRUE(s.parent_id().empty()); + EXPECT_TRUE(s.type().empty()); EXPECT_FALSE(s.fault()); /*server error*/ EXPECT_TRUE(s.error()); /*client error*/ EXPECT_FALSE(s.throttle()); /*request throttled*/ @@ -208,6 +211,7 @@ TEST_F(XRayTracerTest, SerializeSpanTestClientErrorWithThrottle) { EXPECT_FALSE(s.trace_id().empty()); EXPECT_FALSE(s.id().empty()); EXPECT_TRUE(s.parent_id().empty()); + EXPECT_TRUE(s.type().empty()); EXPECT_FALSE(s.fault()); /*server error*/ EXPECT_TRUE(s.error()); /*client error*/ EXPECT_TRUE(s.throttle()); /*request throttled*/ @@ -239,6 +243,7 @@ TEST_F(XRayTracerTest, SerializeSpanTestWithEmptyValue) { EXPECT_FALSE(s.trace_id().empty()); EXPECT_FALSE(s.id().empty()); EXPECT_TRUE(s.parent_id().empty()); + EXPECT_TRUE(s.type().empty()); EXPECT_FALSE(s.http().request().fields().contains(Tracing::Tags::get().Status)); }; @@ -270,6 +275,7 @@ TEST_F(XRayTracerTest, SerializeSpanTestWithStatusCodeNotANumber) { EXPECT_FALSE(s.trace_id().empty()); EXPECT_FALSE(s.id().empty()); EXPECT_TRUE(s.parent_id().empty()); + EXPECT_TRUE(s.type().empty()); EXPECT_FALSE(s.http().request().fields().contains(Tracing::Tags::get().Status)); EXPECT_FALSE(s.http().request().fields().contains("content_length")); }; @@ -346,7 +352,9 @@ TEST_F(XRayTracerTest, ChildSpanHasParentInfo) { TestUtility::validate(s); // Hex encoded 64 bit identifier EXPECT_STREQ("00000000000003e7", s.parent_id().c_str()); - EXPECT_EQ(expected_->span_name, s.name().c_str()); + EXPECT_EQ(expected_->operation_name, s.name().c_str()); + EXPECT_TRUE(xray_parent_span->type().empty()); + EXPECT_EQ(Subsegment, s.type().c_str()); EXPECT_STREQ(xray_parent_span->traceId().c_str(), s.trace_id().c_str()); EXPECT_STREQ("0000003d25bebe62", s.id().c_str()); }; diff --git a/tools/spelling/spelling_dictionary.txt b/tools/spelling/spelling_dictionary.txt index 48a8222002..8c30997bef 100644 --- a/tools/spelling/spelling_dictionary.txt +++ b/tools/spelling/spelling_dictionary.txt @@ -1177,6 +1177,7 @@ subnets suboptimal subsecond subseconds +subsegment subsequence subsetting substr