Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(tracing): do not raise exception if partial flush is triggered without any spans #9349

Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
61 commits
Select commit Hold shift + click to select a range
c1f25f2
chore(internal): log a warning if partial flushes has zero spans to send
romainkomorndatadog May 22, 2024
005a279
add test
romainkomorndatadog May 22, 2024
76271f6
relnote
romainkomorndatadog May 22, 2024
90d5346
force test to run with partial spans
romainkomorndatadog May 22, 2024
8bd0aaf
comment to force test failure
romainkomorndatadog May 22, 2024
e38af19
Update releasenotes/notes/fix-tracing-dont_raise_exception_on_empty_p…
romainkomorndatadog May 23, 2024
4c430ca
stash
romainkomorndatadog May 23, 2024
f2a9bc5
fix test
romainkomorndatadog May 23, 2024
c2d6fe0
Merge branch 'romain.komorn/AIT-10242/dont_crash_when_spans_list_empt…
romainkomorndatadog May 23, 2024
fbf212e
better log message
romainkomorndatadog May 23, 2024
ebb5ec0
test for missing trace, or missing span
romainkomorndatadog May 23, 2024
715ec1e
remove comment
romainkomorndatadog May 23, 2024
be16fb6
tiny tweaks
romainkomorndatadog May 23, 2024
3eddcce
move test and make ruff lint a little happier
romainkomorndatadog May 23, 2024
abc9e55
Merge branch 'main' into romain.komorn/AIT-10242/dont_crash_when_span…
romainkomorndatadog May 23, 2024
986e662
switch to reconfiguring span processor
romainkomorndatadog May 24, 2024
69a3e5a
update test
romainkomorndatadog May 24, 2024
5a5de55
Merge branch 'romain.komorn/AIT-10242/dont_crash_when_spans_list_empt…
romainkomorndatadog May 24, 2024
c992644
Revert "update test"
romainkomorndatadog May 24, 2024
13df4a9
Revert "switch to reconfiguring span processor"
romainkomorndatadog May 24, 2024
d2303af
revert back to just logging warnings
romainkomorndatadog May 24, 2024
f01664b
send one-span trace when a trace can't be found
romainkomorndatadog May 24, 2024
1554f4a
Merge branch 'main' into romain.komorn/AIT-10242/dont_crash_when_span…
romainkomorndatadog May 24, 2024
ce231a4
Apply suggestions from code review
brettlangdon May 24, 2024
6ec0335
update logic and test cases
brettlangdon May 24, 2024
a7c4a6b
Update tests/tracer/test_processors.py
brettlangdon May 24, 2024
94d3cd3
add additional partial flushing scenario
brettlangdon May 24, 2024
0871185
fix tests
brettlangdon May 24, 2024
677409c
Merge branch 'main' into romain.komorn/AIT-10242/dont_crash_when_span…
brettlangdon May 24, 2024
13f14b6
Merge branch 'main' into romain.komorn/AIT-10242/dont_crash_when_span…
brettlangdon May 24, 2024
b4e53b2
Apply suggestions from code review
brettlangdon May 28, 2024
93468d9
Apply suggestions from code review
brettlangdon May 29, 2024
afd47b6
Merge branch 'main' into romain.komorn/AIT-10242/dont_crash_when_span…
brettlangdon May 29, 2024
3fd0f11
Ensure that lock is held, add tests
romainkomorndatadog Jun 11, 2024
0c021b9
Merge branch 'refs/heads/main' into romain.komorn/AIT-10242/dont_cras…
romainkomorndatadog Jun 11, 2024
94efd3b
Merge branch 'main' into romain.komorn/AIT-10242/dont_crash_when_span…
romainkomorndatadog Jun 11, 2024
ca31190
add utility method
romainkomorndatadog Jun 11, 2024
9021780
Merge branch 'romain.komorn/AIT-10242/dont_crash_when_spans_list_empt…
romainkomorndatadog Jun 11, 2024
4de8531
Merge branch 'main' into romain.komorn/AIT-10242/dont_crash_when_span…
brettlangdon Jun 11, 2024
3c6c7d5
Merge branch 'main' into romain.komorn/AIT-10242/dont_crash_when_span…
romainkomorndatadog Jun 11, 2024
ae7bd63
Merge branch 'main' into romain.komorn/AIT-10242/dont_crash_when_span…
romainkomorndatadog Jun 11, 2024
8bce760
Merge branch 'main' into romain.komorn/AIT-10242/dont_crash_when_span…
romainkomorndatadog Jun 12, 2024
c909866
Merge branch 'main' into romain.komorn/AIT-10242/dont_crash_when_span…
romainkomorndatadog Jun 12, 2024
48051b0
Update ddtrace/_trace/processor/__init__.py
romainkomorndatadog Jun 12, 2024
43224b8
Update ddtrace/_trace/processor/__init__.py
romainkomorndatadog Jun 12, 2024
036db1f
Merge branch 'main' into romain.komorn/AIT-10242/dont_crash_when_span…
emmettbutler Jun 12, 2024
4328b10
Merge branch 'main' into romain.komorn/AIT-10242/dont_crash_when_span…
emmettbutler Jun 12, 2024
dbdde04
Merge branch 'main' into romain.komorn/AIT-10242/dont_crash_when_span…
emmettbutler Jun 13, 2024
9f6ca75
Do not finish spans in os.fork child process
romainkomorndatadog Jun 14, 2024
24ba591
refactor test
romainkomorndatadog Jun 17, 2024
535d8c6
Merge branch 'main' into romain.komorn/AIT-10242/dont_crash_when_span…
romainkomorndatadog Jun 17, 2024
4f73a8a
comment out originally commented line
romainkomorndatadog Jun 17, 2024
0467e22
Merge branch 'romain.komorn/AIT-10242/dont_crash_when_spans_list_empt…
romainkomorndatadog Jun 17, 2024
d7434ef
revert os.fork instrumentation changes
romainkomorndatadog Jun 17, 2024
14c1cc4
revert changes to re-add trace on new traces found
romainkomorndatadog Jun 17, 2024
07649c9
return test to initial PR plan
romainkomorndatadog Jun 17, 2024
a5bb63b
remove duplicate test
romainkomorndatadog Jun 17, 2024
a86c5ad
Merge branch 'main' into romain.komorn/AIT-10242/dont_crash_when_span…
romainkomorndatadog Jun 17, 2024
5120910
switch logging levels to debug, telemetry to 'ERROR'
romainkomorndatadog Jun 17, 2024
1193055
Merge branch 'main' into romain.komorn/AIT-10242/dont_crash_when_span…
romainkomorndatadog Jun 17, 2024
a49062d
Merge branch 'main' into romain.komorn/AIT-10242/dont_crash_when_span…
romainkomorndatadog Jun 18, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 8 additions & 8 deletions ddtrace/_trace/processor/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -324,14 +324,14 @@ def on_span_finish(self, span):

