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

Simplifying Oversampling Mitigation #356

Merged
merged 2 commits into from
Oct 12, 2022
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 @@ -32,7 +32,6 @@
import com.amazonaws.xray.exceptions.SubsegmentNotFoundException;
import com.amazonaws.xray.internal.FastIdGenerator;
import com.amazonaws.xray.internal.IdGenerator;
import com.amazonaws.xray.internal.SamplingStrategyOverride;
import com.amazonaws.xray.internal.SecureIdGenerator;
import com.amazonaws.xray.listeners.SegmentListener;
import com.amazonaws.xray.strategy.ContextMissingStrategy;
Expand Down Expand Up @@ -636,10 +635,7 @@ public Subsegment beginSubsegmentWithoutSampling(String name) {
// context to be propagated downstream
return Subsegment.noOp(this, false);
}
return context.beginSubsegmentWithSamplingOverride(
this,
name,
SamplingStrategyOverride.FALSE);
return context.beginSubsegmentWithoutSampling(this, name);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@
import com.amazonaws.xray.entities.TraceHeader.SampleDecision;
import com.amazonaws.xray.entities.TraceID;
import com.amazonaws.xray.exceptions.SubsegmentNotFoundException;
import com.amazonaws.xray.internal.SamplingStrategyOverride;
import com.amazonaws.xray.listeners.SegmentListener;
import java.util.List;
import java.util.Objects;
Expand Down Expand Up @@ -62,11 +61,17 @@ private static FacadeSegment newFacadeSegment(AWSXRayRecorder recorder, String n
}

