Add custom sampler Trace Plugin to sample individual spans #50
Conversation
Pull Request Test Coverage Report for Build 325
💛 - Coveralls |
| @@ -10,7 +10,7 @@ class TraceSamplerConfig { | |||
| errorAwareSamplerConfig: ErrorAwareSamplerConfig; | |||
| durationAwareSampler: DurationAwareSampler; | |||
| errorAwareSampler: ErrorAwareSampler; | |||
| customSampler: () => Sampler<ThundraSpan>; | |||
| customSampler: Sampler<ThundraSpan>; | |||
There was a problem hiding this comment.
where we use customSampler for sampling?
src/plugins/Trace.ts
Outdated
| if (sampled) { | ||
| for (const span of spanList) { | ||
| if (span) { | ||
| if (customSampler && !customSampler.isSampled(span)) { |
There was a problem hiding this comment.
Individual span should not be sampled but root span. Because it might break parent-child relation between spans and leads to problems.
There was a problem hiding this comment.
So if root span is not sampled, all of the others spans should not be sampled and ignored
There was a problem hiding this comment.
User should explicitly set runCustomSamplerOnEachSpan to apply custom sampler on each span and the responsibility will be on them. When runCustomSamplerOnEachSpan is not set
or set the false, custom sampler is applied to only root span and root span is not sampled no chlid span is sampled.
The configuration will look like this.
const config = {
traceConfig: {
samplerConfig: {
runCustomSamplerOnEachSpan: true,
customSampler:{
isSampled(span) {
// custom sample logic is applied here
return true;
}
}
}
}
}
| @@ -44,6 +49,14 @@ class TraceSamplerConfig { | |||
| isSampled = isSampled || this.errorAwareSampler.isSampled(span); | |||
| } | |||
|
|
|||
| if (this.customSampler) { | |||
| if (this.runCustomSamplerOnEachSpan) { | |||
| isSampled = true; | |||
There was a problem hiding this comment.
Why we didn't call custom sampler and set isSampled to true directly even it might be set to false (decided not to be sampled) above by durationAwareSampler and/or errorAwareSampler?
There was a problem hiding this comment.
if this.runCustomSamplerOnEachSpan is true. We run custom sampler on each span including root span. We skip running on custom sampler on root span by returning isSampled = true;. The running custom sampler on root span along with all spans are done in trace plugin here. https://github.com/thundra-io/thundra-lambda-agent-nodejs/pull/50/files/a44c6f9885e16034752d8d6133eeef67ccd3c6ef..187d3d3000bc7216416a2f4c290cd71a9f79a41e#diff-9a3803680d4e920a1c7139b6469e0613R201
Basically, I did it to prevent running custom sampler on root span twice.
No description provided.