From 77bbb732271ebc8be48d27e69e82b87e72d8800f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Neum=C3=BCller?= Date: Thu, 24 Sep 2020 17:20:50 +0200 Subject: [PATCH 01/13] Specify Span ID creation with sampling. --- specification/trace/sdk.md | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/specification/trace/sdk.md b/specification/trace/sdk.md index 64303a39269..0427adfb7e0 100644 --- a/specification/trace/sdk.md +++ b/specification/trace/sdk.md @@ -60,11 +60,24 @@ The following table summarizes the expected behavior for each combination of The SDK defines the interface [`Sampler`](#sampler) as well as a set of [built-in samplers](#built-in-samplers) and associates a `Sampler` with each [`TracerProvider`]. -When asked to create a Span, the SDK MUST query the `Sampler`'s [`ShouldSample`](#shouldsample) method before actually creating the span, and act accordingly: -see description of [`ShouldSample`'s](#shouldsample) return value below for how to set `IsRecording` and `Sampled` on the Span, -and the [table above](#recording-sampled-reaction-table) on whether to pass the `Span` to `SpanProcessor`s. -A non-recording span MAY be implemented using the same mechanism as when a `Span` is created with no API-implementation installed -(sometimes called a `NoOpSpan` or `DefaultSpan`). +### SDK Span creation + +When asked to create a Span, the SDK MUST act as if doing the following in order: + +1. If there is a valid parent trace ID, use it. Otherwise generate a new one ( + note: this must be done before calling `ShouldSample`, because it expects + a valid trace Id as input). +2. Query the `Sampler`'s [`ShouldSample`](#shouldsample) method +3. If the decision is `DROP`, use the parent span ID or all-zero if there is none. + Otherwise (if the decision is not `DROP`) a new span ID MUST be generated. +4. Create a span depending on the decision returned by `ShouldSample`: + see description of [`ShouldSample`'s](#shouldsample) return value below + for how to set `IsRecording` and `Sampled` on the Span, + and the [table above](#recording-sampled-reaction-table) on whether + to pass the `Span` to `SpanProcessor`s. + A non-recording span MAY be implemented using the same mechanism as when a + `Span` is created with no API-implementation installed + (sometimes called a `NoOpSpan` or `DefaultSpan`). ### Sampler From 48b76e4c6d846e825e2b206add294e9933f11ba6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Neum=C3=BCller?= Date: Thu, 24 Sep 2020 17:28:25 +0200 Subject: [PATCH 02/13] Add CHANGELOG. --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index da2409a8cda..c9af36d551c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -64,6 +64,8 @@ Updates: and `RECORD_AND_SAMPLE` for consistency ([#938](https://github.com/open-telemetry/opentelemetry-specification/pull/938), [#956](https://github.com/open-telemetry/opentelemetry-specification/pull/956)) +- SDK: Specify when to generate new IDs with sampling + ([#998](https://github.com/open-telemetry/opentelemetry-specification/pull/998)) ## v0.6.0 (07-01-2020) From 57a4e255fe9c0c06568ea791c4e5a5d9115a486d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Neum=C3=BCller?= Date: Thu, 24 Sep 2020 18:13:35 +0200 Subject: [PATCH 03/13] Mismerge/lint. --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a71ac0b12f6..4518430ffe4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -83,7 +83,7 @@ Updates: ([#984](https://github.com/open-telemetry/opentelemetry-specification/pull/984) - Metrics SDK: Specify TBD default aggregation for ValueRecorder ([#984](https://github.com/open-telemetry/opentelemetry-specification/pull/984) - - SDK: Specify when to generate new IDs with sampling +- SDK: Specify when to generate new IDs with sampling ([#998](https://github.com/open-telemetry/opentelemetry-specification/pull/998)) ## v0.6.0 (07-01-2020) From c59c207b150194529ec712e991e416c594677183 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Neum=C3=BCller?= Date: Thu, 24 Sep 2020 18:17:59 +0200 Subject: [PATCH 04/13] Period. --- specification/trace/sdk.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/specification/trace/sdk.md b/specification/trace/sdk.md index 154c58a00ef..fe17a80a6be 100644 --- a/specification/trace/sdk.md +++ b/specification/trace/sdk.md @@ -67,7 +67,7 @@ When asked to create a Span, the SDK MUST act as if doing the following in order 1. If there is a valid parent trace ID, use it. Otherwise generate a new one ( note: this must be done before calling `ShouldSample`, because it expects a valid trace Id as input). -2. Query the `Sampler`'s [`ShouldSample`](#shouldsample) method +2. Query the `Sampler`'s [`ShouldSample`](#shouldsample) method. 3. If the decision is `DROP`, use the parent span ID or all-zero if there is none. Otherwise (if the decision is not `DROP`) a new span ID MUST be generated. 4. Create a span depending on the decision returned by `ShouldSample`: From d0fbf4e1c4ce2dba82871ad3df9d041fdf36d0d9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Neum=C3=BCller?= Date: Wed, 7 Oct 2020 19:34:18 +0200 Subject: [PATCH 05/13] Update specification/trace/sdk.md Co-authored-by: Armin Ruech --- specification/trace/sdk.md | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/specification/trace/sdk.md b/specification/trace/sdk.md index c4c83a406cc..973f22837c6 100644 --- a/specification/trace/sdk.md +++ b/specification/trace/sdk.md @@ -76,8 +76,7 @@ When asked to create a Span, the SDK MUST act as if doing the following in order and the [table above](#recording-sampled-reaction-table) on whether to pass the `Span` to `SpanProcessor`s. A non-recording span MAY be implemented using the same mechanism as when a - `Span` is created with no API-implementation installed - (sometimes called a `NoOpSpan` or `DefaultSpan`). + `Span` is created with no API-implementation installed (i.e., a [Propagated Span](api.md#propagated-span-creation)). ### Sampler From 40068167a1347e13ffaf06a44373d5731a655eba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Neum=C3=BCller?= Date: Mon, 12 Oct 2020 14:10:43 +0200 Subject: [PATCH 06/13] Ensure that even DROPed spans always have a valid ID. --- specification/trace/sdk.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/specification/trace/sdk.md b/specification/trace/sdk.md index 973f22837c6..4166bc0f55a 100644 --- a/specification/trace/sdk.md +++ b/specification/trace/sdk.md @@ -68,8 +68,9 @@ When asked to create a Span, the SDK MUST act as if doing the following in order note: this must be done before calling `ShouldSample`, because it expects a valid trace Id as input). 2. Query the `Sampler`'s [`ShouldSample`](#shouldsample) method. -3. If the decision is `DROP`, use the parent span ID or all-zero if there is none. - Otherwise (if the decision is not `DROP`) a new span ID MUST be generated. +3. If the decision is `DROP` and there is a valid parent span ID, use it. + Otherwise (if the decision is not `DROP` or there was no valid parent span ID) + a new span ID MUST be generated. 4. Create a span depending on the decision returned by `ShouldSample`: see description of [`ShouldSample`'s](#shouldsample) return value below for how to set `IsRecording` and `Sampled` on the Span, From 08b15ed25e6a5cc39736ce74f888f50eb4bdfeed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Neum=C3=BCller?= Date: Thu, 22 Oct 2020 13:29:37 +0200 Subject: [PATCH 07/13] Update specification/trace/sdk.md Co-authored-by: Armin Ruech --- specification/trace/sdk.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/specification/trace/sdk.md b/specification/trace/sdk.md index 4166bc0f55a..e08df9cb492 100644 --- a/specification/trace/sdk.md +++ b/specification/trace/sdk.md @@ -64,9 +64,9 @@ The SDK defines the interface [`Sampler`](#sampler) as well as a set of When asked to create a Span, the SDK MUST act as if doing the following in order: -1. If there is a valid parent trace ID, use it. Otherwise generate a new one ( - note: this must be done before calling `ShouldSample`, because it expects - a valid trace Id as input). +1. If there is a valid parent trace ID, use it. Otherwise generate a new one + (note: this must be done before calling `ShouldSample`, because it expects + a valid trace ID as input). 2. Query the `Sampler`'s [`ShouldSample`](#shouldsample) method. 3. If the decision is `DROP` and there is a valid parent span ID, use it. Otherwise (if the decision is not `DROP` or there was no valid parent span ID) From bfd652f60413c20bae8f7f3669badf8b53ba57eb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Neum=C3=BCller?= Date: Thu, 22 Oct 2020 13:34:19 +0200 Subject: [PATCH 08/13] Add compliance matrix entry. --- spec-compliance-matrix.md | 1 + 1 file changed, 1 insertion(+) diff --git a/spec-compliance-matrix.md b/spec-compliance-matrix.md index e7fb73a0f50..11675c81d7a 100644 --- a/spec-compliance-matrix.md +++ b/spec-compliance-matrix.md @@ -63,6 +63,7 @@ status of the feature is not known. |RecordException with extra parameters | - | + | + | [-](https://github.com/open-telemetry/opentelemetry-python/issues/1102) | - | - | | + | - | + | |[Sampling](https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/trace/sdk.md#sampling)| |Allow samplers to modify tracestate | | | | | | | | | | | +|[Span IDs wrt. sampling generated is compliant](https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/trace/sdk.md#sdk-span-creation) | | | | | | | | | | | ## Baggage From 6f04106b3e646a2666ec6b754edd01077f3f0335 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Neum=C3=BCller?= Date: Thu, 22 Oct 2020 13:54:56 +0200 Subject: [PATCH 09/13] Fix compliance matrix. --- spec-compliance-matrix.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec-compliance-matrix.md b/spec-compliance-matrix.md index b524b2dd642..6974f812bf3 100644 --- a/spec-compliance-matrix.md +++ b/spec-compliance-matrix.md @@ -68,7 +68,7 @@ status of the feature is not known. |RecordException with extra parameters | - | + | + | [-](https://github.com/open-telemetry/opentelemetry-python/issues/1102) | - | - | | + | - | + | |[Sampling](https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/trace/sdk.md#sampling)| |Allow samplers to modify tracestate | | | | | | | | | | | -|[Span IDs wrt. sampling generated is compliant](https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/trace/sdk.md#sdk-span-creation) | | | | | | | | | | | +|[Span/Trace ID creation wrt. sampling is compliant](specification/trace/sdk.md#sdk-span-creation) | | | | | | | | | | | |Allow samplers to modify tracestate | | | | [-](https://github.com/open-telemetry/opentelemetry-python/issues/1220) | | | | | | | |ShouldSample gets full parent Context | | | | | | | | | | | From 34263a3529b5736261eca81edb885c70481ab98f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Neum=C3=BCller?= Date: Wed, 28 Oct 2020 10:29:46 +0100 Subject: [PATCH 10/13] Apply suggestions from code review --- specification/trace/sdk.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/specification/trace/sdk.md b/specification/trace/sdk.md index d9dc93ae47f..d30528465ee 100644 --- a/specification/trace/sdk.md +++ b/specification/trace/sdk.md @@ -65,11 +65,11 @@ The SDK defines the interface [`Sampler`](#sampler) as well as a set of When asked to create a Span, the SDK MUST act as if doing the following in order: -1. If there is a valid parent trace ID, use it. Otherwise generate a new one +1. If there is a valid parent trace ID, use it. Otherwise generate a new trace ID (note: this must be done before calling `ShouldSample`, because it expects a valid trace ID as input). 2. Query the `Sampler`'s [`ShouldSample`](#shouldsample) method. -3. If the decision is `DROP` and there is a valid parent span ID, use it. +3. If the decision is `DROP` and there is a valid parent span ID, reuse it as the new `Span`'s span ID. Otherwise (if the decision is not `DROP` or there was no valid parent span ID) a new span ID MUST be generated. 4. Create a span depending on the decision returned by `ShouldSample`: From 9933136463b9a872153caf912de27e50415051d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Neum=C3=BCller?= Date: Wed, 28 Oct 2020 10:40:18 +0100 Subject: [PATCH 11/13] Fix dead internal link. --- specification/trace/sdk.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/specification/trace/sdk.md b/specification/trace/sdk.md index d30528465ee..fef5c435600 100644 --- a/specification/trace/sdk.md +++ b/specification/trace/sdk.md @@ -78,7 +78,8 @@ When asked to create a Span, the SDK MUST act as if doing the following in order and the [table above](#recording-sampled-reaction-table) on whether to pass the `Span` to `SpanProcessor`s. A non-recording span MAY be implemented using the same mechanism as when a - `Span` is created with no API-implementation installed (i.e., a [Propagated Span](api.md#propagated-span-creation)). + `Span` is created with no API-implementation installed or as described in + [wrapping a SpanContext in a Span](api.md#wrapping-a-spancontext-in-a-span). ### Sampler From eb989b72d78bbc6b6a50fc35e1b379a0bebee19c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Neum=C3=BCller?= Date: Tue, 3 Nov 2020 22:29:36 +0100 Subject: [PATCH 12/13] Add forward-ref to ParentBased sampler. --- specification/trace/sdk.md | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/specification/trace/sdk.md b/specification/trace/sdk.md index 306df5c079b..83b5d63e5e0 100644 --- a/specification/trace/sdk.md +++ b/specification/trace/sdk.md @@ -68,7 +68,10 @@ When asked to create a Span, the SDK MUST act as if doing the following in order 1. If there is a valid parent trace ID, use it. Otherwise generate a new trace ID (note: this must be done before calling `ShouldSample`, because it expects a valid trace ID as input). -2. Query the `Sampler`'s [`ShouldSample`](#shouldsample) method. +2. Query the `Sampler`'s [`ShouldSample`](#shouldsample) method + (Note that the [built-in `ParentBasedSampler`](#parentbased) can be used to + use the sampling decison of the parent, + translating a set SampledFlag to RECORD and an unset one to DROP). 3. If the decision is `DROP` and there is a valid parent span ID, reuse it as the new `Span`'s span ID. Otherwise (if the decision is not `DROP` or there was no valid parent span ID) a new span ID MUST be generated. From 9c9d65257bbb1da9861a5b8cd7c8cd0010b1d5b8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Neum=C3=BCller?= Date: Tue, 3 Nov 2020 22:37:26 +0100 Subject: [PATCH 13/13] Typo --- specification/trace/sdk.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/specification/trace/sdk.md b/specification/trace/sdk.md index 83b5d63e5e0..830e422044c 100644 --- a/specification/trace/sdk.md +++ b/specification/trace/sdk.md @@ -70,7 +70,7 @@ When asked to create a Span, the SDK MUST act as if doing the following in order a valid trace ID as input). 2. Query the `Sampler`'s [`ShouldSample`](#shouldsample) method (Note that the [built-in `ParentBasedSampler`](#parentbased) can be used to - use the sampling decison of the parent, + use the sampling decision of the parent, translating a set SampledFlag to RECORD and an unset one to DROP). 3. If the decision is `DROP` and there is a valid parent span ID, reuse it as the new `Span`'s span ID. Otherwise (if the decision is not `DROP` or there was no valid parent span ID)