xray tracer: correctly annotate child spans as xray subsegments #20170

rexnp · 2022-03-02T00:46:34Z

Commit Message:
In Xray tracing, there is the concept of an xray segment and each segment can contain subsegments. The segment and subsegment maps to Envoy's parent and child spans. In the existing xray extension implementation, each span is emitted independently without the distinction whether the span is a child or not, so Xray treats each span as its own segment.

Whereas in fact, the Xray extension should annotate child spans as subsegments by setting a key value pair "type=subsegment" in the segment payload: https://docs.aws.amazon.com/xray/latest/devguide/xray-api-segmentdocuments.html#api-segmentdocuments-subsegments. This PR is to perform this annotation so Xray subsegments are correctly related to their parent segment.

Additional Description: N/A
Risk Level: low
Testing: unit tests, manual setup using https://github.com/aws/aws-app-mesh-examples/tree/main/walkthroughs/howto-ecs-basics that exercises Xray integration with Envoy.
Docs Changes: Added this change to version history.
Release Notes: Added this change to version history.
Platform Specific Features: N/A
[Optional Fixes #Issue] This will partially address aws/aws-app-mesh-roadmap#354

rexnp · 2022-03-02T01:25:12Z

CI failing in presubmit due to spell check. adding the terms Xray and subsegment to the dictionary.

rojkov · 2022-03-02T07:35:29Z

Thank you for fixing it!

BTW, you can exempt a word from spellchecking with backquotes.

/assign @abaptiste
/wait
on CI

repokitteh-read-only · 2022-03-02T18:15:57Z

CC @envoyproxy/dependency-shepherds: Your approval is needed for changes made to (bazel/.*repos.*\.bzl)|(bazel/dependency_imports\.bzl)|(api/bazel/.*\.bzl)|(.*/requirements\.txt)|(.*\.patch).
envoyproxy/dependency-shepherds assignee is @wrowe

🐱

Caused by: #20170 was synchronize by rexnp.

see: more, trace.

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>

…nvoyproxy#20105) Signed-off-by: Bin Wu <wub@google.com> Signed-off-by: Rex Chang <58710378+rexnp@users.noreply.github.com>

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>

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>

…old (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>

…#19993) Signed-off-by: Alyssa Wilk <alyssar@chromium.org> Signed-off-by: Rex Chang <58710378+rexnp@users.noreply.github.com>

…proxy#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>

Signed-off-by: Rex Chang <58710378+rexnp@users.noreply.github.com>

* 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 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 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>

Signed-off-by: Rex Chang <58710378+rexnp@users.noreply.github.com>

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>

Signed-off-by: Rex Chang <58710378+rexnp@users.noreply.github.com>

abaptiste · 2022-03-03T16:46:13Z

LGTM. Thanks @rexnp!

wrowe

If you could quickly mop up spelling_dictionary.txt and the associated comments, this is looking about ready to merge.

wrowe · 2022-03-10T22:43:05Z

tools/spelling/spelling_dictionary.txt

@@ -1176,6 +1178,7 @@ subnets
 suboptimal
 subsecond
 subseconds
+subsegment


M-W agrees this is valid.

wrowe · 2022-03-10T22:44:49Z

tools/spelling/spelling_dictionary.txt

@@ -401,7 +401,9 @@ XFF
 XML
 XN
 XNOR
+XRay


This doesn't look right, https://docs.aws.amazon.com/xray/latest/devguide/ spells this X-Ray as the canonical name, no?

wrowe · 2022-03-10T22:48:38Z

tools/spelling/spelling_dictionary.txt

 XSS
+Xray


This is definitely a problem, we shouldn't be using 3+ different terms. But if we were agnostic about exchanging these so liberally, then refer to lines 1-4 of this file which describe how case of the terms affects variant capitalization. "XRAY" would permit any casing and would be preferred to having multiple entries. Using one proper term consistently is preferred of that.

hi @wrowe, thanks for taking a look this! my mistake for adding duplicate xray entries to the dictionary. I realize I should just fix the terms XRay to X-Ray where they were violating the spell check so XRAY doesn't even have to be in the dictionary anymore.

Fantastic, I'm not back till Tuesday but if this is cleaned up, it seems like we are good to go. Someone else can also perhaps step up and merge it before then. (Or I can merge in the next few hours.)

Signed-off-by: Rex Chang <58710378+rexnp@users.noreply.github.com>

wrowe

Thank you for your submission @rexnp, and for fixing the wording! Thank you @abaptiste for the review! LGTM

…yproxy#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>

rexnp requested a review from mattklein123 as a code owner March 2, 2022 00:46

rexnp mentioned this pull request Mar 2, 2022

Bug: AWS X-Ray tracing: remote subsegments should name the remote service, not the local service aws/aws-app-mesh-roadmap#354

Open

repokitteh-read-only bot assigned abaptiste Mar 2, 2022

repokitteh-read-only bot added the waiting label Mar 2, 2022

rexnp requested review from dio, lizan and alyssawilk as code owners March 2, 2022 18:15

repokitteh-read-only bot added deps Approval required for changes to Envoy's external dependencies and removed waiting labels Mar 2, 2022

repokitteh-read-only bot assigned wrowe Mar 2, 2022

rexnp changed the base branch from main to dependabot/github_actions/actions/checkout-3 March 2, 2022 18:22

rexnp changed the base branch from dependabot/github_actions/actions/checkout-3 to main March 2, 2022 18:22

alyssawilk and others added 12 commits March 2, 2022 18:28

Add a congestionWindowInBytes method to Envoy::Network::Connection (e…

6a86a26

…nvoyproxy#20105) Signed-off-by: Bin Wu <wub@google.com> Signed-off-by: Rex Chang <58710378+rexnp@users.noreply.github.com>

test: removing a bunch of direct runtime singleton access (envoyproxy…

fdec215

…#19993) Signed-off-by: Alyssa Wilk <alyssar@chromium.org> Signed-off-by: Rex Chang <58710378+rexnp@users.noreply.github.com>

adds to spellcheck

c120667

Signed-off-by: Rex Chang <58710378+rexnp@users.noreply.github.com>

xray tracer: set subsegment type for child spans (#2)

7e8134f

* 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>

fixes spellcheck

5be2062

Signed-off-by: Rex Chang <58710378+rexnp@users.noreply.github.com>

rexnp force-pushed the main branch from 2972366 to 5be2062 Compare March 2, 2022 18:38

rexnp requested review from rojkov and antoniovicente as code owners March 2, 2022 18:38

rexnp changed the base branch from main to dependabot/github_actions/actions/checkout-3 March 2, 2022 18:40

rexnp changed the base branch from dependabot/github_actions/actions/checkout-3 to main March 2, 2022 18:40

rexnp added 3 commits March 2, 2022 18:44

Merge branch 'envoyproxy:main' into main

cee9167

Merge branch 'main' of github.com:rexnp/envoy

7a485f3

Signed-off-by: Rex Chang <58710378+rexnp@users.noreply.github.com>

wrowe suggested changes Mar 10, 2022

View reviewed changes

fixes spell check

83e97f8

Signed-off-by: Rex Chang <58710378+rexnp@users.noreply.github.com>

wrowe approved these changes Mar 15, 2022

View reviewed changes

wrowe merged commit 82c02b7 into envoyproxy:main Mar 15, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

xray tracer: correctly annotate child spans as xray subsegments #20170

xray tracer: correctly annotate child spans as xray subsegments #20170

rexnp commented Mar 2, 2022

rexnp commented Mar 2, 2022

rojkov commented Mar 2, 2022

repokitteh-read-only bot commented Mar 2, 2022

abaptiste commented Mar 3, 2022

wrowe left a comment

wrowe Mar 10, 2022

wrowe Mar 10, 2022

wrowe Mar 10, 2022

rexnp Mar 11, 2022

wrowe Mar 11, 2022 •

edited

Loading

wrowe left a comment

@@ @@ -401,7 +401,9 @@ XFF @@
               XML
               XN
               XNOR
+              XRay

xray tracer: correctly annotate child spans as xray subsegments #20170

xray tracer: correctly annotate child spans as xray subsegments #20170

Conversation

rexnp commented Mar 2, 2022

rexnp commented Mar 2, 2022

rojkov commented Mar 2, 2022

repokitteh-read-only bot commented Mar 2, 2022

abaptiste commented Mar 3, 2022

wrowe left a comment

Choose a reason for hiding this comment

wrowe Mar 10, 2022

Choose a reason for hiding this comment

wrowe Mar 10, 2022

Choose a reason for hiding this comment

wrowe Mar 10, 2022

Choose a reason for hiding this comment

rexnp Mar 11, 2022

Choose a reason for hiding this comment

wrowe Mar 11, 2022 • edited Loading

Choose a reason for hiding this comment

wrowe left a comment

Choose a reason for hiding this comment

wrowe Mar 11, 2022 •

edited

Loading