Skip to content

Commit

Permalink
[#9575] Remove unnecessary sampling flag
Browse files Browse the repository at this point in the history
  • Loading branch information
@emeroad
emeroad committed Jan 6, 2023
1 parent dc60c05 commit 74275da
Show file tree
Hide file tree
Showing 10 changed files with 41 additions and 53 deletions.
6 changes: 2 additions & 4 deletions profiler/src/main/java/com/navercorp/pinpoint/profiler/context/AsyncChildTrace.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
import com.navercorp.pinpoint.bootstrap.context.TraceId;
import com.navercorp.pinpoint.bootstrap.context.scope.TraceScope;
import com.navercorp.pinpoint.common.annotations.VisibleForTesting;
import com.navercorp.pinpoint.common.util.Assert;
import com.navercorp.pinpoint.exception.PinpointException;
import com.navercorp.pinpoint.profiler.context.id.TraceRoot;
import com.navercorp.pinpoint.profiler.context.recorder.WrappedSpanEventRecorder;
Expand Down Expand Up @@ -53,13 +52,12 @@ public class AsyncChildTrace implements Trace {
private final TraceRoot traceRoot;
private final LocalAsyncId localAsyncId;

public AsyncChildTrace(final TraceRoot traceRoot, CallStack<SpanEvent> callStack, Storage storage, boolean sampling,
SpanRecorder spanRecorder, WrappedSpanEventRecorder wrappedSpanEventRecorder, final LocalAsyncId localAsyncId) {
public AsyncChildTrace(final TraceRoot traceRoot, CallStack<SpanEvent> callStack, Storage storage,
SpanRecorder spanRecorder, WrappedSpanEventRecorder wrappedSpanEventRecorder, final LocalAsyncId localAsyncId) {

this.traceRoot = Objects.requireNonNull(traceRoot, "traceRoot");
this.callStack = Objects.requireNonNull(callStack, "callStack");
this.storage = Objects.requireNonNull(storage, "storage");
Assert.isTrue(sampling, "sampling must be true");

this.spanRecorder = Objects.requireNonNull(spanRecorder, "spanRecorder");
this.wrappedSpanEventRecorder = Objects.requireNonNull(wrappedSpanEventRecorder, "wrappedSpanEventRecorder");
Expand Down
29 changes: 13 additions & 16 deletions profiler/src/main/java/com/navercorp/pinpoint/profiler/context/DefaultBaseTraceFactory.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,19 +84,18 @@ public Trace continueTraceObject(final TraceId traceId) {
// TODO need to consider as a target to sample in case Trace object has a sampling flag (true) marked on previous node.
// Check max throughput(permits per seconds)
final TraceSampler.State state = traceSampler.isContinueSampled();
final boolean sampling = state.isSampled();
if (sampling) {
if (state.isSampled()) {
final TraceRoot traceRoot = traceRootFactory.continueTraceRoot(traceId, state.nextId());
final Span span = spanFactory.newSpan(traceRoot);
final SpanChunkFactory spanChunkFactory = new DefaultSpanChunkFactory(traceRoot);
final Storage storage = storageFactory.createStorage(spanChunkFactory);
final CallStack<SpanEvent> callStack = callStackFactory.newCallStack();

final SpanRecorder spanRecorder = recorderFactory.newSpanRecorder(span, traceId.isRoot(), sampling);
final SpanRecorder spanRecorder = recorderFactory.newSpanRecorder(span, traceId.isRoot());
final WrappedSpanEventRecorder wrappedSpanEventRecorder = recorderFactory.newWrappedSpanEventRecorder(traceRoot);
final ActiveTraceHandle handle = registerActiveTrace(traceRoot);

return new DefaultTrace(span, callStack, storage, sampling, spanRecorder,
return new DefaultTrace(span, callStack, storage, spanRecorder,
wrappedSpanEventRecorder, handle, uriStatStorage);
} else {
return newLocalTrace(state.nextId());
Expand Down Expand Up @@ -125,21 +124,20 @@ public Trace newTraceObject(String urlPath) {
}

Trace newTraceObject(TraceSampler.State state) {
final boolean sampling = state.isSampled();
if (sampling) {
if (state.isSampled()) {
final TraceRoot traceRoot = traceRootFactory.newTraceRoot(state.nextId());
final Span span = spanFactory.newSpan(traceRoot);
final SpanChunkFactory spanChunkFactory = new DefaultSpanChunkFactory(traceRoot);
final Storage storage = storageFactory.createStorage(spanChunkFactory);
final CallStack<SpanEvent> callStack = callStackFactory.newCallStack();

final TraceId traceId = traceRoot.getTraceId();
final SpanRecorder spanRecorder = recorderFactory.newSpanRecorder(span, traceId.isRoot(), sampling);
final SpanRecorder spanRecorder = recorderFactory.newSpanRecorder(span, traceId.isRoot());
final WrappedSpanEventRecorder wrappedSpanEventRecorder = recorderFactory.newWrappedSpanEventRecorder(traceRoot);

final ActiveTraceHandle handle = registerActiveTrace(traceRoot);

return new DefaultTrace(span, callStack, storage, sampling, spanRecorder, wrappedSpanEventRecorder,
return new DefaultTrace(span, callStack, storage, spanRecorder, wrappedSpanEventRecorder,
handle, uriStatStorage);
} else {
return newLocalTrace(state.nextId());
Expand All @@ -155,11 +153,11 @@ public Trace continueAsyncContextTraceObject(TraceRoot traceRoot, LocalAsyncId l

final CallStack<SpanEvent> callStack = callStackFactory.newCallStack();

final SpanRecorder spanRecorder = recorderFactory.newTraceRootSpanRecorder(traceRoot, sampling);
final SpanRecorder spanRecorder = recorderFactory.newTraceRootSpanRecorder(traceRoot);

final WrappedSpanEventRecorder wrappedSpanEventRecorder = recorderFactory.newWrappedSpanEventRecorder(traceRoot);

return new AsyncChildTrace(traceRoot, callStack, storage, sampling, spanRecorder, wrappedSpanEventRecorder, localAsyncId);
return new AsyncChildTrace(traceRoot, callStack, storage, spanRecorder, wrappedSpanEventRecorder, localAsyncId);
} else {
return new DisableAsyncChildTrace(traceRoot, localAsyncId);
}
Expand All @@ -171,8 +169,7 @@ public Trace continueAsyncContextTraceObject(TraceRoot traceRoot, LocalAsyncId l
@Override
public Trace continueAsyncTraceObject(final TraceId traceId) {
final TraceSampler.State state = traceSampler.isContinueSampled();
final boolean sampling = state.isSampled();
if (sampling) {
if (state.isSampled()) {
final TraceRoot traceRoot = traceRootFactory.continueTraceRoot(traceId, state.nextId());
final Span span = spanFactory.newSpan(traceRoot);

Expand All @@ -184,11 +181,11 @@ public Trace continueAsyncTraceObject(final TraceId traceId) {
final SpanAsyncStateListener asyncStateListener = new SpanAsyncStateListener(span, storageFactory);
final AsyncState asyncState = new ListenableAsyncState(traceRoot, asyncStateListener, handle, uriStatStorage);

final SpanRecorder spanRecorder = recorderFactory.newSpanRecorder(span, traceId.isRoot(), sampling);
final SpanRecorder spanRecorder = recorderFactory.newSpanRecorder(span, traceId.isRoot());
final WrappedSpanEventRecorder wrappedSpanEventRecorder = recorderFactory.newWrappedSpanEventRecorder(traceRoot, asyncState);


final DefaultTrace trace = new DefaultTrace(span, callStack, storage, sampling, spanRecorder, wrappedSpanEventRecorder);
final DefaultTrace trace = new DefaultTrace(span, callStack, storage, spanRecorder, wrappedSpanEventRecorder);

return new AsyncTrace(traceRoot, trace, asyncState);
} else {
Expand Down Expand Up @@ -225,11 +222,11 @@ Trace newAsyncTraceObject(TraceSampler.State state) {


final TraceId traceId = traceRoot.getTraceId();
final SpanRecorder spanRecorder = recorderFactory.newSpanRecorder(span, traceId.isRoot(), sampling);
final SpanRecorder spanRecorder = recorderFactory.newSpanRecorder(span, traceId.isRoot());
final WrappedSpanEventRecorder wrappedSpanEventRecorder = recorderFactory.newWrappedSpanEventRecorder(traceRoot, asyncState);


final DefaultTrace trace = new DefaultTrace(span, callStack, storage, sampling, spanRecorder, wrappedSpanEventRecorder);
final DefaultTrace trace = new DefaultTrace(span, callStack, storage, spanRecorder, wrappedSpanEventRecorder);

return new AsyncTrace(traceRoot, trace, asyncState);
} else {
Expand Down
8 changes: 3 additions & 5 deletions profiler/src/main/java/com/navercorp/pinpoint/profiler/context/DefaultTrace.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
import com.navercorp.pinpoint.bootstrap.context.TraceId;
import com.navercorp.pinpoint.bootstrap.context.scope.TraceScope;
import com.navercorp.pinpoint.common.annotations.VisibleForTesting;
import com.navercorp.pinpoint.common.util.Assert;
import com.navercorp.pinpoint.exception.PinpointException;
import com.navercorp.pinpoint.profiler.context.active.ActiveTraceHandle;
import com.navercorp.pinpoint.profiler.context.id.Shared;
Expand Down Expand Up @@ -64,20 +63,19 @@ public final class DefaultTrace implements Trace {
@Nullable
private final UriStatStorage uriStatStorage;

public DefaultTrace(Span span, CallStack<SpanEvent> callStack, Storage storage, boolean sampling,
public DefaultTrace(Span span, CallStack<SpanEvent> callStack, Storage storage,
SpanRecorder spanRecorder, WrappedSpanEventRecorder wrappedSpanEventRecorder) {
this(span, callStack, storage, sampling, spanRecorder, wrappedSpanEventRecorder, null, null);
this(span, callStack, storage, spanRecorder, wrappedSpanEventRecorder, null, null);
}

public DefaultTrace(Span span, CallStack<SpanEvent> callStack, Storage storage, boolean sampling,
public DefaultTrace(Span span, CallStack<SpanEvent> callStack, Storage storage,
SpanRecorder spanRecorder, WrappedSpanEventRecorder wrappedSpanEventRecorder,
ActiveTraceHandle activeTraceHandle,
UriStatStorage uriStatStorage) {

this.span = Objects.requireNonNull(span, "span");
this.callStack = Objects.requireNonNull(callStack, "callStack");
this.storage = Objects.requireNonNull(storage, "storage");
Assert.isTrue(sampling, "sampling must be true");

this.spanRecorder = Objects.requireNonNull(spanRecorder, "spanRecorder");
this.wrappedSpanEventRecorder = Objects.requireNonNull(wrappedSpanEventRecorder, "wrappedSpanEventRecorder");
Expand Down
8 changes: 4 additions & 4 deletions ...rc/main/java/com/navercorp/pinpoint/profiler/context/recorder/DefaultRecorderFactory.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,13 +50,13 @@ public DefaultRecorderFactory(Provider<AsyncContextFactory> asyncContextFactoryP
}

@Override
public SpanRecorder newSpanRecorder(Span span, boolean isRoot, boolean sampling) {
return new DefaultSpanRecorder(span, isRoot, sampling, stringMetaDataService, sqlMetaDataService, errorHandler);
public SpanRecorder newSpanRecorder(Span span, boolean isRoot) {
return new DefaultSpanRecorder(span, isRoot, stringMetaDataService, sqlMetaDataService, errorHandler);
}

@Override
public SpanRecorder newTraceRootSpanRecorder(TraceRoot traceRoot, boolean sampling) {
return new TraceRootSpanRecorder(traceRoot, sampling);
public SpanRecorder newTraceRootSpanRecorder(TraceRoot traceRoot) {
return new TraceRootSpanRecorder(traceRoot);
}

@Override
Expand Down
12 changes: 5 additions & 7 deletions ...r/src/main/java/com/navercorp/pinpoint/profiler/context/recorder/DefaultSpanRecorder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,25 +29,23 @@
import org.apache.logging.log4j.Logger;

/**
*
*
* @author jaehong.kim
*
*/
public class DefaultSpanRecorder extends AbstractRecorder implements SpanRecorder {
private static final Logger logger = LogManager.getLogger(DefaultTrace.class.getName());
private static final boolean isDebug = logger.isDebugEnabled();

private final Span span;
private final boolean isRoot;
private final boolean sampling;

public DefaultSpanRecorder(final Span span, final boolean isRoot, final boolean sampling,

public DefaultSpanRecorder(final Span span, final boolean isRoot,
final StringMetaDataService stringMetaDataService, SqlMetaDataService sqlMetaDataService,
final IgnoreErrorHandler errorHandler) {
super(stringMetaDataService, sqlMetaDataService, errorHandler);
this.span = span;
this.isRoot = isRoot;
this.sampling = sampling;
}

public Span getSpan() {
Expand Down Expand Up @@ -124,7 +122,7 @@ public void recordAcceptorHost(String host) {

@Override
public boolean canSampled() {
return sampling;
return true;
}

@Override
Expand Down
4 changes: 2 additions & 2 deletions profiler/src/main/java/com/navercorp/pinpoint/profiler/context/recorder/RecorderFactory.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,9 @@
*/
public interface RecorderFactory {

SpanRecorder newSpanRecorder(final Span span, final boolean isRoot, final boolean sampling);
SpanRecorder newSpanRecorder(final Span span, final boolean isRoot);

SpanRecorder newTraceRootSpanRecorder(TraceRoot traceRoot, boolean sampling);
SpanRecorder newTraceRootSpanRecorder(TraceRoot traceRoot);

SpanRecorder newDisableSpanRecorder(LocalTraceRoot traceRoot);

Expand Down
7 changes: 2 additions & 5 deletions ...src/main/java/com/navercorp/pinpoint/profiler/context/recorder/TraceRootSpanRecorder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,17 +33,14 @@
public class TraceRootSpanRecorder implements SpanRecorder {

private final TraceRoot traceRoot;
private final boolean sampling;

public TraceRootSpanRecorder(TraceRoot traceRoot, boolean sampling) {
public TraceRootSpanRecorder(TraceRoot traceRoot) {
this.traceRoot = Objects.requireNonNull(traceRoot, "traceRoot");

this.sampling = sampling;
}

@Override
public boolean canSampled() {
return sampling;
return true;
}

@Override
Expand Down
4 changes: 2 additions & 2 deletions profiler/src/test/java/com/navercorp/pinpoint/profiler/context/DefaultTraceTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -165,9 +165,9 @@ private Trace newTrace(final int maxCallStackDepth) {

final Span span = spanFactory.newSpan(traceRoot);
final boolean root = span.getTraceRoot().getTraceId().isRoot();
final SpanRecorder spanRecorder = new DefaultSpanRecorder(span, root, true, stringMetaDataService, sqlMetaDataService, errorHandler);
final SpanRecorder spanRecorder = new DefaultSpanRecorder(span, root, stringMetaDataService, sqlMetaDataService, errorHandler);
final WrappedSpanEventRecorder wrappedSpanEventRecorder = new WrappedSpanEventRecorder(traceRoot, asyncContextFactory, stringMetaDataService, sqlMetaDataService, errorHandler);

return new DefaultTrace(span, callStack, storage, true, spanRecorder, wrappedSpanEventRecorder, ActiveTraceHandle.EMPTY_HANDLE, uriStatStorage);
return new DefaultTrace(span, callStack, storage, spanRecorder, wrappedSpanEventRecorder, ActiveTraceHandle.EMPTY_HANDLE, uriStatStorage);
}
}
8 changes: 4 additions & 4 deletions profiler/src/test/java/com/navercorp/pinpoint/profiler/context/TraceTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,15 +74,15 @@ public void trace() {
final Span span = newSpan(traceRoot);

boolean root = span.getTraceRoot().getTraceId().isRoot();
SpanRecorder spanRecorder = new DefaultSpanRecorder(span, root, true, stringMetaDataService, sqlMetaDataService, errorHandler);
SpanRecorder spanRecorder = new DefaultSpanRecorder(span, root, stringMetaDataService, sqlMetaDataService, errorHandler);
WrappedSpanEventRecorder wrappedSpanEventRecorder = new WrappedSpanEventRecorder(traceRoot, asyncContextFactory, stringMetaDataService, sqlMetaDataService, errorHandler);

AsyncContextFactory asyncContextFactory = mock(AsyncContextFactory.class);

Storage storage = mock(Storage.class);
UriStatStorage uriStatStorage = mock(UriStatStorage.class);

Trace trace = new DefaultTrace(span, callStack, storage, true, spanRecorder, wrappedSpanEventRecorder, ActiveTraceHandle.EMPTY_HANDLE, uriStatStorage);
Trace trace = new DefaultTrace(span, callStack, storage, spanRecorder, wrappedSpanEventRecorder, ActiveTraceHandle.EMPTY_HANDLE, uriStatStorage);
trace.traceBlockBegin();

// get data form db
Expand All @@ -108,7 +108,7 @@ public void popEventTest() {
final Span span = newSpan(traceRoot);

final boolean root = span.getTraceRoot().getTraceId().isRoot();
SpanRecorder spanRecorder = new DefaultSpanRecorder(span, root, true, stringMetaDataService, sqlMetaDataService, errorHandler);
SpanRecorder spanRecorder = new DefaultSpanRecorder(span, root, stringMetaDataService, sqlMetaDataService, errorHandler);
WrappedSpanEventRecorder wrappedSpanEventRecorder = new WrappedSpanEventRecorder(traceRoot, asyncContextFactory, stringMetaDataService, sqlMetaDataService, errorHandler);


Expand All @@ -117,7 +117,7 @@ public void popEventTest() {
Storage storage = mock(Storage.class);
UriStatStorage uriStatStorage = mock(UriStatStorage.class);

Trace trace = new DefaultTrace(span, callStack, storage, true, spanRecorder, wrappedSpanEventRecorder,
Trace trace = new DefaultTrace(span, callStack, storage, spanRecorder, wrappedSpanEventRecorder,
ActiveTraceHandle.EMPTY_HANDLE, uriStatStorage);

trace.close();
Expand Down
8 changes: 4 additions & 4 deletions ...c/test/java/com/navercorp/pinpoint/profiler/context/recorder/DefaultSpanRecorderTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,10 +54,10 @@ public class DefaultSpanRecorderTest {
private final IgnoreErrorHandler errorHandler = new BypassErrorHandler();

@Test
public void testRecordApiId() throws Exception {
public void testRecordApiId() {
Span span = new Span(traceRoot);

SpanRecorder recorder = new DefaultSpanRecorder(span, true, true, stringMetaDataService, sqlMetaDataService, errorHandler);
SpanRecorder recorder = new DefaultSpanRecorder(span, true, stringMetaDataService, sqlMetaDataService, errorHandler);

final int API_ID = 1000;
recorder.recordApiId(API_ID);
Expand All @@ -66,13 +66,13 @@ public void testRecordApiId() throws Exception {
}

@Test
public void testRecordEndPoint() throws Exception {
public void testRecordEndPoint() {

when(traceRoot.getShared()).thenReturn(shared);

Span span = new Span(traceRoot);

SpanRecorder recorder = new DefaultSpanRecorder(span, true, true, stringMetaDataService, sqlMetaDataService, errorHandler);
SpanRecorder recorder = new DefaultSpanRecorder(span, true, stringMetaDataService, sqlMetaDataService, errorHandler);

final String endPoint = "endPoint";
recorder.recordEndPoint(endPoint);
Expand Down

0 comments on commit 74275da

Please # to comment.