workflows: start tracing action and match sampling#412
workflows: start tracing action and match sampling#412mattklein123 wants to merge 1 commit intomainfrom
Conversation
Signed-off-by: Matt Klein <mklein@bitdrift.io>
| return true; | ||
| } | ||
|
|
||
| rng.random_u32(SAMPLE_RATE_DENOMINATOR) < sample_rate |
There was a problem hiding this comment.
I know this would make this a bit more complicated but should we do what envoy does and roll the dice once per workflow execution to avoid different sampling decisions to be reached by different workflows evaluating the same workflow? With multiple workflows using a sample node you would roll the dice N times and since we dedupe counters it would then appear that we are counting more than the sample rate?
There was a problem hiding this comment.
Yeah not sure it's worth it we can discuss. We would have to persist the random value. Realistically I can't see this being used more than once per workflow but dunno.
There was a problem hiding this comment.
I wasn't thinking about doing it once per workflow but once per log such that multiple workflows would make a consistent sampling decision, like in the A->B(sampled) workflow and A->B(sampled)->C case
There was a problem hiding this comment.
Ah sure that's easy. Can do.
| self.state.active_run_tracing_count = 0; | ||
| self.state.active_streaming_tracing_count = 0; | ||
| // TODO(mattklein123): Switch pending flush actions to a map. | ||
| self.state.pending_flush_actions = std::mem::take(&mut self.state.pending_flush_actions) |
There was a problem hiding this comment.
Just so I understand, if tracing is its own action why does it seem so tied to the flush actions?
| 0 | ||
| }; | ||
|
|
||
| let traced_run_ended = |
There was a problem hiding this comment.
I guess what we're saying is that tracing extends to any streaming logs that may be emitted while we're tracing? Maybe add a comment explaining the interaction somewhere unless I missed it
There was a problem hiding this comment.
Yeah exactly we decided that tracing would extend through the post flush stream. I will add more comments.
2 participants
Summary
SampledMatchinbd-log-matcherwith deterministic RNG abstraction and testsStartTracinglifecycle in workflows/logger, including persisted active-state behavior and streaming carryover handlingis_tracing_activeand remove transition-onlystart_trace_samplingsignalingValidation
cargo clippy -p bd-workflows --tests -- --no-depscargo nextest run -p bd-workflowscargo nextest run -p bd-loggercargo nextest run --workspaceFixes BIT-7464