ref: Allow to start and finish StreamedSpans (9)#5598
ref: Allow to start and finish StreamedSpans (9)#5598sentrivana wants to merge 12 commits intoivana/span-first-8-bucket-based-limits-in-batcherfrom
StreamedSpans (9)#5598Conversation
Semver Impact of This PR🟢 Patch (bug fixes) 📋 Changelog PreviewThis is how your changes will appear in the changelog. This PR will not appear in the changelog. 🤖 This preview updates automatically when you update the PR. |
Codecov Results 📊✅ 14 passed | Total: 14 | Pass Rate: 100% | Execution Time: 3.70s 📊 Comparison with Base Branch
✨ No test changes detected All tests are passing successfully. ❌ Patch coverage is 23.60%. Project has 15139 uncovered lines. Files with missing lines (2)
Coverage diff@@ Coverage Diff @@
## main #PR +/-##
==========================================
- Coverage 25.17% 24.68% -0.49%
==========================================
Files 189 189 —
Lines 19924 20100 +176
Branches 6458 6500 +42
==========================================
+ Hits 5015 4961 -54
- Misses 14909 15139 +230
- Partials 387 381 -6Generated by Codecov Action |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
… ivana/span-first-9-start-end
… ivana/span-first-9-start-end
| if item._timestamp: | ||
| res["end_timestamp"] = item._timestamp.timestamp() |
There was a problem hiding this comment.
There is no scenario where a span that's already in the span batcher wouldn't have an end _timestamp, but mypy doesn't know that. 🤡
| try: | ||
| # profiling depends on this value and requires that | ||
| # it is measured in nanoseconds | ||
| self._start_timestamp_monotonic_ns = nanosecond_time() |
There was a problem hiding this comment.
Anything related to _start_timestamp_monotonic_ns is just copy-pasted from the Span implementation
| """ | ||
| self._end(end_timestamp) | ||
|
|
||
| def finish(self, end_timestamp: "Optional[Union[float, datetime]]" = None) -> None: |
There was a problem hiding this comment.
Just here for compat reasons, to make the transition from the old API easier (very slightly).
| self.end(end_timestamp) | ||
|
|
||
| def _start(self) -> None: | ||
| if self._active: |
There was a problem hiding this comment.
_active basically controls whether a span will be set on the scope, so if it's False, we don't need to keep track of the old parent span to restore it later, because we're never replacing it.
| if self._scope is None: | ||
| return | ||
|
|
||
| old_span = self._scope.span | ||
| self._scope.span = self | ||
| self._previous_span_on_scope = old_span |
There was a problem hiding this comment.
Unlike the main span class, no-op spans don't have an active flag. The user cannot control whether a no-op span will be set on the scope; the SDK does this internally, and it uses the _scope attribute to remember the decision. In the case of normal spans, the _scope attribute plays a different role: they all need a scope, regardless of whether they'll actually be set on it or not, because they will be captured using that scope in either case.
Coming back to no-op spans, the decision whether to set them on the scope or not is tightly coupled to how we want ignore_spans to work (coming in a future PR). Basically, if a segment/root span would be ignored, all of its children should be as well. That's easiest to accomplish if we actually set the root no-op span on the scope. On the other hand, if a non-root span is ignored, it's not set on scope, so that any children spans effectively use the last not ignored span that's set on the scope as parent.
1 participant
Description
This PR adds logic for starting and finishing spans, both via a context manager
with start_span(...), as well as directly withspan = start_span(...)andspan.end().Summary
_start()and_end()functions that handle the starting and ending logic._start()handles keeping track of what was calledcontext_manager_stateon old Spans, i.e., remembering which span was set on the scope before this span so that it can be restored once this span is over._end()handles restoring the old span, setting the end timestamp, adding final attributes on it, and queueing it in the span batcher.__enter__and__exit__methods on bothStreamedSpanandNoOpStreamedSpan. These call the internal methods above, with__exit__additionally setting the span status to error if it encountered one.span.end()public function that allows to set an optionalend_timestamp. If provided, it sets it and calls_end()internally.span.finish()too, which is deprecated from the get go, just to make the migration to span first a bit easier (since the method is calledfinish()on legacy spans).Issues
Reminders
tox -e linters.feat:,fix:,ref:,meta:)