-
Notifications
You must be signed in to change notification settings - Fork 63
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
fix(resource-detector): properly detect zone on AppEngine standard #715
Conversation
Summary for this issue in other language detectors:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: The console.log
statements could be removed.
The change looks good.
I think it does have the issue. This test doesn't provide the slash separated string for GAE standard and the detector just returns the zone directly |
We might need to add a test to prevent regression, but going by the implementation of |
Here is a test update GoogleCloudPlatform/opentelemetry-operations-java#341 thanks |
Fixes #655
It turns out GAE standard environment's metadata server returns a zone string like
projects/myproject/zones/us15
. ut integration tests only run on AppEngine flex not standard so this wasn't caught 🙁. Here are HTTP logs I printed out from a sample app:I updated the tests and manually tested a sample app on GAE standard writing metrics:
I think this bug is also affecting all other resource detectors e.g. Go's impl that this was copied from