Skip to content
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

Check for XRAY_PATCH_AWS_SDK env var, and do not patch aws_sdk if unset #61

Merged
merged 2 commits into from
Oct 4, 2018

Conversation

barrucadu
Copy link
Contributor

@barrucadu barrucadu commented Oct 3, 2018

I'll open an issue on aws-xray-sdk if other people agree with my conclusion.


Since applying the last round of govuk_app_config upgrades, asset-manager and publishing-api have been using far more memory:

asset-manager

publishing-api

The memory shoots up as soon as the app is deployed, so I believe this is an overhead imposed at initialisation-time, rather than some sort of leak which manifests when requests are made.

After doing some debugging (commenting out lines of code and crossing my fingers), I narrowed this down to having X-Ray patch the aws_sdk gem. If that gem is patched, the problem occurs; if it is not, even if net_http is patched, the memory usage seems much closer to before.

It's a shame to lose tracking for stuff which goes through aws_sdk, but that's only a small amount of stuff (asset uploads from the asset-manger, and nothing(!) from the publishing-api), and I think we can assume that AWS is not the bottleneck in our architecture.


Trello card

@barrucadu
Copy link
Contributor Author

barrucadu commented Oct 4, 2018

Here are some more graphs. The spikes are when the app was deployed.

asset-manager continues to look good, memory usage has been creeping up but it's not yet at the point it was before X-Ray, and it's quite slow:

asset-manager

publishing-api was good, then a branch with X-Ray instrumenting aws_sdk was deployed. Memory usage is below what it was, but is still rather higher than it was without X-Ray instrumenting aws_sdk:

publishing-api

Copy link

@bevanloon bevanloon left a comment

Choose a reason for hiding this comment

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

Sounds like you've got a cause and effect here.

@barrucadu barrucadu changed the title [WIP] Check for XRAY_PATCH_AWS_SDK env var, and do not patch aws_sdk if unset Check for XRAY_PATCH_AWS_SDK env var, and do not patch aws_sdk if unset Oct 4, 2018
@barrucadu barrucadu merged commit 77a4018 into master Oct 4, 2018
@barrucadu barrucadu deleted the msw/xrayperiments branch October 4, 2018 12:56
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