opentelemetry-python-contrib
opentelemetry-python-contrib copied to clipboard
Audit and test otel instrumentation for httpx
Description
Audit and test otel instrumentation for httpx
Fixes #979
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?
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
- [ ] Test A
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
The committers listed above are authorized under a signed CLA.
- :white_check_mark: login: SuryanarayanaPeri / name: Suryanarayana Peri (51b2910b2b0c9ce343d014afe94320acce2fdbb4, d327a1dada46dac410bdaf5354636a679adc41c4, 102a8cad1877cda60f6d101c6fac57269f472a0f)
Also a small suggestion is include the related issue's link to the description, would be easier for reviewer to check. #979 instead of # 979
@zhihali - Thank you for your comments. I am working on your recommendations and will be submitting the PR again.
@brianwarner since it looks like you are the boss of the people doing this kind of PRs could you please sort out the CLA or even setup an internal review before opening PRs here? That would save quite a bit of time on this side. Thanks.
@brianwarner since it looks like you are the boss of the people doing this kind of PRs could you please sort out the CLA or even setup an internal review before opening PRs here? That would save quite a bit of time on this side. Thanks.
Hi @xrmx, sure, I can provide some context here. I'm not the boss, but I am responsible for managing our internal processes.
We've been configured in EasyCLA as a corporate contributor for quite some time, and I've enabled as much automation as the platform allows. However, in EasyCLA, every new contributor must verify that they want to be covered by the CCLA, and they're not prompted to do this until the PR is opened. There are also occasional config issues to dig through, but these also don't surface until the PR is opened. Either way, I'm helping contributors fix the issues on the back end.
As for reviews, we have an internal process where commits are first staged against an internal mirror of this project, run through various checks, and then get pushed to the public fork. Any requested changes are first staged internally, and when they're approved I push them out to the public fork. If there's something additional you'd like to see from contributors, I can provide that feedback to the teams doing the work.
@zhihali - I have made the changes to the lint and see all the tests are successful, except for the "changelog". I am not able to add the labels. Can you please review and let me know if all looks good.
@xrmx - The Tox tests for Python 3.10 are failing on my local machine after several changes were pushed to the repository in the past few days. Previously, they were running completely fine, but some of the tests are now failing, possibly due to the recent changes. The tests seem to be running fine with Python 3.9. Please help me with this issue and let me know if I am missing anything.
@SuryanarayanaPeri Not sure what are you talking about, here it is failing for this reason: https://github.com/open-telemetry/opentelemetry-python-contrib/actions/runs/10090168599/job/27899069159#step:6:261
@xrmx - I have made the lint related changes and now all the Checks are passed. Can you please review.
Marking as a draft until test is fixed
@emdneto Can you please suggest the similar tests that are implemented for me to take as a reference to complete this issue.
- Surya
emdneto left a comment • I think the test is wrong. Please take a look at how other tests are implemented. The no_op_tracer_provider test should be run for both Async and Sync clients.