Suppress botocore downstream instrumentation like urllib3

[NathanielRN]

Description

I noticed that if I instrument both urllib3 and botocore that I will get 2 traces (e.g. S3 and s3.amazon.com) when I should only get 1.

Before this fix:

After this fix:

Type of change

Please delete options that are not relevant.

• Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

The first way I thought to write a test for this would be to create an instance of the urllib3 Instrumentor and prove that only 1 trace gets created, but since this is a common pattern among all the instrumentation libraries I didn't add it.

Let me know if this would be useful though!

Does This PR Require a Core Repo Change?

• 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

[lzchen]

Does botocore and urllib3 share the same code path when executing a client call?

[NathanielRN]

@lzchen Yes as far as I am aware!

A quick dive into the botocore package helped me find that botocore > httpsession.py has the following code:

class URLLib3Session(object):
...
def send(self, request):
        try:
            ...
            # conn is a `urllib3.connectionpool.HTTPConnectionPool` derived class type
            urllib_response = conn.urlopen(
                method=request.method,
                url=request_target,
                body=request.body,
                headers=request.headers,
                retries=Retry(False),
                assert_same_host=False,
                preload_content=False,
                decode_content=False,
                chunked=self._chunked(request.headers),
            )

And in our instrumentation we do the following for the opentelemtry-instrumentation-urllib package:

opentelemetry-python-contrib/instrumentation/opentelemetry-instrumentation-urllib3/src/opentelemetry/instrumentation/urllib3/__init__.py

Lines 141 to 145 in bf97e17

	wrapt.wrap_function_wrapper(
	urllib3.connectionpool.HTTPConnectionPool,
	"urlopen",
	instrumented_urlopen,
	)

So the trace is recording twice.

[lzchen]

I believe requests also has this problem. We reserved this key word for http instrumentation suppression. Maybe we should use this instead, and also do something similar.

[NathanielRN]

@lzchen That's a fair ask, I see that the urllib3 instrumentation will check both keys:

opentelemetry-python-contrib/instrumentation/opentelemetry-instrumentation-urllib3/src/opentelemetry/instrumentation/urllib3/__init__.py

Lines 220 to 224 in bf97e17

	def _is_instrumentation_suppressed() -> bool:
	return bool(
	context.get_value(_SUPPRESS_INSTRUMENTATION_KEY)
	or context.get_value(_SUPPRESS_HTTP_INSTRUMENTATION_KEY)
	)

Further botocore only requires jmespath, urllib3, python-dateutil so the only one we will instrument is probably the HTTP library. Will make the change!

[lzchen]

Not a blocker for this pr, but we should probably move the context variable to a common location like here.