chore(tracing): make traceback size limit configurable #7558

emmettbutler · 2023-11-10T19:53:55Z

This pull request fixes #4623 by making the size limit of tracebacks included in span tags configurable. It does not change the default limit.

Checklist

Change(s) are motivated and described in the PR description.
Testing strategy is described if automated tests are not included in the PR.
Risk is outlined (performance impact, potential for breakage, maintainability, etc).
Change is maintainable (easy to change, telemetry, documentation).
Library release note guidelines are followed. If no release note is required, add label changelog/no-changelog.
Documentation is included (in-code, generated user docs, public corp docs).
Backport labels are set (if applicable)

Reviewer Checklist

pr-commenter · 2023-11-10T20:42:12Z

Benchmarks

Benchmark execution time: 2023-11-13 17:29:04

Comparing candidate commit 76491c0 in PR branch emmett.butler/traceback-config with baseline commit 2348097 in branch 2.x.

Found 1 performance improvements and 5 performance regressions! Performance is the same for 84 metrics, 0 unstable metrics.

scenario:iastpropagation-no-propagation

🟥 max_rss_usage [+0.959MB; +1.070MB] or [+2.864%; +3.195%]

scenario:otelspan-start-finish

🟥 max_rss_usage [+661.606KB; +835.482KB] or [+2.245%; +2.835%]

scenario:sethttpmeta-no-useragentvariant

🟥 max_rss_usage [+576.381KB; +788.406KB] or [+2.006%; +2.744%]

scenario:sethttpmeta-obfuscation-regular-case-explicit-query

🟥 max_rss_usage [+611.880KB; +770.930KB] or [+2.119%; +2.669%]

scenario:sethttpmeta-obfuscation-regular-case-implicit-query

🟩 max_rss_usage [-919.466KB; -734.498KB] or [-3.098%; -2.475%]

scenario:sethttpmeta-useragentvariant_not_exists_2

🟥 max_rss_usage [+646.795KB; +806.466KB] or [+2.254%; +2.811%]

This pull request fixes #4623 by making the size limit of tracebacks included in span tags configurable. It does not change the default limit. ## Checklist - [x] Change(s) are motivated and described in the PR description. - [x] Testing strategy is described if automated tests are not included in the PR. - [x] Risk is outlined (performance impact, potential for breakage, maintainability, etc). - [x] Change is maintainable (easy to change, telemetry, documentation). - [x] [Library release note guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html) are followed. If no release note is required, add label `changelog/no-changelog`. - [x] Documentation is included (in-code, generated user docs, [public corp docs](https://github.com/DataDog/documentation/)). - [x] Backport labels are set (if [applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)) ## Reviewer Checklist - [x] Title is accurate. - [x] No unnecessary changes are introduced. - [x] Description motivates each change. - [x] Avoids breaking [API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces) changes unless absolutely necessary. - [x] Testing strategy adequately addresses listed risk(s). - [x] Change is maintainable (easy to change, telemetry, documentation). - [x] Release note makes sense to a user of the library. - [x] Reviewer has explicitly acknowledged and discussed the performance implications of this PR as reported in the benchmarks PR comment. - [x] Backport labels are set in a manner that is consistent with the [release branch maintenance policy](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting) - [x] If this PR touches code that signs or publishes builds or packages, or handles credentials of any kind, I've requested a review from `@DataDog/security-design-and-guidance`. - [x] This PR doesn't touch any of that.

DataDog#7558 made the traceback size configurable, but tracebacks for exceptions were still hardcoded with a limit of 30.

DataDog#7558 made the traceback size configurable via `DD_SPAN_TRACEBACK_MAX_SIZE`, but tracebacks for exceptions were still hardcoded with a limit of 30. The fix is modeled after `Span#set_traceback`'s existing logic, and tests were modeled after both `test_traceback_with_error` and `test_custom_traceback_size`. Not much new logic, just an edge case.

