Skip to content

Commit

Permalink
From Peter's feedback: Root samplers MAY set explicit randomness. SDK…
Browse files Browse the repository at this point in the history 
…s are not required to. Built-in samplers are not required to.
  • Loading branch information
@jmacd
jmacd committed Oct 30, 2024
1 parent f336fb1 commit b75113c
Showing 1 changed file with 13 additions and 9 deletions.
22 changes: 13 additions & 9 deletions specification/trace/sdk.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -525,17 +525,21 @@ TraceID randomness, above. It has two parts.

API users control the initial TraceState of a root span, so they can
provide explicit trace randomness for a trace by defining the [`rv`
sub-key of the OpenTelemetry TraceState][OTELRVALUE]. SDKs MUST NOT
overwrite explicit trace randomness in an OpenTelemetry TraceState
sub-key of the OpenTelemetry TraceState][OTELRVALUE]. SDKs and Samplers
MUST NOT overwrite explicit trace randomness in an OpenTelemetry TraceState
value.

##### Set explicit trace randomness for non-random TraceIDs
##### Root samplers set explicit trace randomness for non-random TraceIDs

For root span contexts, when the SDK generates a TraceID that does not
meet the [W3C Trace Context Level 2 randomness requirements][W3CCONTEXTTRACEID],
and when the the [`rv` sub-key of the OpenTelemetry TraceState][OTELRVALUE] is
not already set, the SDK SHOULD insert an explicit trace randomness value
into the OpenTelemetry TraceState value containing 56 random bits.
When the SDK has generated a TraceID that does not meet the [W3C Trace
Context Level 2 randomness requirements][W3CCONTEXTTRACEID], indicated
by an unset trace random flag, and when the the [`rv` sub-key of the
OpenTelemetry TraceState][OTELRVALUE] is not already set, the Root
sampler has the opportunity to insert explicit trace randomness.

Root Samplers MAY insert an explicit trace randomness value into the
OpenTelemetry TraceState value in cases where an explicit trace
randomness value is not already set.

This comment has been minimized.

Sign in to view

Copy link
@PeterF778

PeterF778 Oct 31, 2024 •
edited
Loading

This is too strong, I'd remove "in cases where an explicit trace randomness value is not already set". We need to permit Root Samplers to overwrite the rv value set by SDK.

I think it should work like this:

  • if the SDK can guarantee that the lower 56 bits of the generated TraceId are random, it SHOULD set the random flag, otherwise it SHOULD generate and set the rv value,
  • Consistent Probability Root Samplers MUST generate and set the rv if there's no rv and no random flag,
  • Root Samplers MAY generate and set the rv value even if the above condition is not met

This comment has been minimized.

Sign in to view

Copy link
@jmacd

jmacd Oct 31, 2024

Author Contributor

Open question: about "otherwise it SHOULD generate and set the rv value," whether it is important to have this as a requirement. Without this requirement, SDKs can leave out a potentially unnecessary bit of code.

For example, here's a W3C Trace Context with non-random identifiers and an
explicit randomness value:
Expand All @@ -551,7 +555,7 @@ For all span contexts, OpenTelemetry samplers SHOULD presume that TraceIDs meet

#### IdGenerator randomness

If the SDK uses an `IdGenerator` extension point, the SDK SHOULD allow the extension to determine whether the Random flag is set when new IDs are generated. When an `IdGenerator` instance does not meet the randomness requirements, the SDK SHOULD insert explicit randomness instead, otherwise samplers will incorrectly presume TraceID randomness.
If the SDK uses an `IdGenerator` extension point, the SDK SHOULD allow the extension to determine whether the Random flag is set when new IDs are generated.

## Span Limits

Expand Down

0 comments on commit b75113c

Please sign in to comment.