From 5f5dda115162e836ace97bb08993d3a1fed4c701 Mon Sep 17 00:00:00 2001 From: William Armiros Date: Sat, 22 Aug 2020 12:48:13 -0700 Subject: [PATCH 1/3] added beginSegmentWithSampling --- .../main/java/com/amazonaws/xray/AWSXRay.java | 4 ++ .../com/amazonaws/xray/AWSXRayRecorder.java | 26 +++++++++++++ .../xray/entities/AWSLogReference.java | 1 - .../sampling/CentralizedSamplingStrategy.java | 15 +++++++- .../sampling/rule/CentralizedRule.java | 8 +++- .../amazonaws/xray/AWSXRayRecorderTest.java | 37 +++++++++++++++++++ 6 files changed, 86 insertions(+), 5 deletions(-) diff --git a/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/AWSXRay.java b/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/AWSXRay.java index a173bfbe..b6c0072b 100644 --- a/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/AWSXRay.java +++ b/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/AWSXRay.java @@ -92,6 +92,10 @@ public static void createSubsegment(String name, Runnable runnable) { globalRecorder.createSubsegment(name, runnable); } + public static Segment beginSegmentWithSampling(String name) { + return globalRecorder.beginSegmentWithSampling(name); + } + public static Segment beginSegment(String name) { return globalRecorder.beginSegment(name); } diff --git a/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/AWSXRayRecorder.java b/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/AWSXRayRecorder.java index db59e426..55eee072 100644 --- a/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/AWSXRayRecorder.java +++ b/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/AWSXRayRecorder.java @@ -39,6 +39,8 @@ import com.amazonaws.xray.strategy.StreamingStrategy; import com.amazonaws.xray.strategy.ThrowableSerializationStrategy; import com.amazonaws.xray.strategy.sampling.DefaultSamplingStrategy; +import com.amazonaws.xray.strategy.sampling.SamplingRequest; +import com.amazonaws.xray.strategy.sampling.SamplingResponse; import com.amazonaws.xray.strategy.sampling.SamplingStrategy; import java.io.IOException; import java.io.InputStream; @@ -392,6 +394,23 @@ public Segment beginSegment(String name) { return beginSegment(new SegmentImpl(this, name)); } + /** + * Begins a new segment after applying the configured sampling strategy. This method only uses the segment name and origin + * (if defined) to compute a sampling decision. + * + * @param name the segment name, to be used for the sampling decision + * @return Returns a proper segment if a sampled decision is made, and a no-op segment otherwise. + */ + public Segment beginSegmentWithSampling(String name) { + final SamplingRequest samplingRequest = new SamplingRequest(name, null, null, null, this.origin); + final SamplingResponse samplingResponse = this.getSamplingStrategy().shouldTrace(samplingRequest); + if (samplingResponse.isSampled()) { + return beginSegment(name); + } + + return beginNoOpSegment(); + } + public Segment beginSegment(String name, TraceID traceId, @Nullable String parentId) { Segment segment = new SegmentImpl(this, name, traceId); segment.setParentId(parentId); @@ -491,6 +510,13 @@ public void endSegment() { Entity current = getTraceEntity(); if (current != null) { Segment segment = current.getParentSegment(); + + // Return immediately if ending a no-op segment + if (!segment.isRecording()) { + clearTraceEntity(); + return; + } + logger.debug("Ending segment named '" + segment.getName() + "'."); segmentListeners diff --git a/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/AWSLogReference.java b/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/AWSLogReference.java index d16718e9..3ff700bc 100644 --- a/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/AWSLogReference.java +++ b/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/AWSLogReference.java @@ -63,7 +63,6 @@ public void setArn(final String arn) { /** * Compares ARN and log group between references to determine equality. - * @param reference * @return */ @Override diff --git a/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/strategy/sampling/CentralizedSamplingStrategy.java b/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/strategy/sampling/CentralizedSamplingStrategy.java index abccd8e6..f7f67f33 100644 --- a/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/strategy/sampling/CentralizedSamplingStrategy.java +++ b/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/strategy/sampling/CentralizedSamplingStrategy.java @@ -98,8 +98,19 @@ public SamplingResponse shouldTrace(SamplingRequest samplingRequest) { if (!applicable) { continue; } - logger.debug("Applicable rule:" + rule.getName()); - return rule.sample(Instant.now()); + + if (logger.isDebugEnabled()) { + logger.debug("Applicable rule:" + rule.getName()); + } + + SamplingResponse response = rule.sample(Instant.now()); + + if (logger.isDebugEnabled()) { + logger.debug("Segment " + samplingRequest.getService().orElse("") + " has" + + (response.isSampled() ? " " : " NOT ") + "been sampled."); + } + + return response; } // Match against default rule diff --git a/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/strategy/sampling/rule/CentralizedRule.java b/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/strategy/sampling/rule/CentralizedRule.java index 623b0a3c..5da51144 100644 --- a/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/strategy/sampling/rule/CentralizedRule.java +++ b/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/strategy/sampling/rule/CentralizedRule.java @@ -199,14 +199,18 @@ private SamplingResponse doSampleCustomerRule(Instant now, SamplingResponse res, // Attempt to borrow request if (centralizedReservoir.isBorrow(now)) { - logger.debug("Sampling target has expired for rule " + getName() + ". Burrowing a request."); + logger.debug("Sampling target has expired for rule " + getName() + ". Borrowing a request."); statistics.incBorrowed(); res.setSampled(true); return res; } - logger.debug("Sampling target has expired for rule " + getName() + ". Using fixed rate."); + if (logger.isDebugEnabled()) { + logger.debug(String.format("Sampling target has expired for rule %s. Using fixed rate of %d percent.", + getName(), (int) (fixedRate*100))); + } + // Fallback to bernoulli sampling if (random < fixedRate) { statistics.incSampled(); diff --git a/aws-xray-recorder-sdk-core/src/test/java/com/amazonaws/xray/AWSXRayRecorderTest.java b/aws-xray-recorder-sdk-core/src/test/java/com/amazonaws/xray/AWSXRayRecorderTest.java index dc6842cb..d42eb702 100644 --- a/aws-xray-recorder-sdk-core/src/test/java/com/amazonaws/xray/AWSXRayRecorderTest.java +++ b/aws-xray-recorder-sdk-core/src/test/java/com/amazonaws/xray/AWSXRayRecorderTest.java @@ -38,6 +38,9 @@ import com.amazonaws.xray.strategy.LogErrorContextMissingStrategy; import com.amazonaws.xray.strategy.RuntimeErrorContextMissingStrategy; import com.amazonaws.xray.strategy.sampling.LocalizedSamplingStrategy; +import com.amazonaws.xray.strategy.sampling.SamplingRequest; +import com.amazonaws.xray.strategy.sampling.SamplingResponse; +import com.amazonaws.xray.strategy.sampling.SamplingStrategy; import com.fasterxml.jackson.databind.node.JsonNodeFactory; import com.fasterxml.jackson.databind.node.ObjectNode; import java.util.ArrayList; @@ -825,4 +828,38 @@ public void noOpSubsegmentWithParent() { Subsegment subsegment = Subsegment.noOp(parent, AWSXRay.getGlobalRecorder()); assertThat(subsegment.getParentSegment().getTraceId()).isEqualTo(traceID); } + + @Test + public void testBeginSegmentWithSamplingDoesSample() { + AWSXRay.getGlobalRecorder().setSamplingStrategy(new TestSamplingStrategy(true)); + + Segment segment = AWSXRay.beginSegmentWithSampling("test"); + assertThat(segment.isSampled()).isTrue(); + } + + @Test + public void testBeginSegmentWithSamplingDoesNotSample() { + AWSXRay.getGlobalRecorder().setSamplingStrategy(new TestSamplingStrategy(false)); + + Segment segment = AWSXRay.beginSegmentWithSampling("test"); + assertThat(segment.isSampled()).isFalse(); + } + + private static class TestSamplingStrategy implements SamplingStrategy { + boolean sampled; + + TestSamplingStrategy(boolean sampled) { + this.sampled = sampled; + } + + @Override + public SamplingResponse shouldTrace(SamplingRequest sampleRequest) { + return new SamplingResponse(sampled, "rule"); + } + + @Override + public boolean isForcedSamplingSupported() { + return false; + } + } } From 1b85b3e68e9f49a4df2ae751990c3fbb18dcce8f Mon Sep 17 00:00:00 2001 From: William Armiros Date: Sat, 22 Aug 2020 12:53:02 -0700 Subject: [PATCH 2/3] checkstyle --- .../amazonaws/xray/strategy/sampling/rule/CentralizedRule.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/strategy/sampling/rule/CentralizedRule.java b/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/strategy/sampling/rule/CentralizedRule.java index 5da51144..4f9e3544 100644 --- a/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/strategy/sampling/rule/CentralizedRule.java +++ b/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/strategy/sampling/rule/CentralizedRule.java @@ -208,7 +208,7 @@ private SamplingResponse doSampleCustomerRule(Instant now, SamplingResponse res, if (logger.isDebugEnabled()) { logger.debug(String.format("Sampling target has expired for rule %s. Using fixed rate of %d percent.", - getName(), (int) (fixedRate*100))); + getName(), (int) (fixedRate * 100))); } // Fallback to bernoulli sampling From 1535dc157b19964cb7e3f0bd1b2e2eeca2215297 Mon Sep 17 00:00:00 2001 From: William Armiros Date: Sun, 23 Aug 2020 11:01:56 -0700 Subject: [PATCH 3/3] switched to using concatenation --- .../xray/strategy/sampling/rule/CentralizedRule.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/strategy/sampling/rule/CentralizedRule.java b/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/strategy/sampling/rule/CentralizedRule.java index 4f9e3544..1fdf5534 100644 --- a/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/strategy/sampling/rule/CentralizedRule.java +++ b/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/strategy/sampling/rule/CentralizedRule.java @@ -207,8 +207,8 @@ private SamplingResponse doSampleCustomerRule(Instant now, SamplingResponse res, } if (logger.isDebugEnabled()) { - logger.debug(String.format("Sampling target has expired for rule %s. Using fixed rate of %d percent.", - getName(), (int) (fixedRate * 100))); + logger.debug("Sampling target has expired for rule " + getName() + ". Using fixed rate of " + + (int) (fixedRate * 100) + " percent."); } // Fallback to bernoulli sampling