Skip to content

Conversation

@PapaPedro
Copy link
Contributor

When using API calls from boto we often do not see the operation names
in the stack traces even though there is a dedicate function name. This
is because of the generic way that api calls are implemented in boto.
Unfortunately this is not convenient for profiling as we do not see in
flamegraphs which calls are using resources.

This change adds a specific check when we build the stacks to detect api
calls inside boto and if possible add a new frame with the operation
function name.

i.e. instead of

(...)
botocore.client:CloudWatch:_make_api_call
botocore.client:CloudWatch:_api_call
user_module:send_metric

we should end up with

(...)
botocore.client:CloudWatch:_make_api_call
botocore.client:CloudWatch:_api_call
botocore.client:CloudWatch:put_metric_data
user_module:send_metric

This new frame should help when investigating on flamegraphs but also
with automatic analysis.

Issue #, if available:

Description of changes:

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@mirelap-amazon mirelap-amazon requested a review from a team June 22, 2021 17:07
Comment on lines 88 to 91
and raw_frame.f_code.co_filename.endswith('botocore/client.py')
and raw_frame.f_locals and 'py_operation_name' in raw_frame.f_locals.keys()
and raw_frame.f_locals.get('py_operation_name')):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible to have a unit/integration test that checks a (real, not mocked) boto api call has this "py_operation_name" in the stack? It would help if boto is changing something, so we'd find out as soon as possible.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was a bit worried about doing a real boto call in unit tests because it can fail for different users based on their environment. But I added one that I think should work for everyone.

@mirelap-amazon mirelap-amazon requested a review from a team June 22, 2021 17:49
Copy link
Contributor

@mirelap-amazon mirelap-amazon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor comment if a new unit test is possible.

@PapaPedro PapaPedro force-pushed the boto-operation-name branch 4 times, most recently from 766b546 to 71b0824 Compare June 23, 2021 16:06
When using API calls from boto we often do not see the operation names
in the stack traces even though there is a dedicate function name. This
is because of the generic way that api calls are implemented in boto.
Unfortunately this is not convenient for profiling as we do not see in
flamegraphs which calls are using resources.

This change adds a specific check when we build the stacks to detect api
calls inside boto and if possible add a new frame with the operation
function name.

i.e. instead of
```
(...)
botocore.client:CloudWatch:_make_api_call
botocore.client:CloudWatch:_api_call
user_module:send_metric
```

we should end up with
```
(...)
botocore.client:CloudWatch:_make_api_call
botocore.client:CloudWatch:_api_call
botocore.client:CloudWatch:put_metric_data
user_module:send_metric
```

This new frame should help when investigating on flamegraphs but also
with automatic analysis.
@PapaPedro PapaPedro force-pushed the boto-operation-name branch from 71b0824 to 5f1cc70 Compare June 23, 2021 16:36
@PapaPedro PapaPedro merged commit da597fa into main Jun 23, 2021
@PapaPedro PapaPedro deleted the boto-operation-name branch July 8, 2021 17:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants