[opentelemetry-instrumentation-django] Handle exceptions from request/response hooks

[ebracho]

Ensure that we are cleaning up spans and contexts in the case that user-provided hooks raise an exception.

Description

This change ensures that we are properly cleaning up spans and context tokens in the case that user-provided request/response hooks raise an exception. The motivation is a recent issue we had in one of our services where our request_hook unexpectedly raised an exception due to a type error and caused spans to leak.

slack thread with more context

Fixes # (issue): N/A

Type of change

Please delete options that are not relevant.

• [x] Bug fix (non-breaking change which fixes an issue)
• [ ] New feature (non-breaking change which adds functionality)
• [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)
• [ ] This change requires a documentation update

How Has This Been Tested?

Added new test cases to ensure that request/response hook exceptions don't cause spans to leak and are handled appropriately.

• TestMiddleware.test_request_hook_exception
• TestMiddleware.test_response_hook_exception

Does This PR Require a Core Repo Change?

• [ ] Yes. - Link to PR:
• [x] No.

Checklist:

See contributing.md for styleguide, changelog guidelines, and more.

• [x] Followed the style guidelines of this project
• [ ] Changelogs have been updated
• [x] Unit tests have been added
• [ ] Documentation has been updated

[linux-foundation-easycla[bot]]

The committers listed above are authorized under a signed CLA.

• :white_check_mark: login: ebracho / name: Eddie Bracho (8200f0bca3ba6471bffed466efd6eb22a1e3698d, aed8370b317699ff24fd58a67d88e4231445d54f, 354405dd4625ef1b7e5ca93f70be1a77bd3c6e62, 8acfdceba96761a95f1f351a4eb474c11aae2b56, 68cce000d4bd5f1df9cbef6c6133da347d1df685, 2bcfc6ca823ea937924c49f43718d51c4736945f, 41e5fffd126d7e8e6a333cc732cc67c1664b878a)
• :white_check_mark: login: lzchen / name: Leighton Chen (af00dd8f4d93ea119333f30a2fe999ba476e7f72, cafb6fb8ae5954dca5bdd575a6f8e14dfc2bfb67)
• :white_check_mark: login: ocelotl / name: Diego Hurtado (6c9595b6395e33a73ef7a35642d137936ee74252)

[lzchen]

I think we need to first answer the question: do we want to capture and handle exceptions and exception information for errors that occur in user supplied hooks? Is span information pertaining to misconfigured hooks something of substance to capture in a span and as telemetry?

@aabmass @ocelotl @srikanthccv

Thoughts?

[ebracho]

I think we need to first answer the question: do we want to capture and handle exceptions and exception information for errors that occur in user supplied hooks? Is span information pertaining to misconfigured hooks something of substance to capture in a span and as telemetry?

@aabmass @ocelotl @srikanthccv

Thoughts?

Looks like there’s some guidance on this in the OpenTelemetry spec: https://github.com/open-telemetry/opentelemetry-specification/blob/6fd4f0809bcff0ea5c4abe161803b4ad8628375e/specification/error-handling.md?plain=1#L24

At the very least I would expect spans created by an instrumentor to always be closed by that instrumentor regardless of whether an exception occurs.

[lzchen]

@ebracho

I agree with the "handling exception" portion, I am just not sure about the "capturing telemetry information" part about the exception itself in the span.

[srikanthccv]

I think we should log the exception and move on. This is our approach to all unexpected things that happen during instrumentation/export/recording metrics et al.

[xrmx]

@ebracho Please also update the branch with latest main to add the changelog entry to Unreleased and not 1.23.0.

[ebracho]

For some reason I'm having trouble running tox locally after one of the merges 🤔

