Oversampling mitigation - #354
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #354 +/- ##
============================================
+ Coverage 58.30% 58.54% +0.23%
- Complexity 1209 1234 +25
============================================
Files 131 133 +2
Lines 4948 4986 +38
Branches 590 595 +5
============================================
+ Hits 2885 2919 +34
- Misses 1788 1793 +5
+ Partials 275 274 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Sample code in Lambda Test 1 - This is the JSON sent to lambda Test 2 - Note the different Sampled value |
|
|
||
| import com.amazonaws.services.lambda.runtime.events.SQSEvent; | ||
|
|
||
| public final class SQSMessageHelper { |
There was a problem hiding this comment.
XRayLambdaSQSMessageHelper? it might be a horrible name.
There was a problem hiding this comment.
I think it makes sense to change the package/module name from SQS to lambda. But let's keep the class name concise. This is what they did in the Lambda library where SQSMessage lives.
|
|
||
| tasks.jar { | ||
| manifest { | ||
| attributes("Automatic-Module-Name" to "com.amazonaws.xray_sqs") |
There was a problem hiding this comment.
Module name could be lambda helper since it is help Lambda user, not for sqs instrumentation
| package com.amazonaws.xray.internal; | ||
|
|
||
| public enum SamplingStrategyOverride { | ||
| DISABLED, // Does not override the SamplingStrategy. |
There was a problem hiding this comment.
Not quite get why have to use DISABLED.
Can we use existing API, then update the sampled flag in SegmentImpl?
There was a problem hiding this comment.
If we are changing the parent's flag (the Lambda Frontend Segment), then will that not cause an issue in multithreading?
|
|
||
| boolean isRecording = (parentSegment.isRecording() && | ||
| samplingStrategyOverride == SamplingStrategyOverride.DISABLED) || | ||
| samplingStrategyOverride == SamplingStrategyOverride.TRUE; |
There was a problem hiding this comment.
When parentSegment.isRecording() == false and samplingStrategyOverride == SamplingStrategyOverride.TRUE, should we create a new subsegment?
There was a problem hiding this comment.
Yes. The override to true will ignore any other indicators.
I guess this creates a problem of an orphaned subsegment?


Issue #, if available:
Description of changes:
Oversampling Mitigation and SQS Helper
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.