Skip to content

Commit

Permalink
Account for Sampled=0 trace header in Active mode
Browse files Browse the repository at this point in the history
  • Loading branch information
majanjua-amzn committed Aug 6, 2024
1 parent de6566b commit 622be17
Show file tree
Hide file tree
Showing 6 changed files with 27 additions and 28 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import com.amazonaws.xray.AWSXRay;
import com.amazonaws.xray.AWSXRayRecorder;
import com.amazonaws.xray.entities.Namespace;
import com.amazonaws.xray.entities.NoOpSubSegment;
import com.amazonaws.xray.entities.Segment;
import com.amazonaws.xray.entities.Subsegment;
import com.amazonaws.xray.entities.TraceHeader;
Expand Down Expand Up @@ -104,15 +105,15 @@ public static void addRequestInformation(Subsegment subsegment, HttpRequest requ
Segment parentSegment = subsegment.getParentSegment();

if (subsegment.shouldPropagate()) {
// If the trace is not sampled, passing Parent and Sampled won't matter
// If no-op, only propagate root trace ID to not taint sampling decision
TraceHeader t = TraceHeader.fromEntity(subsegment);
if (t.getSampled() != TraceHeader.SampleDecision.SAMPLED) {
if (subsegment instanceof NoOpSubSegment) {
request.addHeader(
TraceHeader.HEADER_KEY,
"Root=" + t.getRootTraceId().toString());
} else {
// This will propagate Parent and Sampled
request.addHeader(TraceHeader.HEADER_KEY, TraceHeader.fromEntity(subsegment).toString());
request.addHeader(TraceHeader.HEADER_KEY, t.toString());
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ public void unsampledButForcedPropagation() throws Exception {

verify(getRequestedFor(urlPathEqualTo("/"))
.withHeader(TraceHeader.HEADER_KEY,
equalTo("Root=1-67891233-abcdef012345678912345678")));
equalTo("Root=1-67891233-abcdef012345678912345678;Sampled=0")));
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import com.amazonaws.xray.entities.EntityDataKeys;
import com.amazonaws.xray.entities.EntityHeaderKeys;
import com.amazonaws.xray.entities.Namespace;
import com.amazonaws.xray.entities.NoOpSubSegment;
import com.amazonaws.xray.entities.Subsegment;
import com.amazonaws.xray.entities.TraceHeader;
import com.amazonaws.xray.handlers.config.AWSOperationHandler;
Expand Down Expand Up @@ -296,9 +297,9 @@ public SdkHttpRequest modifyHttpRequest(Context.ModifyHttpRequest context, Execu
return httpRequest;
}

// If the trace is not sampled, passing Parent and Sampled won't matter
// If no-op, only propagate root trace ID to not taint sampling decision
TraceHeader t = TraceHeader.fromEntity(subsegment);
if (t.getSampled() != TraceHeader.SampleDecision.SAMPLED) {
if (subsegment instanceof NoOpSubSegment) {
return httpRequest.toBuilder().putHeader(
TraceHeader.HEADER_KEY,
"Root=" + t.getRootTraceId().toString()).build();
Expand All @@ -307,7 +308,7 @@ public SdkHttpRequest modifyHttpRequest(Context.ModifyHttpRequest context, Execu
// This will propagate Parent and Sampled
return httpRequest.toBuilder().putHeader(
TraceHeader.HEADER_KEY,
TraceHeader.fromEntity(subsegment).toString()).build();
t.toString()).build();
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import com.amazonaws.xray.entities.EntityDataKeys;
import com.amazonaws.xray.entities.EntityHeaderKeys;
import com.amazonaws.xray.entities.Namespace;
import com.amazonaws.xray.entities.NoOpSubSegment;
import com.amazonaws.xray.entities.Subsegment;
import com.amazonaws.xray.entities.TraceHeader;
import com.amazonaws.xray.handlers.config.AWSOperationHandler;
Expand Down Expand Up @@ -192,15 +193,15 @@ public void beforeRequest(Request<?> request) {
currentSubsegment.setNamespace(Namespace.AWS.toString());

if (recorder.getCurrentSegment() != null && recorder.getCurrentSubsegment().shouldPropagate()) {
// If the trace is not sampled, passing Parent and Sampled won't matter
// If no-op, only propagate root trace ID to not taint sampling decision
TraceHeader t = TraceHeader.fromEntity(currentSubsegment);
if (t.getSampled() != TraceHeader.SampleDecision.SAMPLED) {
if (currentSubsegment instanceof NoOpSubSegment) {
request.addHeader(
TraceHeader.HEADER_KEY,
"Root=" + t.getRootTraceId().toString());
} else {
// This will propagate Parent and Sampled
request.addHeader(TraceHeader.HEADER_KEY, TraceHeader.fromEntity(currentSubsegment).toString());
request.addHeader(TraceHeader.HEADER_KEY, t.toString());
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,24 +58,20 @@ public Subsegment beginSubsegment(AWSXRayRecorder recorder, String name) {
Entity entity = getTraceEntity();
if (entity == null) { // First subsegment of a subsegment branch
Segment parentSegment;
// We rely on the AWS SDK to propagate one or both of these
// It is therefore possible to get just Root, or Root and Parent
if (traceHeader.getRootTraceId() != null) {
if (traceHeader.getParentId() != null) {
parentSegment = new FacadeSegment(
recorder,
traceHeader.getRootTraceId(),
traceHeader.getParentId(),
traceHeader.getSampled());
} else {
if (logger.isDebugEnabled()) {
logger.debug("Creating No-Op parent segment");
}
TraceID t = traceHeader.getRootTraceId();
parentSegment = Segment.noOp(t, recorder);
}
// Trace header either takes the structure `Root=...;<extra-data>` or
// `Root=...;Parent=...;Sampled=...;<extra-data>`
if (traceHeader.getRootTraceId() != null && traceHeader.getParentId() != null && traceHeader.getSampled() != null) {
parentSegment = new FacadeSegment(
recorder,
traceHeader.getRootTraceId(),
traceHeader.getParentId(),
traceHeader.getSampled());
} else {
parentSegment = Segment.noOp(TraceID.create(recorder), recorder);
if (logger.isDebugEnabled()) {
logger.debug("Creating No-Op parent segment");
}
TraceID t = traceHeader.getRootTraceId() != null ? traceHeader.getRootTraceId() : TraceID.create(recorder);
parentSegment = Segment.noOp(t, recorder);
}

boolean isRecording = parentSegment.isRecording();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
import java.util.concurrent.locks.ReentrantLock;
import org.checkerframework.checker.nullness.qual.Nullable;

class NoOpSubSegment implements Subsegment {
public class NoOpSubSegment implements Subsegment {
private final Segment parentSegment;
private final AWSXRayRecorder creator;
private final boolean shouldPropagate;
Expand Down

0 comments on commit 622be17

Please sign in to comment.