Properly handle ? sampling decision
- Dominant language
- JavaScript
- Stars
- 280
- Forks
- 157
- PR merge metrics
- No merged PRs in 30d
Description
As pointed out in the changes to trace header behavior in #284, we are currently treating the `Sampled=?` trait in the trace header as a no-sample decision. That is incorrect behavior, and instead we should explicitly treat it as "to be decided." This can be implemented by creating an enum of sampling decisions like we have in [Java](https://github.com/aws/aws-xray-sdk-java/blob/3e0b21c5bafec8d0577768cdfc31f4139c4fbecc/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/TraceHeader.java#L17-L18).
This will be a pretty large rewrite since currently the sampling decision of a segment is represented by a binary `not_traced` flag, so we'll have to extend or replace that to have more than two possibilities. We should make sure that whatever we choose the field is not serialized when the segment gets streamed.
[Here](https://github.com/aws/aws-xray-sdk-node/blob/master/packages/core/lib/utils.js#L177-L180) is where we parse in sampling decision, [here](https://github.com/aws/aws-xray-sdk-node/blob/test-aws-xray-sdk-express%403.0.1/packages/core/test/unit/middleware/mw_utils.test.js#L202-L208) is a test we'll have to correct afterward.
Contributor guide
Research direction
Start in packages/core/lib/utils.js around the sampling-decision parser and inspect the segment representation and streaming path. Update the behavior covered by packages/core/test/unit/middleware/mw_utils.test.js so Sampled=? means undecided, then verify that the sampling decision is not serialized when the segment is streamed.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- javascript
- Domain
- observability-sre
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100