Skip to content

Commit

Permalink
Align sampling specs in SDK (#1034)
Browse files Browse the repository at this point in the history
  • Loading branch information
lzchen authored Aug 31, 2020
1 parent dd47cd4 commit c435600
Show file tree
Hide file tree
Showing 12 changed files with 261 additions and 165 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,7 @@ def _get_sampling_rate(span):
return (
span.sampler.rate
if ctx.trace_flags.sampled
and isinstance(span.sampler, sampling.ProbabilitySampler)
and isinstance(span.sampler, sampling.TraceIdRatioBased)
else None
)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -498,7 +498,7 @@ def test_sampling_rate(self):
is_remote=False,
trace_flags=trace_api.TraceFlags(trace_api.TraceFlags.SAMPLED),
)
sampler = sampling.ProbabilitySampler(0.5)
sampler = sampling.TraceIdRatioBased(0.5)

span = trace.Span(
name="sampled", context=context, parent=None, sampler=sampler
Expand Down
2 changes: 2 additions & 0 deletions opentelemetry-api/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
([#1023](https:/open-telemetry/opentelemetry-python/pull/1023))
- Change return value type of `correlationcontext.get_correlations` to immutable `MappingProxyType`
([#1024](https:/open-telemetry/opentelemetry-python/pull/1024))
- Change is_recording_events to is_recording
([#1034](https:/open-telemetry/opentelemetry-python/pull/1034))
- Remove lazy Event and Link API from Span interface
([#1045](https:/open-telemetry/opentelemetry-python/pull/1045))

Expand Down
8 changes: 4 additions & 4 deletions opentelemetry-api/src/opentelemetry/trace/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,7 @@ def start_span(
name: str,
parent: ParentSpan = CURRENT_SPAN,
kind: SpanKind = SpanKind.INTERNAL,
attributes: typing.Optional[types.Attributes] = None,
attributes: types.Attributes = None,
links: typing.Sequence[Link] = (),
start_time: typing.Optional[int] = None,
set_status_on_exception: bool = True,
Expand Down Expand Up @@ -281,7 +281,7 @@ def start_as_current_span(
name: str,
parent: ParentSpan = CURRENT_SPAN,
kind: SpanKind = SpanKind.INTERNAL,
attributes: typing.Optional[types.Attributes] = None,
attributes: types.Attributes = None,
links: typing.Sequence[Link] = (),
) -> typing.Iterator["Span"]:
"""Context manager for creating a new span and set it
Expand Down Expand Up @@ -357,7 +357,7 @@ def start_span(
name: str,
parent: ParentSpan = Tracer.CURRENT_SPAN,
kind: SpanKind = SpanKind.INTERNAL,
attributes: typing.Optional[types.Attributes] = None,
attributes: types.Attributes = None,
links: typing.Sequence[Link] = (),
start_time: typing.Optional[int] = None,
set_status_on_exception: bool = True,
Expand All @@ -371,7 +371,7 @@ def start_as_current_span(
name: str,
parent: ParentSpan = Tracer.CURRENT_SPAN,
kind: SpanKind = SpanKind.INTERNAL,
attributes: typing.Optional[types.Attributes] = None,
attributes: types.Attributes = None,
links: typing.Sequence[Link] = (),
) -> typing.Iterator["Span"]:
# pylint: disable=unused-argument,no-self-use
Expand Down
4 changes: 2 additions & 2 deletions opentelemetry-api/src/opentelemetry/trace/span.py
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ def update_name(self, name: str) -> None:
"""

@abc.abstractmethod
def is_recording_events(self) -> bool:
def is_recording(self) -> bool:
"""Returns whether this span will be recorded.
Returns true if this Span is active and recording information like
Expand Down Expand Up @@ -203,7 +203,7 @@ def __init__(self, context: "SpanContext") -> None:
def get_context(self) -> "SpanContext":
return self._context

def is_recording_events(self) -> bool:
def is_recording(self) -> bool:
return False

def end(self, end_time: typing.Optional[int] = None) -> None:
Expand Down
6 changes: 3 additions & 3 deletions opentelemetry-api/tests/test_implementation.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,13 +39,13 @@ def test_default_tracer(self):
with tracer.start_span("test") as span:
self.assertEqual(span.get_context(), trace.INVALID_SPAN_CONTEXT)
self.assertEqual(span, trace.INVALID_SPAN)
self.assertIs(span.is_recording_events(), False)
self.assertIs(span.is_recording(), False)
with tracer.start_span("test2") as span2:
self.assertEqual(
span2.get_context(), trace.INVALID_SPAN_CONTEXT
)
self.assertEqual(span2, trace.INVALID_SPAN)
self.assertIs(span2.is_recording_events(), False)
self.assertIs(span2.is_recording(), False)

def test_span(self):
with self.assertRaises(TypeError):
Expand All @@ -55,7 +55,7 @@ def test_span(self):
def test_default_span(self):
span = trace.DefaultSpan(trace.INVALID_SPAN_CONTEXT)
self.assertEqual(span.get_context(), trace.INVALID_SPAN_CONTEXT)
self.assertIs(span.is_recording_events(), False)
self.assertIs(span.is_recording(), False)

# METER

Expand Down
2 changes: 2 additions & 0 deletions opentelemetry-sdk/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

- Moved samplers from API to SDK
([#1023](https:/open-telemetry/opentelemetry-python/pull/1023))
- Sampling spec changes
([#1034](https:/open-telemetry/opentelemetry-python/pull/1034))
- Remove lazy Event and Link API from Span interface
([#1045](https:/open-telemetry/opentelemetry-python/pull/1045))

Expand Down
55 changes: 24 additions & 31 deletions opentelemetry-sdk/src/opentelemetry/sdk/trace/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -515,7 +515,7 @@ def get_context(self):

def set_attribute(self, key: str, value: types.AttributeValue) -> None:
with self._lock:
if not self.is_recording_events():
if not self.is_recording():
return
has_ended = self.end_time is not None
if has_ended:
Expand All @@ -541,7 +541,7 @@ def set_attribute(self, key: str, value: types.AttributeValue) -> None:

def _add_event(self, event: EventBase) -> None:
with self._lock:
if not self.is_recording_events():
if not self.is_recording():
return
has_ended = self.end_time is not None

Expand Down Expand Up @@ -569,7 +569,7 @@ def add_event(

def start(self, start_time: Optional[int] = None) -> None:
with self._lock:
if not self.is_recording_events():
if not self.is_recording():
return
has_started = self.start_time is not None
if not has_started:
Expand All @@ -583,7 +583,7 @@ def start(self, start_time: Optional[int] = None) -> None:

def end(self, end_time: Optional[int] = None) -> None:
with self._lock:
if not self.is_recording_events():
if not self.is_recording():
return
if self.start_time is None:
raise RuntimeError("Calling end() on a not started span.")
Expand All @@ -610,7 +610,7 @@ def update_name(self, name: str) -> None:
return
self.name = name

def is_recording_events(self) -> bool:
def is_recording(self) -> bool:
return True

def set_status(self, status: trace_api.Status) -> None:
Expand Down Expand Up @@ -703,7 +703,7 @@ def start_as_current_span(
name: str,
parent: trace_api.ParentSpan = trace_api.Tracer.CURRENT_SPAN,
kind: trace_api.SpanKind = trace_api.SpanKind.INTERNAL,
attributes: Optional[types.Attributes] = None,
attributes: types.Attributes = None,
links: Sequence[trace_api.Link] = (),
) -> Iterator[trace_api.Span]:
span = self.start_span(name, parent, kind, attributes, links)
Expand All @@ -714,7 +714,7 @@ def start_span( # pylint: disable=too-many-locals
name: str,
parent: trace_api.ParentSpan = trace_api.Tracer.CURRENT_SPAN,
kind: trace_api.SpanKind = trace_api.SpanKind.INTERNAL,
attributes: Optional[types.Attributes] = None,
attributes: types.Attributes = None,
links: Sequence[trace_api.Link] = (),
start_time: Optional[int] = None,
set_status_on_exception: bool = True,
Expand All @@ -741,6 +741,20 @@ def start_span( # pylint: disable=too-many-locals
trace_flags = parent_context.trace_flags
trace_state = parent_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.
sampling_result = self.source.sampler.should_sample(
parent_context, trace_id, name, attributes, links,
)

trace_flags = (
trace_api.TraceFlags(trace_api.TraceFlags.SAMPLED)
if sampling_result.decision.is_sampled()
else trace_api.TraceFlags(trace_api.TraceFlags.DEFAULT)
)
context = trace_api.SpanContext(
trace_id,
generate_span_id(),
Expand All @@ -749,37 +763,16 @@ def start_span( # pylint: disable=too-many-locals
trace_state=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 decision.
sampling_decision = self.source.sampler.should_sample(
parent_context,
context.trace_id,
context.span_id,
name,
attributes,
links,
)

if sampling_decision.sampled:
options = context.trace_flags | trace_api.TraceFlags.SAMPLED
context.trace_flags = trace_api.TraceFlags(options)
if attributes is None:
span_attributes = sampling_decision.attributes
else:
# apply sampling decision attributes after initial attributes
span_attributes = attributes.copy()
span_attributes.update(sampling_decision.attributes)
# Only record if is_recording() is true
if sampling_result.decision.is_recording():
# pylint:disable=protected-access
span = Span(
name=name,
context=context,
parent=parent_context,
sampler=self.source.sampler,
resource=self.source.resource,
attributes=span_attributes,
attributes=sampling_result.attributes,
span_processor=self.source._active_span_processor,
kind=kind,
links=links,
Expand Down
Loading

0 comments on commit c435600

Please sign in to comment.