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

feat: Add exclude urls feature to HTTPX instrumentation #1900

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

zqumei0
Copy link

@zqumei0 zqumei0 commented Aug 4, 2023

Description

Modified HTTPX instrumentation to allow for skipping instrumentation for a list of excluded URLs similar to the current solutions.

Fixes #539

Type of change

Please delete options that are not relevant.

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

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

  • tox -e py310-test-instrumentation-httpx21

Does This PR Require a Core Repo Change?

  • Yes. - Link to PR:
  • No.

Checklist:

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

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

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Aug 4, 2023

CLA Signed

The committers listed above are authorized under a signed CLA.

@zqumei0 zqumei0 marked this pull request as ready for review August 17, 2023 21:17
@zqumei0 zqumei0 requested a review from a team August 17, 2023 21:17
@macieyng
Copy link
Contributor

@zqumei0 merge in current main. I tested it locally and had no issues with tests. Should be good on pipeline. After this, mention maintainer or approver to run it.

@macieyng
Copy link
Contributor

I don't get why those tests are failing. It worked on my machine. Can anyone help out with debugging?

@ocelotl
Copy link
Contributor

ocelotl commented Oct 2, 2023

I don't get why those tests are failing. It worked on my machine. Can anyone help out with debugging?

Weird, I checked out this PR with gh co 1900. Tried running tox -e py311-test-instrumentation-httpx18 -- -rf. It first failed until I added these changes:

diff --git a/instrumentation/opentelemetry-instrumentation-httpx/pyproject.toml b/instrumentation/opentelemetry-instrumentation-httpx/pyproject.toml
index 50f3fccb..1632f9c4 100644
--- a/instrumentation/opentelemetry-instrumentation-httpx/pyproject.toml
+++ b/instrumentation/opentelemetry-instrumentation-httpx/pyproject.toml
@@ -28,6 +28,7 @@ dependencies = [
   "opentelemetry-api ~= 1.12",
   "opentelemetry-instrumentation == 0.42b0.dev",
   "opentelemetry-semantic-conventions == 0.42b0.dev",
+  "opentelemetry-util-http"
 ]
 
 [project.optional-dependencies]
diff --git a/instrumentation/opentelemetry-instrumentation-httpx/src/opentelemetry/instrumentation/httpx/__init__.py b/instrumentation/opentelemetry-instrumentation-httpx/src/opentelemetry/instrumentation/httpx/__init__.py
index 1ecf4596..1fe3a21b 100644
--- a/instrumentation/opentelemetry-instrumentation-httpx/src/opentelemetry/instrumentation/httpx/__init__.py
+++ b/instrumentation/opentelemetry-instrumentation-httpx/src/opentelemetry/instrumentation/httpx/__init__.py
@@ -176,7 +176,7 @@ from opentelemetry.semconv.trace import SpanAttributes
 from opentelemetry.trace import SpanKind, TracerProvider, get_tracer
 from opentelemetry.trace.span import Span
 from opentelemetry.trace.status import Status
-from opentelemetry.utils.http import (
+from opentelemetry.util.http import (
     ExcludeList,
     get_excluded_urls,
     parse_excluded_urls,

After that, it also failed but by different reasons. @zqumei0 can you please check this?

@kmcquade
Copy link

kmcquade commented Aug 24, 2024

Hi 👋 I could also use this feature, although I don't quite have the time to deeply contribute other than do some testing on my end. In an attempt to be useful, I tried following the steps to get started and at least run the tests on my machine. However, the tests were also failing for me.

@macieyng I would have dug into the test logs in GitHub Actions but it looks like the logs have expired at this point ☹️

I tried the steps that @ocelotl laid out but it didn't work either.

After installing the dependencies in the requirements.txt files, I ran this command: tox -e py311-test-instrumentation-httpx18 -- -rf

And here was the output:

py311-test-instrumentation-httpx18: commands_pre[0] /Users/kinnaird/tmp/opentelemetry-python-contrib/instrumentation/opentelemetry-instrumentation-httpx/tests> python -m pip install -U pip setuptools wheel
Requirement already satisfied: pip in /Users/kinnaird/tmp/opentelemetry-python-contrib/.tox/py311-test-instrumentation-httpx18/lib/python3.11/site-packages (24.2)
Requirement already satisfied: setuptools in /Users/kinnaird/tmp/opentelemetry-python-contrib/.tox/py311-test-instrumentation-httpx18/lib/python3.11/site-packages (73.0.1)
Requirement already satisfied: wheel in /Users/kinnaird/tmp/opentelemetry-python-contrib/.tox/py311-test-instrumentation-httpx18/lib/python3.11/site-packages (0.44.0)
py311-test-instrumentation-httpx18: commands_pre[1] /Users/kinnaird/tmp/opentelemetry-python-contrib/instrumentation/opentelemetry-instrumentation-httpx/tests> pip install '"opentelemetry-api[test] @ "git+https://github.com/open-telemetry/opentelemetry-python.git@main"'
ERROR: Invalid requirement: '"opentelemetry-api[test] @ "git+https://github.com/open-telemetry/opentelemetry-python.git@main"': Expected package name at the start of dependency specifier
    "opentelemetry-api[test] @ "git+https://github.com/open-telemetry/opentelemetry-python.git@main"
    ^
Hint: It looks like a path. File '"opentelemetry-api[test] @ "git+https://github.com/open-telemetry/opentelemetry-python.git@main"' does not exist.
py311-test-instrumentation-httpx18: exit 1 (0.18 seconds) /Users/kinnaird/tmp/opentelemetry-python-contrib/instrumentation/opentelemetry-instrumentation-httpx/tests> pip install '"opentelemetry-api[test] @ "git+https://github.com/open-telemetry/opentelemetry-python.git@main"' pid=80564
  py311-test-instrumentation-httpx18: FAIL code 1 (0.57=setup[0.02]+cmd[0.37,0.18] seconds)
  evaluation failed :( (0.64 seconds)

@zqumei0 if you or anyone wants to pick up this PR, I'm happy to do some testing and give some feedback.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add exclude urls feature to HTTPX instrumentation
4 participants