#7558 made the traceback size configurable via `DD_SPAN_TRACEBACK_MAX_SIZE`, but tracebacks for exceptions were still hardcoded with a limit of 30. The fix is modeled after `Span#set_traceback`'s existing logic, and tests were modeled after both `test_traceback_with_error` and `test_custom_traceback_size`. Not much new logic, just an edge case. ## Checklist - [x] Change(s) are motivated and described in the PR description - [x] Testing strategy is described if automated tests are not included in the PR - [x] Risks are described (performance impact, potential for breakage, maintainability) - [x] Change is maintainable (easy to change, telemetry, documentation) - [x] [Library release note guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html) are followed or label `changelog/no-changelog` is set - [x] Documentation is included (in-code, generated user docs, [public corp docs](https://github.com/DataDog/documentation/)) - [x] Backport labels are set (if [applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)) - [x] If this PR changes the public interface, I've notified `@DataDog/apm-tees`. ## Reviewer Checklist - [x] Title is accurate - [x] All changes are related to the pull request's stated goal - [x] Description motivates each change - [x] Avoids breaking [API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces) changes - [x] Testing strategy adequately addresses listed risks - [x] Change is maintainable (easy to change, telemetry, documentation) - [x] Release note makes sense to a user of the library - [x] Author has acknowledged and discussed the performance implications of this PR as reported in the benchmarks PR comment - [x] Backport labels are set in a manner that is consistent with the [release branch maintenance policy](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting) Co-authored-by: erikayasuda <153395705+erikayasuda@users.noreply.github.com>

#7558 made the traceback size configurable via `DD_SPAN_TRACEBACK_MAX_SIZE`, but tracebacks for exceptions were still hardcoded with a limit of 30. The fix is modeled after `Span#set_traceback`'s existing logic, and tests were modeled after both `test_traceback_with_error` and `test_custom_traceback_size`. Not much new logic, just an edge case. ## Checklist - [x] Change(s) are motivated and described in the PR description - [x] Testing strategy is described if automated tests are not included in the PR - [x] Risks are described (performance impact, potential for breakage, maintainability) - [x] Change is maintainable (easy to change, telemetry, documentation) - [x] [Library release note guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html) are followed or label `changelog/no-changelog` is set - [x] Documentation is included (in-code, generated user docs, [public corp docs](https://github.com/DataDog/documentation/)) - [x] Backport labels are set (if [applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)) - [x] If this PR changes the public interface, I've notified `@DataDog/apm-tees`. ## Reviewer Checklist - [x] Title is accurate - [x] All changes are related to the pull request's stated goal - [x] Description motivates each change - [x] Avoids breaking [API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces) changes - [x] Testing strategy adequately addresses listed risks - [x] Change is maintainable (easy to change, telemetry, documentation) - [x] Release note makes sense to a user of the library - [x] Author has acknowledged and discussed the performance implications of this PR as reported in the benchmarks PR comment - [x] Backport labels are set in a manner that is consistent with the [release branch maintenance policy](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting) Co-authored-by: erikayasuda <153395705+erikayasuda@users.noreply.github.com> (cherry picked from commit 1b954de)

…9389) Backport 1b954de from #9112 to 2.7. #7558 made the traceback size configurable via `DD_TRACE_SPAN_TRACEBACK_MAX_SIZE`, but tracebacks for exceptions were still hardcoded with a limit of 30. The fix is modeled after `Span#set_traceback`'s existing logic, and tests were modeled after both `test_traceback_with_error` and `test_custom_traceback_size`. Not much new logic, just an edge case. ## Checklist - [x] Change(s) are motivated and described in the PR description - [x] Testing strategy is described if automated tests are not included in the PR - [x] Risks are described (performance impact, potential for breakage, maintainability) - [x] Change is maintainable (easy to change, telemetry, documentation) - [x] [Library release note guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html) are followed or label `changelog/no-changelog` is set - [x] Documentation is included (in-code, generated user docs, [public corp docs](https://github.com/DataDog/documentation/)) - [x] Backport labels are set (if [applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)) - [x] If this PR changes the public interface, I've notified `@DataDog/apm-tees`. ## Reviewer Checklist - [x] Title is accurate - [x] All changes are related to the pull request's stated goal - [x] Description motivates each change - [x] Avoids breaking [API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces) changes - [x] Testing strategy adequately addresses listed risks - [x] Change is maintainable (easy to change, telemetry, documentation) - [x] Release note makes sense to a user of the library - [x] Author has acknowledged and discussed the performance implications of this PR as reported in the benchmarks PR comment - [x] Backport labels are set in a manner that is consistent with the [release branch maintenance policy](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting) Co-authored-by: Matt Alexander <1710128+quiqueg@users.noreply.github.com>

