-
Notifications
You must be signed in to change notification settings - Fork 628
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
Audit and test opentelemetry-instrumentation-elasticsearch NoOpTracer… #1616
Audit and test opentelemetry-instrumentation-elasticsearch NoOpTracer… #1616
Conversation
'{"found": false, "timed_out": true, "took": 7}', | ||
) | ||
es = Elasticsearch() | ||
es.get(index="test-index", doc_type="_doc", id=1) |
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.
I suggest validating es.get response value -> so we'll know for sure that no spans were created due to NoOpTracerProvider
…-elasticsearch-no-op-tracer
b0afd30
to
4be4c43
Compare
@@ -24,6 +24,7 @@ | |||
|
|||
import opentelemetry.instrumentation.elasticsearch | |||
from opentelemetry import trace | |||
from opentelemetry import trace as trace_api |
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.
trace already imported in the upper line
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
@@ -37,7 +37,7 @@ | |||
|
|||
major_version = elasticsearch.VERSION[0] | |||
|
|||
if major_version == 7: | |||
if major_version 7: |
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.
Is this a mistake?
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.
I think so, I don't know who pushed this change but I will revert it
…-elasticsearch-no-op-tracer
ffed02e
to
34475e1
Compare
…-elasticsearch-no-op-tracer
…-elasticsearch-no-op-tracer
…-elasticsearch-no-op-tracer
…/github.com/shalevr/opentelemetry-python-contrib into Change-metrics-tests-to-work-with-test_base * 'Change-metrics-tests-to-work-with-test_base' of https://github.com/shalevr/opentelemetry-python-contrib: Fix issue with Flask instrumentation when a request spawn children threads and copies the request context (open-telemetry#1654) Add connection attributes to sqlalchemy connect span (open-telemetry#1608) Add boto3sqs to docs (open-telemetry#1666) Audit and test opentelemetry-instrumentation-elasticsearch NoOpTracer… (open-telemetry#1616) Copy change log updates from release/v1.16.x-0.37bx (open-telemetry#1683) Update version to 1.17.0.dev/0.38b0.dev (open-telemetry#1677) Fix CI Failure (open-telemetry#1680) Add better debugging if hatch subprocess fails (open-telemetry#1672) Add confluent kafka docs (open-telemetry#1668) Support aio_pika 9 (open-telemetry#1670) Audit and test opentelemetry-instrumentation-wsgi NoOpTracerProvider (open-telemetry#1610) bot (open-telemetry#1667) Add commit method for ConfluentKafkaInstrumentor's ProxiedConsumer (open-telemetry#1656) Revert open-telemetry#1097 (open-telemetry#1660) Audit and test opentelemetry-instrumentation-django NoOpTracerProvider (open-telemetry#1611) Audit and test opentelemetry-instrumentation-aiohttp-client NoOpTrace… (open-telemetry#1612) Audit and test opentelemetry-instrumentation-flask NoOpTracerProvider (open-telemetry#1614) Audit and test opentelemetry-instrumentation-dbapi NoOpTracerProvider (open-telemetry#1607)
Add test for elasticsearch using NoOpTracerProvider
Fixes #984
How has this been tested?
tox -e test-instrumentation-elasticsearch