if should_partial_flush:
log.debug("Partially flushing %d spans for trace %d", num_finished, span.trace_id)
if not finished:
log_msg = "Partial flush triggered but no spans to flush (was tracer reconfigured?)"
# Avoid potential crashes if, for some reason, we have no spans to flush even though
# should_partial_flush is True
if config._telemetry_enabled:
telemetry.telemetry_writer.add_log("WARNING", log_msg)
log.warning(log_msg)
return
# if not finished:
# log_msg = "Partial flush triggered but no spans to flush (was tracer reconfigured?)"
# # Avoid potential crashes if, for some reason, we have no spans to flush even though
# # should_partial_flush is True
# if config._telemetry_enabled:
# telemetry.telemetry_writer.add_log("WARNING", log_msg)
# log.warning(log_msg)
# return
finished[0].set_metric("_dd.py.partial_flush", num_finished)

trace.num_finished -= num_finished
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
---
#instructions:
# The style guide below provides explanations, instructions, and templates to write your own release note.
# Once finished, all irrelevant sections (including this instruction section) should be removed,
# and the release note should be committed with the rest of the changes.
#
# The main goal of a release note is to provide a brief overview of a change and provide actionable steps to the user.
# The release note should clearly communicate what the change is, why the change was made, and how a user can migrate their code.
#
# The release note should also clearly distinguish between announcements and user instructions. Use:
# * Past tense for previous/existing behavior (ex: ``resulted, caused, failed``)
# * Third person present tense for the change itself (ex: ``adds, fixes, upgrades``)
# * Active present infinitive for user instructions (ex: ``set, use, add``)
#
# Release notes should:
# * Use plain language
# * Be concise
# * Include actionable steps with the necessary code changes
# * Include relevant links (bug issues, upstream issues or release notes, documentation pages)
# * Use full sentences with sentence-casing and punctuation.
# * Before using Datadog specific acronyms/terminology, a release note must first introduce them with a definition.
#
# Release notes should not:
# * Be vague. Example: ``fixes an issue in tracing``.
# * Use overly technical language
# * Use dynamic links (``stable/latest/1.x`` URLs). Instead, use static links (specific version, commit hash) whenever possible so that they don't break in the future.
romainkomorndatadog marked this conversation as resolved.
Show resolved Hide resolved
fixes:
- |
tracing: fixes a potential crash where using partial flushes and tracer.configure() could result in an IndexError
romainkomorndatadog marked this conversation as resolved.
Show resolved Hide resolved
20 changes: 20 additions & 0 deletions tests/internal/test_settings.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import json
import os
import textwrap

import mock
import pytest
Expand Down Expand Up @@ -605,3 +606,22 @@ def test_remoteconfig_header_tags(run_python_code_in_subprocess):
env=env,
)
assert status == 0, f"err={err.decode('utf-8')} out={out.decode('utf-8')}"


def test_tracer_reconfigure_does_not_crash_tracer(run_python_code_in_subprocess):
env = os.environ.copy()
env.update({"DD_TRACER_PARTIAL_FLUSH_ENABLED": "true", "DD_TRACER_PARTIAL_FLUSH_MIN_SPANS": "1"})
romainkomorndatadog marked this conversation as resolved.
Show resolved Hide resolved

out, err, status, _ = run_python_code_in_subprocess(
textwrap.dedent(
"""
import ddtrace

with ddtrace.tracer.trace("regression"):
ddtrace.tracer.configure(partial_flush_min_spans=1)
"""
),
env=env,
)

assert status == 0, f"err={err.decode('utf-8')} out={out.decode('utf-8')}"
Loading