Skip to content

feat(api): add random-trace-id flag to TraceFlags - #142

Open
harshitt13 wants to merge 1 commit into
MindfulSoftwareLLC:mainfrom
harshitt13:feat/104-traceflags-random-flag
Open

harshitt13 wants to merge 1 commit into
MindfulSoftwareLLC:mainfrom
harshitt13:feat/104-traceflags-random-flag

Conversation

@harshitt13

Copy link
Copy Markdown
Collaborator

Refs #104

What

Adds RANDOM_FLAG (0x02), isRandom and withRandom to TraceFlags, mirroring the existing sampled pair.

The flags byte already survived propagation via asByte, so an incoming 02 was re-emitted as 02 by accident of the whole byte being copied. What was missing was the ability to express the bit: nothing could ask whether an incoming trace ID is declared random, and the SDK had no supported way to set it on a root span context.

W3C Trace Context Level 2 makes preserving the bit across a continued trace a MUST. https://www.w3.org/TR/trace-context-2/#random-trace-id-flag

Notes

  • withRandom does not verify randomness and cannot. Setting the flag is an assertion by the caller; enforcing it belongs to the SDK, which only sets it for a conforming ID generator. Documented on the method.
  • Class doc now states that TraceFlags is a bit set and that flags == TraceFlags.sampled is wrong once a second bit exists.
  • Changelog entry under Added, not Fixed. Nothing changed for existing callers, so this is new public surface under Keep a Changelog. Per the conventions comment the spec citation carries the audit provenance, and fix(api): Context-driven span creation and root span support (#83, #84) #118 set the precedent of an audit item landing its additive surface under Added.
  • No TraceFlags.random constant. The class doc argues against having named whole-value constants to compare against.
  • Out of scope: fromString leniency ([BUG] Trace API: TraceFlags.fromString coerces invalid input to 0x00 (API half) #92), and the mutable none and sampled statics, which I filed separately as #NNN.

Follow-up: whole-value comparisons in tests

Grepping == TraceFlags and equals(TraceFlags...) turns up 9 sites. Only 4 should change.

Wrong model, assert "is sampled" via whole-value equality: span_context_test.dart lines 73, 119, 168, 169.

Legitimate, leave alone, round-trip assertions where the whole byte is the subject: span_context_test.dart lines 56, 254, 261, 281, and span_context_create_test.dart:36.

The criterion is what the assertion is about. A round trip on withTraceFlags asserts the setter stored the byte you handed it. A sampling check asserts a predicate, and equality only answers it while no other bit is set.

The clearest case is line 150, a test named 'trace flags indicates sampling state' whose entire subject is the sampling predicate, tested by whole-value equality. Three of the four also carry a comment reading // Use accessor method instead of isSampled property directly above an == comparison, which has it backwards.

All 4 are inert today: they build their flags explicitly, so no random bit reaches them. They fail once root spans start emitting 03. git log -L puts them in the initial revision, so it is original habit rather than a later sweep, which means the SDK repo likely carries the same idiom from the same authorship. That is the urgent half, since that is where #132 makes root spans 03 for real.

Testing

Covers all four combinations of the two bits, both orders of application, that each with* preserves the other bit, and round trips through fromString/toString.

Signed-off-by: harshitt13 <find.harshitkushwaha@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant