-
Notifications
You must be signed in to change notification settings - Fork 626
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
adding response_hook to elastic instrumentation #670
Conversation
...ry-instrumentation-elasticsearch/src/opentelemetry/instrumentation/elasticsearch/__init__.py
Outdated
Show resolved
Hide resolved
9e7fded
to
0eff6fe
Compare
This looks fine but it's a bit strange to just have a response hook without request. Let's add a request hook as well and we can merge this. Thanks |
@@ -97,17 +97,23 @@ def _instrument(self, **kwargs): | |||
""" | |||
tracer_provider = kwargs.get("tracer_provider") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you update the usage docs with an example of using the request/response hooks with their function signatures?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ItayGibel-heliosphere
Updates from others aren't allowed in this PR. Can you rebase with main so we can merge this in?
Description
opentelemetry-instrumentation-elasticsearch
: added response_hook callback passed as an argument to the instrument method, enabling the users to add data from the response to the span attributesType of change
How Has This Been Tested?
test_response_hook
- a new unit test testing the hook activationDoes This PR Require a Core Repo Change?
Checklist:
See contributing.md for styleguide, changelog guidelines, and more.