Add Riemann sink #379
Add Riemann sink #379
Conversation
|
Can one of the admins verify this patch? |
|
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project, in which case you'll need to sign a Contributor License Agreement (CLA). 📝 Please visit https://cla.developers.google.com/ to sign. Once you've signed, please reply here (e.g.
|
|
CCLA signed under Whitepages. |
|
/cc @aphyr |
|
cc @vmarmol |
|
Seems I can't see the failing e2e test; is there a way to retrieve the result? |
|
We're having issues with the e2e, I'd ignore it for now. |
|
OK. What else do we need to review before this can be merged? |
| @@ -0,0 +1,22 @@ | |||
| { | |||
There was a problem hiding this comment.
Can we place a symlink to an existing service file instead of creating a new one?
|
Overall LGTM. Sorry for the delay in reviewing this PR. This needs a rebase as well. |
|
And do squash your commits, once you have gone through the comments. |
|
Great -- I'll get this caught up. |
|
@jfoy did you get a chance to work on this PR? |
|
I will -- thanks for your patience. |
|
@jfoy: Ping. |
|
Not forgotten. We've been iterating on this internally, and I have a set of changes ready to go, which I'll push with the rebase and squash. |
|
Done and squashed. |
|
What's going on with our CCLA? We signed one from Whitepages, which seems to have vanished. |
|
Fixing dependencies to unblock unit tests. (How embarrassing.) |
|
CLAs look good, thanks! |
1 similar comment
|
CLAs look good, thanks! |
|
|
||
| deps: | ||
| go get github.com/tools/godep | ||
| go get github.com/progrium/go-extpoints |
There was a problem hiding this comment.
This should not be here since we use the vendored go-extpoints version as seen in https://github.com/kubernetes/heapster/blob/8e4a8ba63e75fe234bedb4171b35553d6ea18981/Godeps/Godeps.json#L6
| @@ -0,0 +1 @@ | |||
| ../influxdb/heapster-service.json | |||
|
@jfoy: This is amazing! Can't wait to try it out. Can you post some documentation on how users can use Reimann with heapster? Maybe some sample alert configs. Other than documentation, everything else LGTM. |
| @@ -13,6 +13,8 @@ install: | |||
| - go get github.com/axw/gocov/gocov | |||
| - go get golang.org/x/tools/cmd/cover | |||
| - go get github.com/mattn/goveralls | |||
| - go get github.com/onsi/ginkgo | |||
| - go get github.com/onsi/gomega | |||
There was a problem hiding this comment.
These should probably be vendored. If not, I'm thinking that the riemann tests may break at any point.
Didn't godep save ./... add them?
There was a problem hiding this comment.
No, my mistake. I hadn't run godeps save.
|
That was weird. Reverting to the prior state until I figure out what's going on with |
|
@jfoy I can take care of the godep stuff if you like - remove all the godep changes and I'll do a PR on top of yours. |
|
@mvdan Awesome, thank you. I would like to understand where the variation comes from, though -- if I do a 'godep restore' followed by a 'godep save -t', why does the resulting Godeps/Godeps.json show so many deleted packages? |
|
Running exactly those commands, the answer is simple - you have to give |
|
Here's what I see: |
|
@vishh I'm pulling together some config samples. |
Support a Riemann sink (derived from the influxdb sink). Map Heapster events into Riemann: * Map Heapster event structures to Riemann events. * Coerce Int64 metrics to Int for transport to Riemann. * Support user-provided Riemann tags and event state, with defaults. Includes basic documentation and sample Riemann config.
|
@jfoy funnily enough, I get the same thing you get when running save on master. It obviously breaks: Some of the things it does, like removing |
|
@cadvisorJenkinsBot: test this please |
|
I can't tell by looking at the output -- is this another artifact of the e2e pipeline? |
|
It may be enough to just rebase & do |
|
Godep is working fine for me now from a clean |
|
@jfoy: Any updates on this PR? I can help address the godeps issue if you are busy. |
|
I would love help addressing the godeps issue. I'm anxious to get this merged. |
|
@jfoy: godeps can be a pain at times. I went ahead and fixed godeps on HEAD and also included your patches in the process. I will open a separate PR that will include your patches in this PR. Sorry for the trouble with godeps. |
|
closing this in favor of #687 |
Fixes #293