-
Notifications
You must be signed in to change notification settings - Fork 33
Gcr in process collector #339
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
69b58bb
to
931fa2b
Compare
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 entirely sure about the helper method signature and maybe this is a good chance for us to change it. The with_snapshot
parameter only used in one helper, which suggests that snapshot collection is not a common behavior and probably should be extracted from the helper method.
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.
Overall LGTM, added some minor comments for your consideration. The request for changes is mostly because of typo suggestions.
Co-authored-by: Andrey Slotin <andrey.slotin@instana.com>
Co-authored-by: Andrey Slotin <andrey.slotin@instana.com>
Co-authored-by: Andrey Slotin <andrey.slotin@instana.com>
Co-authored-by: Andrey Slotin <andrey.slotin@instana.com>
b9bdf2f
to
3fe6df9
Compare
Implementing the Google Cloud Run in process collector according to the technical documentation https://github.com/instana/technical-documentation/tree/a9a649ea2200e373766a537742083fc0a7db1b5e/tracing/in-process-collectors/google-cloud-run and following the existing architecture used for aws fargate