-
Notifications
You must be signed in to change notification settings - Fork 675
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-boto #3376
base: main
Are you sure you want to change the base?
Audit and test opentelemetry-instrumentation-boto #3376
Conversation
Signed-off-by: Venkata Shreyas Kabekkodu <[email protected]>
Signed-off-by: Venkata Shreyas Kabekkodu <[email protected]>
Signed-off-by: Venkata Shreyas Kabekkodu <[email protected]>
Signed-off-by: Venkata Shreyas Kabekkodu <[email protected]>
Signed-off-by: Venkata Shreyas Kabekkodu <[email protected]>
Signed-off-by: Venkata Shreyas Kabekkodu <[email protected]>
Signed-off-by: Venkata Shreyas Kabekkodu <[email protected]>
Signed-off-by: Venkata Shreyas Kabekkodu <[email protected]>
Signed-off-by: Venkata Shreyas Kabekkodu <[email protected]>
Signed-off-by: Venkata Shreyas Kabekkodu <[email protected]>
Signed-off-by: Venkata Shreyas Kabekkodu <[email protected]>
Hi @codeboten , Could you please review this PR or help in getting necessary approvals/reviews for this PR? Thanks! |
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.
No need for a new file, just add the testcase to the other file
self.assertEqual(len(spans), 0) | ||
|
||
|
||
if __name__ == "__main__": |
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.
We don't need to run tests like this
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.
We don't need to run tests like this
Okay, I removed it in my new commit as I moved the entire function to the other file.
|
||
# Ensure no spans are created | ||
tracer = get_tracer_provider().get_tracer("test") | ||
if isinstance(self.noop_tracer_provider, NoOpTracerProvider): |
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.
Isn't this always true?
When you said other file, you meant this right? instrumentation/opentelemetry-instrumentation-boto/tests/test_boto_instrumentation.py |
Description
An #956 was found in the wsgi instrumentation by integration tests in the core repository. The issue should have been caught by unit tests in the instrumentation using the NoOpTracerProvider.
The outcome of this issue is to ensure each instrumentation contains a test using the NoOpTracerProvider and that it does not rely on the OpenTelemetry SDK. This test is specific to boto.
Fixes #989
Type of change
Please delete options that are not relevant.
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
Does This PR Require a Core Repo Change?
Checklist:
See contributing.md for styleguide, changelog guidelines, and more.