…9390) Backport 1b954de from #9112 to 2.8. #7558 made the traceback size configurable via `DD_TRACE_SPAN_TRACEBACK_MAX_SIZE`, but tracebacks for exceptions were still hardcoded with a limit of 30. The fix is modeled after `Span#set_traceback`'s existing logic, and tests were modeled after both `test_traceback_with_error` and `test_custom_traceback_size`. Not much new logic, just an edge case. ## Checklist - [x] Change(s) are motivated and described in the PR description - [x] Testing strategy is described if automated tests are not included in the PR - [x] Risks are described (performance impact, potential for breakage, maintainability) - [x] Change is maintainable (easy to change, telemetry, documentation) - [x] [Library release note guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html) are followed or label `changelog/no-changelog` is set - [x] Documentation is included (in-code, generated user docs, [public corp docs](https://github.com/DataDog/documentation/)) - [x] Backport labels are set (if [applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)) - [x] If this PR changes the public interface, I've notified `@DataDog/apm-tees`. ## Reviewer Checklist - [x] Title is accurate - [x] All changes are related to the pull request's stated goal - [x] Description motivates each change - [x] Avoids breaking [API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces) changes - [x] Testing strategy adequately addresses listed risks - [x] Change is maintainable (easy to change, telemetry, documentation) - [x] Release note makes sense to a user of the library - [x] Author has acknowledged and discussed the performance implications of this PR as reported in the benchmarks PR comment - [x] Backport labels are set in a manner that is consistent with the [release branch maintenance policy](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting) Co-authored-by: Matt Alexander <1710128+quiqueg@users.noreply.github.com>

…9391) Backport 1b954de from #9112 to 2.9. #7558 made the traceback size configurable via `DD_TRACE_SPAN_TRACEBACK_MAX_SIZE`, but tracebacks for exceptions were still hardcoded with a limit of 30. The fix is modeled after `Span#set_traceback`'s existing logic, and tests were modeled after both `test_traceback_with_error` and `test_custom_traceback_size`. Not much new logic, just an edge case. ## Checklist - [x] Change(s) are motivated and described in the PR description - [x] Testing strategy is described if automated tests are not included in the PR - [x] Risks are described (performance impact, potential for breakage, maintainability) - [x] Change is maintainable (easy to change, telemetry, documentation) - [x] [Library release note guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html) are followed or label `changelog/no-changelog` is set - [x] Documentation is included (in-code, generated user docs, [public corp docs](https://github.com/DataDog/documentation/)) - [x] Backport labels are set (if [applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)) - [x] If this PR changes the public interface, I've notified `@DataDog/apm-tees`. ## Reviewer Checklist - [x] Title is accurate - [x] All changes are related to the pull request's stated goal - [x] Description motivates each change - [x] Avoids breaking [API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces) changes - [x] Testing strategy adequately addresses listed risks - [x] Change is maintainable (easy to change, telemetry, documentation) - [x] Release note makes sense to a user of the library - [x] Author has acknowledged and discussed the performance implications of this PR as reported in the benchmarks PR comment - [x] Backport labels are set in a manner that is consistent with the [release branch maintenance policy](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting) Co-authored-by: Matt Alexander <1710128+quiqueg@users.noreply.github.com>

emmettbutler added 4 commits November 10, 2023 11:28

make traceback size configurable

