dd-trace-py icon indicating copy to clipboard operation
dd-trace-py copied to clipboard

fix(wsgi): do not mark StopIteration spans with error

Open ZStriker19 opened this issue 1 year ago • 3 comments

Following this: https://github.com/miguelgrinberg/flask-sock/issues/64#issuecomment-1664709789 StopIteration is just a normal expected exception upon socket close. I am wondering if we should re-raise though? If we do, then we need to add additional code so this isn't marked as an error.

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 are followed or label changelog/no-changelog is set
  • [x] Documentation is included (in-code, generated user docs, public corp docs)
  • [x] Backport labels are set (if applicable)
  • [x] If this PR changes the public interface, I've notified @DataDog/apm-tees.
  • [x] If change 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.

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 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

ZStriker19 avatar Jan 19 '24 19:01 ZStriker19

Benchmarks

Benchmark execution time: 2024-01-31 19:12:28

Comparing candidate commit 609f3453ff1a2ba9153d76e34d9998e60f195640 in PR branch zachg/ignore_stopiteration_exception with baseline commit 222ce130c5bdbfbafc92de1a88361e164b8dae19 in branch main.

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

scenario:span-add-metrics

  • 🟥 max_rss_usage [+21.839MB; +21.950MB] or [+52.113%; +52.378%]

pr-commenter[bot] avatar Jan 19 '24 20:01 pr-commenter[bot]

Agreed, I saw your change but wasn't sure mine belonged there, after more thought, this seems like the best way.

ZStriker19 avatar Jan 19 '24 22:01 ZStriker19

Hi @ZStriker19, is this good to merge in?

Jhoong-Harvey avatar Mar 09 '24 22:03 Jhoong-Harvey

@ZStriker19 the span module was moved, would you be able to update this changeset from main so we can work on getting it merged in?

brettlangdon avatar May 07 '24 17:05 brettlangdon

Replacing with https://github.com/DataDog/dd-trace-py/pull/9788

tabgok avatar Jul 11 '24 12:07 tabgok