-
Notifications
You must be signed in to change notification settings - Fork 624
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
Added aio-pika instrumentation #1095
Added aio-pika instrumentation #1095
Conversation
I did not understand exactly what i should add to the |
PR title and Link to it. Follow the same pattern of other entries in CHANGELOG.md. |
…iss/opentelemetry-python-contrib into feature/aio-pika-instrumentation
Thanks for the PR! Following the contributing guidelines, would you be okay to be a CODEOWNER/maintainer for this package? |
@lzchen sounds good to me 👍 |
instrumentation/opentelemetry-instrumentation-aio-pika/README.rst
Outdated
Show resolved
Hide resolved
instrumentation/opentelemetry-instrumentation-aio-pika/README.rst
Outdated
Show resolved
Hide resolved
instrumentation/opentelemetry-instrumentation-aio-pika/README.rst
Outdated
Show resolved
Hide resolved
instrumentation/opentelemetry-instrumentation-aio-pika/setup.cfg
Outdated
Show resolved
Hide resolved
...metry-instrumentation-aio-pika/src/opentelemetry/instrumentation/aio_pika/aio_pika_getter.py
Outdated
Show resolved
Hide resolved
...instrumentation-aio-pika/src/opentelemetry/instrumentation/aio_pika/instrumented_exchange.py
Outdated
Show resolved
Hide resolved
...elemetry-instrumentation-aio-pika/src/opentelemetry/instrumentation/aio_pika/span_builder.py
Outdated
Show resolved
Hide resolved
...elemetry-instrumentation-aio-pika/src/opentelemetry/instrumentation/aio_pika/span_builder.py
Outdated
Show resolved
Hide resolved
...opentelemetry-instrumentation-aio-pika/src/opentelemetry/instrumentation/aio_pika/version.py
Outdated
Show resolved
Hide resolved
Please add yourself to component-owners |
@ofek1weiss You more enough approvals for this. Please fix the lint and resolve conflicts so that we can merge this. |
Hi @ofek1weiss, I'd happily address the linting problems for you if you don't have time at the moment I'm very interested on this integration 😅 |
Hi sorry about that, I saw the approvals and thought it meant it was done... 😅 I`m on it now |
there seem to be issues with the linting in files that I did not touch, does anybody know why is that? |
@ofek1weiss If the failures are not related to your changes, please ignore they will get fixed on main. |
@srikanthccv 👍 , then everything should be OK. |
Please update the core repo sha. I believe you are missing some breaking changes from the core repo which is causing lint to fail. |
@lzchen it seems that on my branch it is the same as the one you sent ( |
@ofek1weiss try updating that hash in your PR to the latest commit in the core repo |
Apparently this means the problem is in the virtual environment that is created for lint with this PR. Nevertheless, I checked the dependencies installed in both the lint environment created with Here are some results: Pip freeze of this PR lint environment:
Pip freeze of
Results of running the linter with
Results of running the linter with
Results of running the linter with this-PR code with a this-PR-generated lint environment:
Weird, right? 🤔 |
…y-python-contrib into feature/aio-pika-instrumentation
…iss/opentelemetry-python-contrib into feature/aio-pika-instrumentation
I looked at it a bit, and it seems that the requirement for |
@ofek1weiss |
@lzchen yeah, but it was not in pylint, but in the readme validation (which is the last one), I fixed it so this time (hopefully) everything should work |
Description
Added instrumentation for
aio-pika
Type of change
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
aio-pika
s docs and checked them with the instrumentation)Does This PR Require a Core Repo Change?
Checklist: