-
Notifications
You must be signed in to change notification settings - Fork 45
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
Resource Detector for Cloud Run/Cloud Functions #194
Resource Detector for Cloud Run/Cloud Functions #194
Conversation
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, @dashpole can you approve if the resource detection logic looks good?
/gcbrun |
"cloud.platform": "gcp_cloud_run", | ||
"cloud.region": all_metadata["instance"]["region"].split("/")[-1], | ||
"faas.id": all_metadata["instance"]["id"], | ||
"gcp.resource_type": "cloud_run", |
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'm not familiar with gcp.resource_type
. It seems like duplicate information with cloud.platform. Why is it included?
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.
It predates cloud.platform
so it was our hack to make things work. We should get rid of it, see #85
"cloud.platform": "gcp_cloud_functions", | ||
"cloud.region": all_metadata["instance"]["region"].split("/")[-1], | ||
"faas.id": all_metadata["instance"]["id"], | ||
"gcp.resource_type": "cloud_functions", |
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.
same as above.
Going to merge this, and then track removal of the extra attribute with #85 |
Based on the Go code.
Fixes #193