Change should_sample parameters to be spec compliant (#1764)

Author: lzchen

CHANGELOG.md

@@ -10,6 +10,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Added `py.typed` file to every package. This should resolve a bunch of mypy
   errors for users.
   ([#1720](https://github.com/open-telemetry/opentelemetry-python/pull/1720))
+- Added `SpanKind` to `should_sample` parameters, suggest using parent span context's tracestate
+  instead of manually passed in tracestate in `should_sample`
+  ([#1764](https://github.com/open-telemetry/opentelemetry-python/pull/1764))
 
 ### Changed
 - Adjust `B3Format` propagator to be spec compliant by not modifying context

opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py

@@ -905,20 +905,17 @@ def start_span(  # pylint: disable=too-many-locals
         if parent_span_context is None or not parent_span_context.is_valid:
             parent_span_context = None
             trace_id = self.id_generator.generate_trace_id()
-            trace_flags = None
-            trace_state = None
         else:
             trace_id = parent_span_context.trace_id
-            trace_flags = parent_span_context.trace_flags
-            trace_state = parent_span_context.trace_state
 
         # The sampler decides whether to create a real or no-op span at the
         # time of span creation. No-op spans do not record events, and are not
         # exported.
         # The sampler may also add attributes to the newly-created span, e.g.
         # to include information about the sampling result.
+        # The sampler may also modify the parent span context's tracestate
         sampling_result = self.sampler.should_sample(
-            context, trace_id, name, attributes, links, trace_state
+            context, trace_id, name, kind, attributes, links
         )
 
         trace_flags = (

opentelemetry-sdk/src/opentelemetry/sdk/trace/sampling.py

@@ -33,6 +33,9 @@
 
 Custom samplers can be created by subclassing `Sampler` and implementing `Sampler.should_sample` as well as `Sampler.get_description`.
 
+Samplers are able to modify the `opentelemetry.trace.span.TraceState` of the parent of the span being created. For custom samplers, it is suggested to implement `Sampler.should_sample` to utilize the
+parent span context's `opentelemetry.trace.span.TraceState` and pass into the `SamplingResult` instead of the explicit trace_state field passed into the parameter of `Sampler.should_sample`.
+
 To use a sampler, pass it into the tracer provider constructor. For example:
 
 .. code:: python
@@ -108,7 +111,7 @@
     OTEL_TRACES_SAMPLER,
     OTEL_TRACES_SAMPLER_ARG,
 )
-from opentelemetry.trace import Link, get_current_span
+from opentelemetry.trace import Link, SpanKind, get_current_span
 from opentelemetry.trace.span import TraceState
 from opentelemetry.util.types import Attributes
 
@@ -167,6 +170,7 @@ def should_sample(
         parent_context: Optional["Context"],
         trace_id: int,
         name: str,
+        kind: SpanKind = None,
         attributes: Attributes = None,
         links: Sequence["Link"] = None,
         trace_state: "TraceState" = None,
@@ -189,13 +193,18 @@ def should_sample(
         parent_context: Optional["Context"],
         trace_id: int,
         name: str,
+        kind: SpanKind = None,
         attributes: Attributes = None,
         links: Sequence["Link"] = None,
         trace_state: "TraceState" = None,
     ) -> "SamplingResult":
         if self._decision is Decision.DROP:
             attributes = None
-        return SamplingResult(self._decision, attributes, trace_state)
+        return SamplingResult(
+            self._decision,
+            attributes,
+            _get_parent_trace_state(parent_context),
+        )
 
     def get_description(self) -> str:
         if self._decision is Decision.DROP:
@@ -246,6 +255,7 @@ def should_sample(
         parent_context: Optional["Context"],
         trace_id: int,
         name: str,
+        kind: SpanKind = None,
         attributes: Attributes = None,
         links: Sequence["Link"] = None,
         trace_state: "TraceState" = None,
@@ -255,7 +265,9 @@ def should_sample(
             decision = Decision.RECORD_AND_SAMPLE
         if decision is Decision.DROP:
             attributes = None
-        return SamplingResult(decision, attributes, trace_state)
+        return SamplingResult(
+            decision, attributes, _get_parent_trace_state(parent_context),
+        )
 
     def get_description(self) -> str:
         return "TraceIdRatioBased{{{}}}".format(self._rate)
@@ -296,6 +308,7 @@ def should_sample(
         parent_context: Optional["Context"],
         trace_id: int,
         name: str,
+        kind: SpanKind = None,
         attributes: Attributes = None,
         links: Sequence["Link"] = None,
         trace_state: "TraceState" = None,
@@ -322,9 +335,9 @@ def should_sample(
             parent_context=parent_context,
             trace_id=trace_id,
             name=name,
+            kind=kind,
             attributes=attributes,
             links=links,
-            trace_state=trace_state,
         )
 
     def get_description(self):
@@ -385,3 +398,10 @@ def _get_from_env_or_default() -> Sampler:
         return _KNOWN_SAMPLERS[trace_sampler](rate)
 
     return _KNOWN_SAMPLERS[trace_sampler]
+
+
+def _get_parent_trace_state(parent_context) -> Optional["TraceState"]:
+    parent_span_context = get_current_span(parent_context).get_span_context()
+    if parent_span_context is None or not parent_span_context.is_valid:
+        return None
+    return parent_span_context.trace_state