1f30bcd

document new config variable

756ad34

reno

e706ca8

add a stubbed test

1129eec

emmettbutler requested review from a team as code owners November 10, 2023 19:53

emmettbutler requested review from majorgreys and mabdinur November 10, 2023 19:53

emmettbutler marked this pull request as draft November 10, 2023 19:53

fix test assertion

0a10509

emmettbutler changed the title ~~Emmett.butler/traceback config~~ chore(tracing): make traceback size limit configurable Nov 13, 2023

typing

76491c0

emmettbutler marked this pull request as ready for review November 13, 2023 17:23

mabdinur approved these changes Nov 14, 2023

View reviewed changes

emmettbutler enabled auto-merge (squash) November 15, 2023 14:50

emmettbutler added 3 commits November 15, 2023 06:50

Merge branch '2.x' into emmett.butler/traceback-config

6435ec6

Merge branch '2.x' into emmett.butler/traceback-config

e844bfc

Merge branch '2.x' into emmett.butler/traceback-config

d2a215a

majorgreys approved these changes Nov 15, 2023

View reviewed changes

Merge branch '2.x' into emmett.butler/traceback-config

7d0225a

emmettbutler merged commit b09b21e into 2.x Nov 15, 2023
139 checks passed

emmettbutler deleted the emmett.butler/traceback-config branch November 15, 2023 16:41

quiqueg added a commit to quiqueg/dd-trace-py that referenced this pull request Apr 27, 2024

Fix DD_SPAN_TRACEBACK_MAX_SIZE for exception tracebacks

54a870d

DataDog#7558 made the traceback size configurable, but tracebacks for exceptions were still hardcoded with a limit of 30.

quiqueg added a commit to quiqueg/dd-trace-py that referenced this pull request Apr 27, 2024

Fix DD_SPAN_TRACEBACK_MAX_SIZE for exception tracebacks

e2c3b89

DataDog#7558 made the traceback size configurable, but tracebacks for exceptions were still hardcoded with a limit of 30.

quiqueg added a commit to quiqueg/dd-trace-py that referenced this pull request Apr 27, 2024

Fix DD_SPAN_TRACEBACK_MAX_SIZE for exception tracebacks

d760458

DataDog#7558 made the traceback size configurable, but tracebacks for exceptions were still hardcoded with a limit of 30.

quiqueg mentioned this pull request Apr 27, 2024

fix(tracing): fix traceback max size for exceptions #9112

Merged

17 tasks

github-actions bot mentioned this pull request May 24, 2024

fix(tracing): fix traceback max size for exceptions [backport 2.7] #9389

Merged

17 tasks

github-actions bot mentioned this pull request May 24, 2024

fix(tracing): fix traceback max size for exceptions [backport 2.8] #9390

Merged

17 tasks

github-actions bot mentioned this pull request May 24, 2024

fix(tracing): fix traceback max size for exceptions [backport 2.9] #9391

Merged

17 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore(tracing): make traceback size limit configurable #7558

chore(tracing): make traceback size limit configurable #7558

emmettbutler commented Nov 10, 2023 •

edited by mabdinur

Loading

pr-commenter bot commented Nov 10, 2023 •

edited

Loading

chore(tracing): make traceback size limit configurable #7558

chore(tracing): make traceback size limit configurable #7558

Conversation

emmettbutler commented Nov 10, 2023 • edited by mabdinur Loading

Checklist

Reviewer Checklist

pr-commenter bot commented Nov 10, 2023 • edited Loading

Benchmarks

scenario:iastpropagation-no-propagation

scenario:otelspan-start-finish

scenario:sethttpmeta-no-useragentvariant

scenario:sethttpmeta-obfuscation-regular-case-explicit-query

scenario:sethttpmeta-obfuscation-regular-case-implicit-query

scenario:sethttpmeta-useragentvariant_not_exists_2

emmettbutler commented Nov 10, 2023 •

edited by mabdinur

Loading

pr-commenter bot commented Nov 10, 2023 •

edited

Loading