Use a shorter timeout for AWS EC2 metadata requests #1089
Conversation
Fix open-telemetry#1088 According to the docs, the value for `timeout` is in seconds: https://docs.python.org/3/library/urllib.request.html#urllib.request.urlopen. 1000 seconds seems slow and in some cases can block the startup of the program being instrumented (see open-telemetry#1088 as an example), because the request will hang indefinitely in non-AWS environments. Using a much shorter 1 second timeout seems like a reasonable workaround for this.
|
|
|
From the changelog check: @srikanthccv do you have any guidance for if this is worth a changelog entry? Was looking in the contributing guide and didn't see a definite answer one way or another. |
|
Bug fixes require a changelog entry even if the fix is simple. Please add entry. |
|
Thanks for this change @jemisonf ! This looks like the right move to me, OTel JS had a similar issue but in the other direction; a timeout of 1 second was not enough, they needed at least 5 seconds. In that case, do you think changing to be Could you also update the timeout in the |
|
5 seems fine to me! I'll get those changes in |
|
Quick note on the EKS change -- 2000 is probably higher than required so it definitely seems worth changing but I wouldn't expect the same issue to show up there since it's making a request to an internal k8s DNS name rather than using a public AWS IP like the EC2 detector |
Fix #1088
According to the docs, the value for
timeoutis in seconds: https://docs.python.org/3/library/urllib.request.html#urllib.request.urlopen. 1000 seconds seems slow and in some cases can block the startup of the program being instrumented (see #1088 as an example), because the request will hang indefinitely in non-AWS environments. Using a much shorter 1 second timeout seems like a reasonable workaround for this.Description
Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.
Fixes # (issue)
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.