Skip to content

Commit 82c02b7

Browse files
rexnpalyssawilkwu-bindependabot[bot]jmarantz
authored
xray tracer: correctly annotate child spans as xray subsegments (envoyproxy#20170)
* test: adding a multi-envoy test (envoyproxy#20016) Functionally this handles the multi-envoy signal handler crash skips instantiating a runtime singleton (off by default, must stay off until remove global runtime: rebase runtime features on ABSL_flags envoyproxy#19847 is closed) Multi-envoy does not correctly support runtime flags or deprecation stats due to envoyproxy#19847 being incomplete. It can still handle proxy traffic client - L1 - L2 - upstream as shown in test. Risk Level: low Testing: yes Docs Changes: n/a Release Notes: n/a Part of envoyproxy/envoy-mobile#2003 Signed-off-by: Alyssa Wilk <alyssar@chromium.org> Signed-off-by: Rex Chang <58710378+rexnp@users.noreply.github.com> * Add a congestionWindowInBytes method to Envoy::Network::Connection (envoyproxy#20105) Signed-off-by: Bin Wu <wub@google.com> Signed-off-by: Rex Chang <58710378+rexnp@users.noreply.github.com> * Update QUICHE from 50f15e7a5 to cf1588207 (envoyproxy#20154) https://github.com/google/quiche/compare/50f15e7a5..cf1588207 $ git log 50f15e7a5..cf1588207 --date=short --no-merges --format="%ad %al %s" 2022-02-28 wub Deprecate --gfe2_reloadable_flag_quic_crypto_noop_if_disconnected_after_process_chlo. 2022-02-27 vasilvv Remove QuicheMemSlice(QuicUniqueBufferPtr, size_t) constructor. 2022-02-26 fayang Use std::string instead of absl::string_view in CryptoBufferMap. 2022-02-25 bnc Ignore incoming HTTP/3 MAX_PUSH_ID frames. 2022-02-25 bnc Remove Http3DebugVisitor::OnMaxPushIdFrameSent(). 2022-02-25 bnc Remove QuicSpdySession::CanCreatePushStreamWithId(). 2022-02-25 fayang Deprecate gfe2_reloadable_flag_quic_single_ack_in_packet2. Signed-off-by: Alyssa Wilk <alyssar@chromium.org> Signed-off-by: Rex Chang <58710378+rexnp@users.noreply.github.com> * build(deps): bump actions/stale from 4.1.0 to 5 (envoyproxy#20159) Bumps [actions/stale](https://github.com/actions/stale) from 4.1.0 to 5. - [Release notes](https://github.com/actions/stale/releases) - [Changelog](https://github.com/actions/stale/blob/main/CHANGELOG.md) - [Commits](actions/stale@v4.1.0...v5) --- updated-dependencies: - dependency-name: actions/stale dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Signed-off-by: Rex Chang <58710378+rexnp@users.noreply.github.com> * admin: improve test coverage and increase the coverage-percent threshold (envoyproxy#20025) Adds a missing test for recent lookups now that there are no more fake symbol tables. Adds tests for a variety of override methods defined in admin.h that were previously hard to hit. Adds a benchmark test to establish a baseline for the speedups in envoyproxy#19693 Signed-off-by: Joshua Marantz <jmarantz@google.com> Signed-off-by: Rex Chang <58710378+rexnp@users.noreply.github.com> * test: removing a bunch of direct runtime singleton access (envoyproxy#19993) Signed-off-by: Alyssa Wilk <alyssar@chromium.org> Signed-off-by: Rex Chang <58710378+rexnp@users.noreply.github.com> * build(deps): bump grpcio-tools in /examples/grpc-bridge/client (envoyproxy#20040) Bumps [grpcio-tools](https://github.com/grpc/grpc) from 1.43.0 to 1.44.0. - [Release notes](https://github.com/grpc/grpc/releases) - [Changelog](https://github.com/grpc/grpc/blob/master/doc/grpc_release_schedule.md) - [Commits](grpc/grpc@v1.43.0...v1.44.0) --- updated-dependencies: - dependency-name: grpcio-tools dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Signed-off-by: Rex Chang <58710378+rexnp@users.noreply.github.com> * adds to spellcheck Signed-off-by: Rex Chang <58710378+rexnp@users.noreply.github.com> * xray tracer: set subsegment type for child spans (#2) * xray tracer: set subsegment type for child spans Signed-off-by: Rex Chang <58710378+rexnp@users.noreply.github.com> * adds test coverage Signed-off-by: Rex Chang <58710378+rexnp@users.noreply.github.com> * Xray subsegment (#3) * xray tracer: set subsegment type for child spans Signed-off-by: Rex Chang <58710378+rexnp@users.noreply.github.com> * adds test coverage Signed-off-by: Rex Chang <58710378+rexnp@users.noreply.github.com> * updates xray subsegment name to use operation name (instead of parent's span name) Signed-off-by: Rex Chang <58710378+rexnp@users.noreply.github.com> * updates doc Signed-off-by: Rex Chang <58710378+rexnp@users.noreply.github.com> * updates doc Signed-off-by: Rex Chang <58710378+rexnp@users.noreply.github.com> * Xray subsegment (#4) * xray tracer: set subsegment type for child spans Signed-off-by: Rex Chang <58710378+rexnp@users.noreply.github.com> * adds test coverage Signed-off-by: Rex Chang <58710378+rexnp@users.noreply.github.com> * updates xray subsegment name to use operation name (instead of parent's span name) Signed-off-by: Rex Chang <58710378+rexnp@users.noreply.github.com> * updates doc Signed-off-by: Rex Chang <58710378+rexnp@users.noreply.github.com> * updates doc Signed-off-by: Rex Chang <58710378+rexnp@users.noreply.github.com> * adds to spell check dictionary Signed-off-by: Rex Chang <58710378+rexnp@users.noreply.github.com> * fixes spellcheck Signed-off-by: Rex Chang <58710378+rexnp@users.noreply.github.com> * adds to spellcheck Signed-off-by: Rex Chang <58710378+rexnp@users.noreply.github.com> xray tracer: set subsegment type for child spans (#2) * xray tracer: set subsegment type for child spans Signed-off-by: Rex Chang <58710378+rexnp@users.noreply.github.com> * adds test coverage Signed-off-by: Rex Chang <58710378+rexnp@users.noreply.github.com> Xray subsegment (#3) * xray tracer: set subsegment type for child spans Signed-off-by: Rex Chang <58710378+rexnp@users.noreply.github.com> * adds test coverage Signed-off-by: Rex Chang <58710378+rexnp@users.noreply.github.com> * updates xray subsegment name to use operation name (instead of parent's span name) Signed-off-by: Rex Chang <58710378+rexnp@users.noreply.github.com> * updates doc Signed-off-by: Rex Chang <58710378+rexnp@users.noreply.github.com> * updates doc Signed-off-by: Rex Chang <58710378+rexnp@users.noreply.github.com> Xray subsegment (#4) * xray tracer: set subsegment type for child spans Signed-off-by: Rex Chang <58710378+rexnp@users.noreply.github.com> * adds test coverage Signed-off-by: Rex Chang <58710378+rexnp@users.noreply.github.com> * updates xray subsegment name to use operation name (instead of parent's span name) Signed-off-by: Rex Chang <58710378+rexnp@users.noreply.github.com> * updates doc Signed-off-by: Rex Chang <58710378+rexnp@users.noreply.github.com> * updates doc Signed-off-by: Rex Chang <58710378+rexnp@users.noreply.github.com> * adds to spell check dictionary Signed-off-by: Rex Chang <58710378+rexnp@users.noreply.github.com> fixes spellcheck Signed-off-by: Rex Chang <58710378+rexnp@users.noreply.github.com> * fixes spell check Signed-off-by: Rex Chang <58710378+rexnp@users.noreply.github.com> Co-authored-by: alyssawilk <alyssar@chromium.org> Co-authored-by: Bin Wu <46450037+wu-bin@users.noreply.github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Joshua Marantz <jmarantz@google.com>
1 parent 1945b1e commit 82c02b7

File tree

6 files changed

+32
-3
lines changed

6 files changed

+32
-3
lines changed

‎docs/root/version_history/current.rst

+1
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ Bug Fixes
4747
* 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 <https://github.com/envoyproxy/envoy/issues/19526>`_.
4848
* upstream: fix stack overflow when a cluster with large number of idle connections is removed.
4949
* 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``.
50+
* 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.
5051

5152
Removed Config or Runtime
5253
-------------------------

‎source/extensions/tracers/xray/daemon.proto

+3
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,9 @@ message Segment {
4343
}
4444
// Object containing one or more fields that X-Ray indexes for use with filter expressions.
4545
map<string, string> annotations = 8;
46+
// Set type to "subsegment" when sending a child span so X-Ray treats it as a subsegment.
47+
// https://docs.aws.amazon.com/xray/latest/devguide/xray-api-segmentdocuments.html#api-segmentdocuments-subsegments
48+
string type = 14;
4649
}
4750

4851
message Header {

‎source/extensions/tracers/xray/tracer.cc

+5-2
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,9 @@ void Span::finishSpan() {
7171
s.set_error(clientError());
7272
s.set_fault(serverError());
7373
s.set_throttle(isThrottled());
74-
74+
if (type() == Subsegment) {
75+
s.set_type(std::string(Subsegment));
76+
}
7577
auto* aws = s.mutable_aws()->mutable_fields();
7678
for (const auto& field : aws_metadata_) {
7779
aws->insert({field.first, field.second});
@@ -108,13 +110,14 @@ void Span::injectContext(Tracing::TraceContext& trace_context) {
108110
Tracing::SpanPtr Span::spawnChild(const Tracing::Config& config, const std::string& operation_name,
109111
Envoy::SystemTime start_time) {
110112
auto child_span = std::make_unique<XRay::Span>(time_source_, random_, broker_);
111-
child_span->setName(name());
113+
child_span->setName(operation_name);
112114
child_span->setOperation(operation_name);
113115
child_span->setDirection(Tracing::HttpTracerUtility::toString(config.operationName()));
114116
child_span->setStartTime(start_time);
115117
child_span->setParentId(id());
116118
child_span->setTraceId(traceId());
117119
child_span->setSampled(sampled());
120+
child_span->setType(Subsegment);
118121
return child_span;
119122
}
120123

‎source/extensions/tracers/xray/tracer.h

+13
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ namespace Tracers {
2626
namespace XRay {
2727

2828
constexpr auto XRayTraceHeader = "x-amzn-trace-id";
29+
constexpr absl::string_view Subsegment = "subsegment";
2930

3031
class Span : public Tracing::Span, Logger::Loggable<Logger::Id::config> {
3132
public:
@@ -101,6 +102,12 @@ class Span : public Tracing::Span, Logger::Loggable<Logger::Id::config> {
101102
parent_segment_id_ = std::string(parent_segment_id);
102103
}
103104

105+
/**
106+
* Sets the type of the Span. In X-Ray, an independent subsegment has a type of "subsegment".
107+
* https://docs.aws.amazon.com/xray/latest/devguide/xray-api-segmentdocuments.html#api-segmentdocuments-subsegments
108+
*/
109+
void setType(absl::string_view type) { type_ = std::string(type); }
110+
104111
/**
105112
* Sets the aws metadata field of the Span.
106113
*/
@@ -156,6 +163,11 @@ class Span : public Tracing::Span, Logger::Loggable<Logger::Id::config> {
156163
*/
157164
const std::string& direction() const { return direction_; }
158165

166+
/**
167+
* Gets this Span's type.
168+
*/
169+
const std::string& type() const { return type_; }
170+
159171
/**
160172
* Gets this Span's name.
161173
*/
@@ -216,6 +228,7 @@ class Span : public Tracing::Span, Logger::Loggable<Logger::Id::config> {
216228
std::string parent_segment_id_;
217229
std::string name_;
218230
std::string origin_;
231+
std::string type_;
219232
absl::flat_hash_map<std::string, ProtobufWkt::Value> aws_metadata_;
220233
absl::flat_hash_map<std::string, ProtobufWkt::Value> http_request_annotations_;
221234
absl::flat_hash_map<std::string, ProtobufWkt::Value> http_response_annotations_;

‎test/extensions/tracers/xray/tracer_test.cc

+9-1
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,7 @@ TEST_F(XRayTracerTest, SerializeSpanTest) {
9797
EXPECT_FALSE(s.id().empty());
9898
EXPECT_EQ(2, s.annotations().size());
9999
EXPECT_TRUE(s.parent_id().empty());
100+
EXPECT_TRUE(s.type().empty());
100101
EXPECT_FALSE(s.fault()); /*server error*/
101102
EXPECT_FALSE(s.error()); /*client error*/
102103
EXPECT_FALSE(s.throttle()); /*request throttled*/
@@ -142,6 +143,7 @@ TEST_F(XRayTracerTest, SerializeSpanTestServerError) {
142143
EXPECT_FALSE(s.trace_id().empty());
143144
EXPECT_FALSE(s.id().empty());
144145
EXPECT_TRUE(s.parent_id().empty());
146+
EXPECT_TRUE(s.type().empty());
145147
EXPECT_TRUE(s.fault()); /*server error*/
146148
EXPECT_FALSE(s.error()); /*client error*/
147149
EXPECT_EQ(expected_status_code,
@@ -175,6 +177,7 @@ TEST_F(XRayTracerTest, SerializeSpanTestClientError) {
175177
EXPECT_FALSE(s.trace_id().empty());
176178
EXPECT_FALSE(s.id().empty());
177179
EXPECT_TRUE(s.parent_id().empty());
180+
EXPECT_TRUE(s.type().empty());
178181
EXPECT_FALSE(s.fault()); /*server error*/
179182
EXPECT_TRUE(s.error()); /*client error*/
180183
EXPECT_FALSE(s.throttle()); /*request throttled*/
@@ -208,6 +211,7 @@ TEST_F(XRayTracerTest, SerializeSpanTestClientErrorWithThrottle) {
208211
EXPECT_FALSE(s.trace_id().empty());
209212
EXPECT_FALSE(s.id().empty());
210213
EXPECT_TRUE(s.parent_id().empty());
214+
EXPECT_TRUE(s.type().empty());
211215
EXPECT_FALSE(s.fault()); /*server error*/
212216
EXPECT_TRUE(s.error()); /*client error*/
213217
EXPECT_TRUE(s.throttle()); /*request throttled*/
@@ -239,6 +243,7 @@ TEST_F(XRayTracerTest, SerializeSpanTestWithEmptyValue) {
239243
EXPECT_FALSE(s.trace_id().empty());
240244
EXPECT_FALSE(s.id().empty());
241245
EXPECT_TRUE(s.parent_id().empty());
246+
EXPECT_TRUE(s.type().empty());
242247
EXPECT_FALSE(s.http().request().fields().contains(Tracing::Tags::get().Status));
243248
};
244249

@@ -270,6 +275,7 @@ TEST_F(XRayTracerTest, SerializeSpanTestWithStatusCodeNotANumber) {
270275
EXPECT_FALSE(s.trace_id().empty());
271276
EXPECT_FALSE(s.id().empty());
272277
EXPECT_TRUE(s.parent_id().empty());
278+
EXPECT_TRUE(s.type().empty());
273279
EXPECT_FALSE(s.http().request().fields().contains(Tracing::Tags::get().Status));
274280
EXPECT_FALSE(s.http().request().fields().contains("content_length"));
275281
};
@@ -346,7 +352,9 @@ TEST_F(XRayTracerTest, ChildSpanHasParentInfo) {
346352
TestUtility::validate(s);
347353
// Hex encoded 64 bit identifier
348354
EXPECT_STREQ("00000000000003e7", s.parent_id().c_str());
349-
EXPECT_EQ(expected_->span_name, s.name().c_str());
355+
EXPECT_EQ(expected_->operation_name, s.name().c_str());
356+
EXPECT_TRUE(xray_parent_span->type().empty());
357+
EXPECT_EQ(Subsegment, s.type().c_str());
350358
EXPECT_STREQ(xray_parent_span->traceId().c_str(), s.trace_id().c_str());
351359
EXPECT_STREQ("0000003d25bebe62", s.id().c_str());
352360
};

‎tools/spelling/spelling_dictionary.txt

+1
Original file line numberDiff line numberDiff line change
@@ -1177,6 +1177,7 @@ subnets
11771177
suboptimal
11781178
subsecond
11791179
subseconds
1180+
subsegment
11801181
subsequence
11811182
subsetting
11821183
substr

0 commit comments

Comments
 (0)