Skip to content

Fully qualified, configurable Trace ID Injection - #135

Merged
willarmiros merged 9 commits into
aws:masterfrom
willarmiros:full-id-injection
Apr 9, 2020
Merged

Fully qualified, configurable Trace ID Injection#135
willarmiros merged 9 commits into
aws:masterfrom
willarmiros:full-id-injection

Conversation

@willarmiros

@willarmiros willarmiros commented Apr 1, 2020

Copy link
Copy Markdown
Contributor

Description of changes

  • Added listeners for subsegments
  • Inject the fully qualified ID instead of just the Trace ID by default. The fully qualified ID is represented as TraceID@EntityID, for example:
1-5df42873-011e96598b447dfca814c156@541b3365be3dafc3
  • Exposed new AWSXRay.currentFormattedId() API
  • Added the ability to configure, or disable, prefixes in trace ID Injection
  • Cleaned up JavaDocs and added unit tests

Note that this change will affect customers who are currently using trace ID injection. The trace IDs injected into logs will automatically be converted to fully qualified IDs such as the ones described above, which may impact log parsing. Customers should inspect how their logs are consumed to make sure there won't be any difficulties upgrading.

Tested on sample app with Log4J and SLF4J loggers. Verified cross-thread propagation and nested subsegments work as expected.

Example

Log4JSegmentListener listener = new Log4JSegmentListener();
AWSXRayRecorderBuilder builder = new AWSXRayRecorderBuilder()
  .withSegmentListener(listener);
AWSXRay.setGlobalRecorder(builder.build());
AWSXRay.beginSegment("Example1");  // segment ID = 541b3365be3dafc3
logger.info("First logging message");
AWSXRay.beginSubsegment("Example2");  // subsegment ID = 1ce7df03252d99e1
listener.setPrefix("");  // Setting the prefix to an empty string removes it entirely
logger.info("Second logging message");
AWSXRay.endSubsegment();
AWSXRay.endSegment();

Would result in logs similar to this:

14:19:48.618 INFO  AWS-XRAY-TRACE-ID: 1-5df42873-011e96598b447dfca814c156@541b3365be3dafc3 First logging message
14:19:48.745 INFO  1-5df42873-011e96598b447dfca814c156@1ce7df03252d99e1 Second logging message

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@c1tadel

c1tadel commented Apr 1, 2020

Copy link
Copy Markdown
Contributor

Can you discuss impact to any integrations (e.g. Log searches) or other backwards compatibility or upgrade issues customers may face?


/**
* beforeEndSubsegment is invoked just before a subsegment is ended by the recorder.
* The subsegment can be manipulated, e.g. with putAnnotation.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

putAnnotation doesn't seem to be a good example because even with final you can still call subsegment.putAnnotation. BTW I'm confused by "The subsegment cannot be manipulated". The subsegment itself is surely mutable. The argument is a referrence of the subsegment. The method can never replace the original subsegment, so what is the point to allow manipulating the referrence? (ref https://stackoverflow.com/questions/1249917/final-variable-manipulation-in-java)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is a fair callout. The (sub)segment can't be modified in afterEndSegment because it is invoked after the segment is emitted to the daemon, and attempting to modify it would raise an AlreadyEmittedException. I'll update the docs to clarify this.

I added the final keyword to indicate its immutability, but you're right that making a parameter final does not make it immutable. I'll remove the final to avoid confusion.

@willarmiros willarmiros closed this Apr 9, 2020
@willarmiros willarmiros reopened this Apr 9, 2020
@willarmiros
willarmiros merged commit 9eb3cc9 into aws:master Apr 9, 2020
@willarmiros
willarmiros deleted the full-id-injection branch April 9, 2020 22:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants