-
Notifications
You must be signed in to change notification settings - Fork 619
feat(resource-detector-gcp)!: contribute Google's comprehensive resource detector #3007
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
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3007 +/- ##
=======================================
Coverage 91.44% 91.44%
=======================================
Files 146 146
Lines 8192 8192
Branches 1846 1846
=======================================
Hits 7491 7491
Misses 701 701 Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
c9f524d to
c83c261
Compare
c83c261 to
ea3d3af
Compare
ea3d3af to
f9051c0
Compare
|
@psx95 would you mind giving a review here? |
psx95
left a comment
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.
LGTM.
I'm assuming the newly added files are copied over from the operations repo. I noticed that probably Cloud Run Jobs are missing ?
Yup 335091f |
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.
I think we are still unable to suppress tracing on this detector. But it's good for now.
I'll try it once published.
ref: #2320
Ya based on the discussion I don't think this problem will be solved |
Which problem is this PR solving?
Moving the "official" GCP resource detector from GoogleCloudPlatform/opentelemetry-operations-js (@google-cloud/opentelemetry-resource-util on NPM) to this repo. This package will now be the official one we recommend to customers and support, rather than living in Google's repo 😃. Once this is released, we will delete the detector code out of Google's repo.
Short description of the changes
Fixes #2933
I have kept individual commits for easier review. In summary:
GcpDetectorIntegration.test.tspassing, andnpm run lint:format.I've marked this as a breaking change, since the behavior will be different. Overall this implementation covers more environments (GCE, GKE, Cloud Run, Cloud Run (+Functions), App Engine Standard and Flex) and follows Resource SemConv. Google has an extensive E2E test suite that deploys the detector in all supported environments, which runs in GoogleCloudPlatform/opentelemetry-operations-js today. I will be updating those tests to periodically run this resource detector and make any fixes here.
For more context, see discussion on #2818.