From 7e8c33c8ae659e388ea71aa55d934144a38e4571 Mon Sep 17 00:00:00 2001 From: Anuraag Agrawal Date: Tue, 9 Jun 2020 08:06:12 +0900 Subject: [PATCH] =?UTF-8?q?Only=20allocate=20a=20new=20(random)=20trace=20?= =?UTF-8?q?ID=20when=20trace=20ID=20parse=20fails=20rathe=E2=80=A6=20(#161?= =?UTF-8?q?)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Only allocate a new (random) trace ID when trace ID parse fails rather than all the time and prepare for immutable. * Remove deprecated * int --- .../xray/contexts/LambdaSegmentContext.java | 2 +- .../amazonaws/xray/entities/DummySegment.java | 2 +- .../xray/entities/DummySubsegment.java | 2 +- .../amazonaws/xray/entities/SegmentImpl.java | 2 +- .../com/amazonaws/xray/entities/TraceID.java | 75 +++++++++++++------ .../javax/servlet/AWSXRayServletFilter.java | 4 +- 6 files changed, 58 insertions(+), 29 deletions(-) diff --git a/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/contexts/LambdaSegmentContext.java b/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/contexts/LambdaSegmentContext.java index 69bf71d2..d76fce1f 100644 --- a/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/contexts/LambdaSegmentContext.java +++ b/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/contexts/LambdaSegmentContext.java @@ -59,7 +59,7 @@ public Subsegment beginSubsegment(AWSXRayRecorder recorder, String name) { if (LambdaSegmentContext.isInitializing(LambdaSegmentContext.getTraceHeaderFromEnvironment())) { logger.warn(LAMBDA_TRACE_HEADER_KEY + " is missing a trace ID, parent ID, or sampling decision. Subsegment " + name + " discarded."); - parentSegment = new FacadeSegment(recorder, new TraceID(), "", SampleDecision.NOT_SAMPLED); + parentSegment = new FacadeSegment(recorder, TraceID.create(), "", SampleDecision.NOT_SAMPLED); } else { parentSegment = LambdaSegmentContext.newFacadeSegment(recorder); } diff --git a/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/DummySegment.java b/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/DummySegment.java index 0edb2071..5ce41a0e 100644 --- a/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/DummySegment.java +++ b/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/DummySegment.java @@ -53,7 +53,7 @@ public DummySegment(AWSXRayRecorder creator, String name, TraceID traceId) { } public DummySegment(AWSXRayRecorder creator) { - this(creator, new TraceID()); + this(creator, TraceID.create()); } public DummySegment(AWSXRayRecorder creator, TraceID traceId) { diff --git a/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/DummySubsegment.java b/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/DummySubsegment.java index fde8b752..5e6fc386 100644 --- a/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/DummySubsegment.java +++ b/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/DummySubsegment.java @@ -39,7 +39,7 @@ public class DummySubsegment implements Subsegment { private Segment parentSegment; public DummySubsegment(AWSXRayRecorder creator) { - this(creator, new TraceID()); + this(creator, TraceID.create()); } public DummySubsegment(AWSXRayRecorder creator, TraceID traceId) { diff --git a/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/SegmentImpl.java b/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/SegmentImpl.java index ca37fee5..d01f2f0f 100644 --- a/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/SegmentImpl.java +++ b/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/SegmentImpl.java @@ -39,7 +39,7 @@ private SegmentImpl() { } // default constructor for jackson public SegmentImpl(AWSXRayRecorder creator, String name) { - this(creator, name, new TraceID()); + this(creator, name, TraceID.create()); } public SegmentImpl(AWSXRayRecorder creator, String name, TraceID traceId) { diff --git a/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/TraceID.java b/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/TraceID.java index 55f07ec4..72ee75a6 100644 --- a/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/TraceID.java +++ b/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/TraceID.java @@ -22,54 +22,77 @@ public class TraceID { + private static final int TRACE_ID_LENGTH = 35; + private static final int TRACE_ID_DELIMITER_INDEX_1 = 1; + private static final int TRACE_ID_DELIMITER_INDEX_2 = 10; + private static final char VERSION = '1'; private static final char DELIMITER = '-'; private BigInteger number; private long startTime; + /** + * @deprecated Use {@link #create()}. + */ + @Deprecated public TraceID() { this(Instant.now().getEpochSecond()); } + /** + * @deprecated Use {@link #create()}. + */ + @Deprecated public TraceID(long startTime) { number = new BigInteger(96, ThreadLocalStorage.getRandom()); this.startTime = startTime; } - public static TraceID fromString(String string) { - string = string.trim(); - TraceID traceId = new TraceID(); + private TraceID(long startTime, BigInteger number) { + this.startTime = startTime; + this.number = number; + } - long startTime; + /** + * Returns a new {@link TraceID} which represents the start of a new trace. + */ + public static TraceID create() { + return new TraceID(); + } - int delimiterIndex; + /** + * Returns the {@link TraceID} parsed out of the {@link String}. If the parse fails, a new {@link TraceID} will be returned, + * effectively restarting the trace. + */ + public static TraceID fromString(String xrayTraceId) { + xrayTraceId = xrayTraceId.trim(); - // Skip version number - delimiterIndex = string.indexOf(DELIMITER); - if (delimiterIndex < 0) { - return traceId; + if (xrayTraceId.length() != TRACE_ID_LENGTH) { + return TraceID.create(); } - int valueStartIndex = delimiterIndex + 1; - delimiterIndex = string.indexOf(DELIMITER, valueStartIndex); - if (delimiterIndex < 0) { - return traceId; - } else { - startTime = Long.valueOf(string.substring(valueStartIndex, delimiterIndex), 16); + // Check version trace id version + if (xrayTraceId.charAt(0) != VERSION) { + return TraceID.create(); } - valueStartIndex = delimiterIndex + 1; - delimiterIndex = string.indexOf(DELIMITER, valueStartIndex); - if (delimiterIndex < 0) { - // End of string - delimiterIndex = string.length(); + // Check delimiters + if (xrayTraceId.charAt(TRACE_ID_DELIMITER_INDEX_1) != DELIMITER + || xrayTraceId.charAt(TRACE_ID_DELIMITER_INDEX_2) != DELIMITER) { + return TraceID.create(); } - traceId.setNumber(new BigInteger(string.substring(valueStartIndex, delimiterIndex), 16)); - traceId.setStartTime(startTime); + String startTimePart = xrayTraceId.substring(TRACE_ID_DELIMITER_INDEX_1 + 1, TRACE_ID_DELIMITER_INDEX_2); + String randomPart = xrayTraceId.substring(TRACE_ID_DELIMITER_INDEX_2 + 1, TRACE_ID_LENGTH); - return traceId; + final TraceID result; + try { + result = new TraceID(Long.valueOf(startTimePart, 16), new BigInteger(randomPart, 16)); + } catch (NumberFormatException e) { + return TraceID.create(); + } + return result; } @Override @@ -90,7 +113,10 @@ public BigInteger getNumber() { /** * @param number the number to set + * + * @deprecated TraceID is effectively immutable and this will be removed */ + @Deprecated public void setNumber(BigInteger number) { this.number = number; } @@ -104,7 +130,10 @@ public long getStartTime() { /** * @param startTime the startTime to set + * + * @deprecated TraceID is effectively immutable and this will be removed */ + @Deprecated public void setStartTime(long startTime) { this.startTime = startTime; } diff --git a/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/javax/servlet/AWSXRayServletFilter.java b/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/javax/servlet/AWSXRayServletFilter.java index 8251f573..104d7fa9 100644 --- a/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/javax/servlet/AWSXRayServletFilter.java +++ b/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/javax/servlet/AWSXRayServletFilter.java @@ -301,7 +301,7 @@ public Segment preFilter(ServletRequest request, ServletResponse response) { HttpServletRequest httpServletRequest = castServletRequest(request); if (null == httpServletRequest) { logger.warn("Null value for incoming HttpServletRequest. Beginning DummySegment."); - return recorder.beginDummySegment(new TraceID()); + return recorder.beginDummySegment(TraceID.create()); } Optional incomingHeader = getTraceHeader(httpServletRequest); @@ -321,7 +321,7 @@ public Segment preFilter(ServletRequest request, ServletResponse response) { TraceID traceId = incomingHeader.isPresent() ? incomingHeader.get().getRootTraceId() : null; if (null == traceId) { - traceId = new TraceID(); + traceId = TraceID.create(); } String parentId = incomingHeader.isPresent() ? incomingHeader.get().getParentId() : null;