Skip to content

Commit d0e5fb1

Browse files
authored
fix(tracing): add span processors shutdown (backport #3508) (#3516)
This is an automatic backport of pull request #3508 done by [Mergify](https://mergify.com). Cherry-pick of e9b81b9 has failed: ``` On branch mergify/bp/1.0/pr-3508 Your branch is up to date with 'origin/1.0'. You are currently cherry-picking commit e9b81b9. (fix conflicts and run "git cherry-pick --continue") (use "git cherry-pick --skip" to skip this patch) (use "git cherry-pick --abort" to cancel the cherry-pick operation) Changes to be committed: modified: ddtrace/tracer.py modified: tests/tracer/test_tracer.py Unmerged paths: (use "git add <file>..." to mark resolution) both modified: ddtrace/internal/processor/__init__.py both modified: ddtrace/internal/processor/trace.py ``` To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/github/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally --- <details> <summary>Mergify commands and options</summary> <br /> More conditions and actions can be found in the [documentation](https://docs.mergify.com/). You can also trigger Mergify actions by commenting on this pull request: - `@Mergifyio refresh` will re-evaluate the rules - `@Mergifyio rebase` will rebase this PR on its base branch - `@Mergifyio update` will merge the base branch into this PR - `@Mergifyio backport <destination>` will backport this PR on `<destination>` branch Additionally, on Mergify [dashboard](https://dashboard.mergify.com/) you can: - look at your merge queues - generate the Mergify configuration with the config editor. Finally, you can contact us on https://mergify.com </details>
1 parent b5828e4 commit d0e5fb1

File tree

4 files changed

+56
-21
lines changed

4 files changed

+56
-21
lines changed

ddtrace/internal/processor/__init__.py

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import abc
2+
from typing import Optional
23

34
import attr
45
import six
@@ -51,3 +52,11 @@ def on_span_finish(self, span):
5152
applied afterwards.
5253
"""
5354
pass
55+
56+
def shutdown(self, timeout):
57+
# type: (Optional[float]) -> None
58+
"""Called when the processor is done being used.
59+
60+
Any clean-up or flushing should be performed with this method.
61+
"""
62+
pass

ddtrace/internal/processor/trace.py

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111

1212
from ddtrace.internal.logger import get_logger
1313
from ddtrace.internal.processor import SpanProcessor
14+
from ddtrace.internal.service import ServiceStatusError
1415
from ddtrace.internal.writer import TraceWriter
1516
from ddtrace.span import Span
1617

@@ -201,3 +202,19 @@ def on_span_finish(self, span):
201202

202203
log.debug("trace %d has %d spans, %d finished", span.trace_id, len(trace.spans), trace.num_finished)
203204
return None
205+
206+
def shutdown(self, timeout):
207+
# type: (Optional[float]) -> None
208+
"""
209+
This will stop the background writer/worker and flush any finished traces in the buffer. The tracer cannot be
210+
used for tracing after this method has been called. A new tracer instance is required to continue tracing.
211+
212+
:param timeout: How long in seconds to wait for the background worker to flush traces
213+
before exiting or :obj:`None` to block until flushing has successfully completed (default: :obj:`None`)
214+
:type timeout: :obj:`int` | :obj:`float` | :obj:`None`
215+
"""
216+
try:
217+
self._writer.stop(timeout)
218+
except ServiceStatusError:
219+
# It's possible the writer never got started in the first place :(
220+
pass

ddtrace/tracer.py

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -861,22 +861,20 @@ def set_tags(self, tags):
861861

862862
def shutdown(self, timeout=None):
863863
# type: (Optional[float]) -> None
864-
"""Shutdown the tracer.
865-
866-
This will stop the background writer/worker and flush any finished traces in the buffer. The tracer cannot be
867-
used for tracing after this method has been called. A new tracer instance is required to continue tracing.
864+
"""Shutdown the tracer and flush finished traces. Avoid calling shutdown multiple times.
868865
869866
:param timeout: How long in seconds to wait for the background worker to flush traces
870867
before exiting or :obj:`None` to block until flushing has successfully completed (default: :obj:`None`)
871868
:type timeout: :obj:`int` | :obj:`float` | :obj:`None`
872869
"""
873-
try:
874-
self._writer.stop(timeout=timeout)
875-
except service.ServiceStatusError:
876-
# It's possible the writer never got started in the first place :(
877-
pass
878-
879870
with self._shutdown_lock:
871+
# Thread safety: Ensures tracer is shutdown synchronously
872+
span_processors = self._span_processors
873+
self._span_processors = []
874+
for processor in span_processors:
875+
if hasattr(processor, "shutdown"):
876+
processor.shutdown(timeout)
877+
880878
atexit.unregister(self._atexit)
881879
forksafe.unregister(self._child_after_fork)
882880

tests/tracer/test_tracer.py

Lines changed: 22 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -517,13 +517,13 @@ def test_tracer_url():
517517

518518
def test_tracer_shutdown_no_timeout():
519519
t = ddtrace.Tracer()
520-
writer = mock.Mock(wraps=t._writer)
521-
t._writer = writer
522520

523-
# The writer thread does not start until the first write.
524-
t.shutdown()
525-
assert t._writer.stop.called
526-
assert not t._writer.join.called
521+
with mock.patch.object(AgentWriter, "stop") as mock_stop:
522+
with mock.patch.object(AgentWriter, "join") as mock_join:
523+
t.shutdown()
524+
525+
mock_stop.assert_called()
526+
mock_join.assert_not_called()
527527

528528

529529
def test_tracer_configure_writer_stop_unstarted():
@@ -551,13 +551,24 @@ def test_tracer_configure_writer_stop_started():
551551

552552
def test_tracer_shutdown_timeout():
553553
t = ddtrace.Tracer()
554-
t._writer = mock.Mock(wraps=t._writer)
555554

556-
with t.trace("something"):
557-
pass
555+
with mock.patch.object(AgentWriter, "stop") as mock_stop:
556+
with t.trace("something"):
557+
pass
558+
559+
t.shutdown(timeout=2)
560+
mock_stop.assert_called_once_with(2)
561+
562+
563+
def test_tracer_shutdown():
564+
t = ddtrace.Tracer()
565+
t.shutdown()
566+
567+
with mock.patch.object(AgentWriter, "write") as mock_write:
568+
with t.trace("something"):
569+
pass
558570

559-
t.shutdown(timeout=2)
560-
t._writer.stop.assert_called_once_with(timeout=2)
571+
mock_write.assert_not_called()
561572

562573

563574
def test_tracer_dogstatsd_url():

0 commit comments

Comments
 (0)