@Override
public Subsegment beginSubsegmentWithSamplingOverride(
public Subsegment beginSubsegmentWithoutSampling(
AWSXRayRecorder recorder,
String name,
SamplingStrategyOverride samplingStrategyOverride) {
String name) {

Subsegment subsegment = beginSubsegment(recorder, name);
subsegment.setSampledFalse();
return subsegment;
}

@Override
public Subsegment beginSubsegment(AWSXRayRecorder recorder, String name) {
if (logger.isDebugEnabled()) {
logger.debug("Beginning subsegment named: " + name);
}
Expand All @@ -75,12 +80,11 @@ public Subsegment beginSubsegmentWithSamplingOverride(
if (entity == null) { // First subsgment of a subsegment branch.
Segment parentSegment = newFacadeSegment(recorder, name);

boolean isRecording = parentSegment.isRecording() &&
samplingStrategyOverride == SamplingStrategyOverride.DISABLED;
boolean isRecording = parentSegment.isRecording();

Subsegment subsegment = isRecording
? new SubsegmentImpl(recorder, name, parentSegment, samplingStrategyOverride)
: Subsegment.noOp(parentSegment, recorder, samplingStrategyOverride);
? new SubsegmentImpl(recorder, name, parentSegment)
: Subsegment.noOp(parentSegment, recorder);
subsegment.setParent(parentSegment);
// Enable FacadeSegment to keep track of its subsegments for subtree streaming
parentSegment.addSubsegment(subsegment);
Expand All @@ -97,12 +101,11 @@ public Subsegment beginSubsegmentWithSamplingOverride(
}
Segment parentSegment = parentSubsegment.getParentSegment();

boolean isRecording = parentSegment.isRecording() &&
samplingStrategyOverride == SamplingStrategyOverride.DISABLED;
boolean isRecording = parentSegment.isRecording();

Subsegment subsegment = isRecording
? new SubsegmentImpl(recorder, name, parentSegment, samplingStrategyOverride)
: Subsegment.noOp(parentSegment, recorder, samplingStrategyOverride);
? new SubsegmentImpl(recorder, name, parentSegment)
: Subsegment.noOp(parentSegment, recorder);
subsegment.setParent(parentSubsegment);
parentSubsegment.addSubsegment(subsegment);
setTraceEntity(subsegment);
Expand All @@ -116,11 +119,6 @@ public Subsegment beginSubsegmentWithSamplingOverride(
}
}

@Override
public Subsegment beginSubsegment(AWSXRayRecorder recorder, String name) {
return beginSubsegmentWithSamplingOverride(recorder, name, SamplingStrategyOverride.DISABLED);
}

@Override
public void endSubsegment(AWSXRayRecorder recorder) {
Entity current = getTraceEntity();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
import com.amazonaws.xray.entities.Entity;
import com.amazonaws.xray.entities.Segment;
import com.amazonaws.xray.entities.Subsegment;
import com.amazonaws.xray.internal.SamplingStrategyOverride;
import java.util.Objects;
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
Expand Down Expand Up @@ -66,10 +65,9 @@ default void clearTraceEntity() {

Subsegment beginSubsegment(AWSXRayRecorder recorder, String name);

Subsegment beginSubsegmentWithSamplingOverride(
Subsegment beginSubsegmentWithoutSampling(
AWSXRayRecorder recorder,
String name,
SamplingStrategyOverride samplingStrategyOverride);
String name);

void endSubsegment(AWSXRayRecorder recorder);
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
import com.amazonaws.xray.entities.SubsegmentImpl;
import com.amazonaws.xray.exceptions.SegmentNotFoundException;
import com.amazonaws.xray.exceptions.SubsegmentNotFoundException;
import com.amazonaws.xray.internal.SamplingStrategyOverride;
import com.amazonaws.xray.listeners.SegmentListener;
import java.util.List;
import java.util.Objects;
Expand All @@ -34,23 +33,29 @@ public class ThreadLocalSegmentContext implements SegmentContext {
LogFactory.getLog(ThreadLocalSegmentContext.class);

@Override
public Subsegment beginSubsegmentWithSamplingOverride(
public Subsegment beginSubsegmentWithoutSampling(
AWSXRayRecorder recorder,
String name,
SamplingStrategyOverride samplingStrategyOverride) {
String name) {
Subsegment subsegment = beginSubsegment(recorder, name);
subsegment.setSampledFalse();
return subsegment;
}

@Override
public Subsegment beginSubsegment(AWSXRayRecorder recorder, String name) {
Entity current = getTraceEntity();
if (current == null) {
recorder.getContextMissingStrategy().contextMissing("Failed to begin subsegment named '" + name
+ "': segment cannot be found.", SegmentNotFoundException.class);
return Subsegment.noOp(recorder, false, samplingStrategyOverride);
return Subsegment.noOp(recorder, false);
}
if (logger.isDebugEnabled()) {
logger.debug("Beginning subsegment named: " + name);
}
Segment parentSegment = current.getParentSegment();
Subsegment subsegment = parentSegment.isRecording() && (samplingStrategyOverride == SamplingStrategyOverride.DISABLED)
? new SubsegmentImpl(recorder, name, parentSegment, samplingStrategyOverride)
: Subsegment.noOp(parentSegment, recorder, samplingStrategyOverride);
Subsegment subsegment = parentSegment.isRecording()
? new SubsegmentImpl(recorder, name, parentSegment)
: Subsegment.noOp(parentSegment, recorder);
subsegment.setParent(current);
current.addSubsegment(subsegment);
setTraceEntity(subsegment);
Expand All @@ -63,11 +68,6 @@ public Subsegment beginSubsegmentWithSamplingOverride(
return subsegment;
}

@Override
public Subsegment beginSubsegment(AWSXRayRecorder recorder, String name) {
return beginSubsegmentWithSamplingOverride(recorder, name, SamplingStrategyOverride.DISABLED);
}

@Override
public void endSubsegment(AWSXRayRecorder recorder) {
Entity current = getTraceEntity();
Expand All @@ -87,8 +87,7 @@ public void endSubsegment(AWSXRayRecorder recorder) {
.filter(Objects::nonNull)
.forEach(listener -> listener.beforeEndSubsegment(currentSubsegment));

if ((currentSubsegment.end() &&
currentSubsegment.getSamplingStrategyOverride() == SamplingStrategyOverride.DISABLED)) {
if (currentSubsegment.end() && currentSubsegment.isSampled()) {
recorder.sendSegment(currentSubsegment.getParentSegment());
} else {
if (recorder.getStreamingStrategy().requiresStreaming(currentSubsegment.getParentSegment())) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
package com.amazonaws.xray.entities;

import com.amazonaws.xray.AWSXRayRecorder;
import com.amazonaws.xray.internal.SamplingStrategyOverride;
import com.fasterxml.jackson.annotation.JsonIgnore;
import java.util.ArrayList;
import java.util.HashSet;
import java.util.List;
Expand Down Expand Up @@ -44,21 +44,20 @@ public class DummySubsegment implements Subsegment {
private TraceID traceId;
private Segment parentSegment;

private SamplingStrategyOverride samplingStrategyOverride;
@JsonIgnore
private boolean isSampled;

@JsonIgnore
private boolean isRecording;

public DummySubsegment(AWSXRayRecorder creator) {
this(creator, TraceID.create(creator));
}

public DummySubsegment(AWSXRayRecorder creator, TraceID traceId, SamplingStrategyOverride samplingStrategyOverride) {
public DummySubsegment(AWSXRayRecorder creator, TraceID traceId) {
this.creator = creator;
this.traceId = traceId;
this.parentSegment = new DummySegment(creator);
this.samplingStrategyOverride = samplingStrategyOverride;
}

public DummySubsegment(AWSXRayRecorder creator, TraceID traceId) {
this(creator, traceId, SamplingStrategyOverride.DISABLED);
}

@Override
Expand Down Expand Up @@ -396,7 +395,21 @@ public void removeSubsegment(Subsegment subsegment) {
}

@Override
public SamplingStrategyOverride getSamplingStrategyOverride() {
return samplingStrategyOverride;
@JsonIgnore
public boolean isSampled() {
return isSampled;
}

@Override
@JsonIgnore
public boolean isRecording() {
return isRecording;
}

@Override
@JsonIgnore
public void setSampledFalse() {
isSampled = false;
isRecording = false;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
package com.amazonaws.xray.entities;

import com.amazonaws.xray.AWSXRayRecorder;
import com.amazonaws.xray.internal.SamplingStrategyOverride;
import com.fasterxml.jackson.annotation.JsonIgnore;
import java.util.List;
import java.util.Map;
Expand All @@ -34,30 +33,24 @@ class NoOpSubSegment implements Subsegment {
private volatile Entity parent;

@JsonIgnore
private SamplingStrategyOverride samplingStrategyOverride;
private boolean isSampled;

@JsonIgnore
private boolean isRecording;

NoOpSubSegment(Segment parentSegment, AWSXRayRecorder creator) {
this(parentSegment, creator, true);
}

NoOpSubSegment(Segment parentSegment, AWSXRayRecorder creator, boolean shouldPropagate) {
this(parentSegment, creator, shouldPropagate, SamplingStrategyOverride.DISABLED);
}

NoOpSubSegment(Segment parentSegment, AWSXRayRecorder creator, SamplingStrategyOverride samplingStrategyOverride) {
this(parentSegment, creator, true, samplingStrategyOverride);
}

NoOpSubSegment(
Segment parentSegment,
AWSXRayRecorder creator,
boolean shouldPropagate,
SamplingStrategyOverride samplingStrategyOverride) {
boolean shouldPropagate) {
this.parentSegment = parentSegment;
this.creator = creator;
this.shouldPropagate = shouldPropagate;
parent = parentSegment;
this.samplingStrategyOverride = samplingStrategyOverride;
}

@Override
Expand Down Expand Up @@ -391,7 +384,21 @@ public void close() {
}

@Override
public SamplingStrategyOverride getSamplingStrategyOverride() {
return samplingStrategyOverride;
@JsonIgnore
public boolean isSampled() {
return isSampled;
}

@Override
@JsonIgnore
public boolean isRecording() {
return isRecording;
}

@Override
@JsonIgnore
public void setSampledFalse() {
isSampled = false;
isRecording = false;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,21 +16,11 @@
package com.amazonaws.xray.entities;

import com.amazonaws.xray.AWSXRayRecorder;
import com.amazonaws.xray.internal.SamplingStrategyOverride;
import com.fasterxml.jackson.annotation.JsonIgnore;
import java.util.Set;
import org.checkerframework.checker.nullness.qual.Nullable;

public interface Subsegment extends Entity {

static Subsegment noOp(AWSXRayRecorder recorder, boolean shouldPropagate, SamplingStrategyOverride samplingStrategyOverride) {
return new NoOpSubSegment(Segment.noOp(TraceID.invalid(), recorder), recorder, shouldPropagate, samplingStrategyOverride);
}

static Subsegment noOp(Segment parent, AWSXRayRecorder recorder, SamplingStrategyOverride samplingStrategyOverride) {
return new NoOpSubSegment(parent, recorder, samplingStrategyOverride);
}

static Subsegment noOp(AWSXRayRecorder recorder, boolean shouldPropagate) {
return new NoOpSubSegment(Segment.noOp(TraceID.invalid(), recorder), recorder, shouldPropagate);
}
Expand Down Expand Up @@ -119,15 +109,12 @@ static Subsegment noOp(Segment parent, AWSXRayRecorder recorder) {
@Override
void close();

SamplingStrategyOverride getSamplingStrategyOverride();
@JsonIgnore
boolean isSampled();

@JsonIgnore
default boolean isSampled() {
return getParentSegment().isSampled() && getSamplingStrategyOverride() == SamplingStrategyOverride.DISABLED;
}
boolean isRecording();

@JsonIgnore
default boolean isRecording() {
return getParentSegment().isRecording() && getSamplingStrategyOverride() == SamplingStrategyOverride.DISABLED;
}
void setSampledFalse();
}
Loading