Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Only allocate a new (random) trace ID when trace ID parse fails rathe… #161

Merged
merged 3 commits into from
Jun 8, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,54 +22,77 @@

public class TraceID {

private static final int TRACE_ID_LENGTH = 35;
Copy link
Contributor

Choose a reason for hiding this comment

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

TODO: increase by 1 in ~85 years :D

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
Expand All @@ -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;
}
Expand All @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<TraceHeader> incomingHeader = getTraceHeader(httpServletRequest);
Expand All @@ -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;
Expand Down