Add instrumentation for aiobotocore

[stj]

What does this pull request do?

This adds instrumentation for aiobotocore. It heavily uses the existing instrumentation from boto3.

I got the tests to run locally for this, but noticed that the instrumentation of aiohttp.client already sends spans for aiobotocore. Though they are not labelled as AWS specific as the boto3 instrumenter does.

As is, this implementation currently results in 2 spans for each request. I am looking for some advice on what is the better approach here. Try to remove the wrapper on the ClientSession used by aiobotocore or change the spans created by ClientSession?

Related issues

Nothing opened (yet)

[apmmachine]

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS

Expand to view the summary

Build stats

• Start Time: 2022-04-07T17:27:20.660+0000

• Duration: 22 min 10 sec

Test stats 🧪

Test	Results
Failed	0
Passed	4829
Skipped	3293
Total	8122

💚 Flaky test report

Tests succeeded.

🤖 GitHub comments

To re-run your PR in the CI, just comment with:

• /test : Re-trigger the build.

• /test linters : Run the Python linters only.

• /test full : Run the full matrix of tests.

• /test benchmark : Run the APM Agent Python benchmarks tests.

• run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

[basepi]

This is awesome! Very clean way to re-use the botocore instrumentation. 👍

[basepi]

@stj A few tests failing. It appears that with aiobotocore it's not correctly detecting the service. The span type/subtype are set in the handle_* functions, which are chosen dynamically when we detect the service:

apm-agent-python/elasticapm/instrumentation/packages/botocore.py

Lines 61 to 82 in 114ee6c

	service_model = instance.meta.service_model
	if hasattr(service_model, "service_id"): # added in boto3 1.7
	service = service_model.service_id
	else:
	service = service_model.service_name.upper()
	service = endpoint_to_service_id.get(service, service)
	parsed_url = urllib.parse.urlparse(instance.meta.endpoint_url)
	context = {
	"destination": {
	"address": parsed_url.hostname,
	"port": parsed_url.port,
	"cloud": {"region": instance.meta.region_name},
	}
	}
	handler_info = None
	handler = handlers.get(service, False)
	if handler:
	handler_info = handler(operation_name, service, instance, args, kwargs, context)
	if not handler_info:
	handler_info = handle_default(operation_name, service, instance, args, kwargs, context)

Can you take a look and see if you can figure out where we're going wrong in the service detection for aiobotocore?

[stj]

This is awesome! Very clean way to re-use the botocore instrumentation. +1

Thanks, the way the library instruments sync and async made this a fairly straight forward change!

Though, as I mentioned, this is currently creating two spans for each request to AWS (one created by the instrumentation of aiohttp.client and another by this instrumentation). I am not sure what is the cleanest way to handle this and though I put the PR up to discuss this.

My current thinking is to find out if the wrapper for aiohttp.client can be removed for the ClientSession instance used by aiobotocore. WDYT?

[basepi]

Oh, right, sorry, I missed your discussion note.

That also probably explains the test failures, we're probably inspecting the http span (which is external) instead of the s3 span (which is storage).

The correct solution is to mark the spans created by this instrumentation as leaf=True, which means that sub-spans will be dropped. This is how we handle spans where we don't want the underlying http span (such as spans from the elasticsearch client, for example).

But...you already are doing that, so there might be a bug.

[stj]

The correct solution is to mark the spans created by this instrumentation as leaf=True, which means that sub-spans will be dropped. This is how we handle spans where we don't want the underlying http span (such as spans from the elasticsearch client, for example).

TIL

[stj]

Turns out that not creating an async def call method on the async instrumentor closes the span to early. Basically the sync call method, as I had it, returned a coro. The return exits the span context manager. When the coro is awaited the span for aiohttp.client does not find a leaf span in the execution context and continues as normal.

4e778c9 makes changes to botocore and aiobotocore instrumentation classes to avoid code duplication as much as possible.

[basepi]

/test

[basepi]

/test