-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
[receiver/googlecloudmonitoringreceiver] Adding Google Cloud monitoring receiver #34015
base: main
Are you sure you want to change the base?
[receiver/googlecloudmonitoringreceiver] Adding Google Cloud monitoring receiver #34015
Conversation
dff685d
to
97ff395
Compare
Hello @abhishek-at-cloudwerx, please sign the CLA to enable this PR to be reviewed. |
@crobert-1 - Yes we're working on getting the org CLA sorted. Unfortunately we are unable to get any help in figuring out who registered as Org admin. |
@crobert-1 - The CLA is now signed and PR is ready for review. @dashpole - Tagging as Sponsor |
@dashpole I made the changes as per your comment and removed the service_name field since we now fetch details directly based on the metric_name. |
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 after nits addressed
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 after nits addressed, @TylerHelmuth should review as well
@TylerHelmuth @dashpole - Could one of you help merge this ? |
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.
Please take a look at the failing CI
return errors.New("field \"metric_name\" is required and cannot be empty for metric configuration") | ||
} | ||
|
||
if metric.Delay < 0 { |
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.
Is a value of 0 ok?
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.
Yes, this validation is intended to reject negative values only.
Description: Adding Google Cloud Monitoring receiver
Link to tracking Issue: #33762
Testing: Test cases for configuration added
Documentation: README file added for describing the component