Skip to content

Commit

Permalink
Fix ParentBased sampler for implicit parent spans (#1394)
Browse files Browse the repository at this point in the history
consider the sample decision of implicit parent spans (when creating
a span without explicitly providing a context) instead of forwarding
to the delegating sampler.

* fix trace_state erasure for dropped spans

* samplers did not return the trace_state in the sampling result
  when a span was dropped. This caused the trace_state extracted
  from a remote parent span to be erased and further context
  propagation to break.
* fix also TraceIdRatioBased which erased the trace_state independent
  of the sampling outcome.
  • Loading branch information
mariojonke authored Nov 25, 2020
1 parent 2425d66 commit 7b33dd6
Show file tree
Hide file tree
Showing 3 changed files with 231 additions and 187 deletions.
5 changes: 3 additions & 2 deletions opentelemetry-sdk/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,9 @@
([#1373](https://github.com/open-telemetry/opentelemetry-python/pull/1373))
- Rename Meter class to Accumulator in Metrics SDK
([#1372](https://github.com/open-telemetry/opentelemetry-python/pull/1372))
- Rename Meter class to Accumulator in Metrics SDK
([#1372](https://github.com/open-telemetry/opentelemetry-python/pull/1372))
- Fix `ParentBased` sampler for implicit parent spans. Fix also `trace_state`
erasure for dropped spans or spans sampled by the `TraceIdRatioBased` sampler.
([#1394](https://github.com/open-telemetry/opentelemetry-python/pull/1394))

## Version 0.15b0

Expand Down
28 changes: 13 additions & 15 deletions opentelemetry-sdk/src/opentelemetry/sdk/trace/sampling.py
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ def should_sample(
trace_state: "TraceState" = None,
) -> "SamplingResult":
if self._decision is Decision.DROP:
return SamplingResult(self._decision)
attributes = None
return SamplingResult(self._decision, attributes, trace_state)

def get_description(self) -> str:
Expand Down Expand Up @@ -209,8 +209,8 @@ def should_sample(
if trace_id & self.TRACE_ID_LIMIT < self.bound:
decision = Decision.RECORD_AND_SAMPLE
if decision is Decision.DROP:
return SamplingResult(decision)
return SamplingResult(decision, attributes)
attributes = None
return SamplingResult(decision, attributes, trace_state)

def get_description(self) -> str:
return "TraceIdRatioBased{{{}}}".format(self._rate)
Expand Down Expand Up @@ -238,18 +238,16 @@ def should_sample(
links: Sequence["Link"] = None,
trace_state: "TraceState" = None,
) -> "SamplingResult":
if parent_context is not None:
parent_span_context = get_current_span(
parent_context
).get_span_context()
# only drop if parent exists and is not a root span
if (
parent_span_context is not None
and parent_span_context.is_valid
and not parent_span_context.trace_flags.sampled
):
return SamplingResult(Decision.DROP)
return SamplingResult(Decision.RECORD_AND_SAMPLE, attributes)
parent_span_context = get_current_span(
parent_context
).get_span_context()
# respect the sampling flag of the parent if present
if parent_span_context is not None and parent_span_context.is_valid:
decision = Decision.RECORD_AND_SAMPLE
if not parent_span_context.trace_flags.sampled:
decision = Decision.DROP
attributes = None
return SamplingResult(decision, attributes, trace_state)

return self._delegate.should_sample(
parent_context=parent_context,
Expand Down
Loading

0 comments on commit 7b33dd6

Please sign in to comment.