(py311-test-instrumentation-django-3) eddiebracho@FRVG2M7F2Q-eddiebracho opentelemetry-python-contrib % tox -e lint
lint recreate: /Users/eddiebracho/lib/opentelemetry-python-contrib/.tox/lint
lint installdeps: -rdev-requirements.txt
lint installed: alabaster==0.7.16,astroid==3.0.3,attrs==23.2.0,Babel==2.14.0,black==22.3.0,bleach==4.1.0,certifi==2024.2.2,charset-normalizer==3.3.2,click==8.1.7,codespell==2.1.0,coverage==7.4.4,dill==0.3.8,docutils==0.20.1,flake8==6.1.0,flaky==3.7.0,httpretty==1.1.4,idna==3.6,imagesize==1.4.1,iniconfig==2.0.0,isort==5.12.0,Jinja2==3.1.3,MarkupSafe==2.1.5,mccabe==0.7.0,mypy==0.931,mypy-extensions==1.0.0,nh3==0.2.17,packaging==24.0,pathspec==0.12.1,platformdirs==4.2.0,pluggy==1.4.0,protobuf==3.20.3,py==1.11.0,pycodestyle==2.11.1,pyflakes==3.1.0,Pygments==2.17.2,pylint==3.0.2,pytest==7.1.3,pytest-cov==4.1.0,readme-renderer==42.0,requests==2.31.0,ruamel.yaml==0.17.21,six==1.16.0,snowballstemmer==2.2.0,Sphinx==7.1.2,sphinx-autodoc-typehints==1.25.2,sphinx-rtd-theme==2.0.0rc4,sphinxcontrib-applehelp==1.0.8,sphinxcontrib-devhelp==1.0.6,sphinxcontrib-htmlhelp==2.0.5,sphinxcontrib-jquery==4.1,sphinxcontrib-jsmath==1.0.1,sphinxcontrib-qthelp==1.0.7,sphinxcontrib-serializinghtml==1.1.10,tomli==2.0.1,tomlkit==0.12.4,typing_extensions==4.10.0,urllib3==2.2.1,webencodings==0.5.1
lint run-test-pre: PYTHONHASHSEED='484165117'
lint run-test-pre: commands[0] | python -m pip install 'git+https://github.com/open-telemetry/opentelemetry-python.git@main\'
Collecting git+https://github.com/open-telemetry/opentelemetry-python.git@main\
  Cloning https://github.com/open-telemetry/opentelemetry-python.git (to revision main\) to /private/var/folders/vh/qk269rmx5vlg70ghbmp9ht4h0000gp/T/pip-req-build-e256j3e7
  Running command git clone --filter=blob:none --quiet https://github.com/open-telemetry/opentelemetry-python.git /private/var/folders/vh/qk269rmx5vlg70ghbmp9ht4h0000gp/T/pip-req-build-e256j3e7
  WARNING: Did not find branch or tag 'main\', assuming revision or ref.
  Running command git checkout -q 'main\'
  error: pathspec 'main\' did not match any file(s) known to git
  error: subprocess-exited-with-error
  
  × git checkout -q 'main\' did not run successfully.
  │ exit code: 1
  ╰─> See above for output.
  
  note: This error originates from a subprocess, and is likely not a problem with pip.
error: subprocess-exited-with-error

× git checkout -q 'main\' did not run successfully.
│ exit code: 1
╰─> See above for output.

note: This error originates from a subprocess, and is likely not a problem with pip.
ERROR: InvocationError for command /Users/eddiebracho/lib/opentelemetry-python-contrib/.tox/lint/bin/python -m pip install 'git+https://github.com/open-telemetry/opentelemetry-python.git@main\' (exited with code 1)
_____________________________________________________________________________________________ summary _____________________________________________________________________________________________
ERROR:   lint: commands failed

[xrmx]

@ebracho tox -e lint works fine here with tox 4.14.1. What version do you have? are you on macos?

[ocelotl]

@ebracho I fixed lint issues and solved conflicts with main, one test case is failing, please take a look.

[ebracho]

@ocelotl thank you, I fixed the unit tests and an outdated comment.

Also I figured out my tox issues - I'm not exactly sure what what going on but installing and running tox under a fresh virtualenv did the trick.

[ebracho]

thanks everyone 🙏