-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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
Fixes #2099 -- Add GCP Stackdriver support to telegraf. #3774
Conversation
4f45b2a
to
31e61a1
Compare
Thanks for the pull request @derrley, I'll try to take a closer look soon, probably during the 1.7 cycle. |
@derrley thanks so much for this PR! Do you mind providing a little more information about your use case and how you use this in your environment? |
@russorat By doing this we can have all our alerts in one place (kapacitor) and avoid defining, and paying for, alerts in Stackdriver |
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.
Thanks for the PR. Add some tests, a readme, and attend to the requested changes and we'll get this merged.
case metricpb.MetricDescriptor_STRING: | ||
field = p.Value.GetStringValue() | ||
default: | ||
// TODO: AddError here? Is it really an error? |
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.
Maybe either skip the metric, as you currently are, or default it to a chosen type.
@derrley Do you still have time available, and are you interested in continuing this?Thanks |
Latest work is on #4977 |
Required for all PRs:
This is my first PR -- looking to get some feedback on the direction of this. We've got it running in our own environment with mostly successful results, but I know others might have different environments. In the mean time, I'm going to be working on unit tests and beefing up the README. The example config should make the features fairly clear.