pickle support for the GraphiteReporter #536
Conversation
This is based on https://github.com/BrightcoveOS/metrics-graphite-pickle but refactored for metrics 3.
| writer.write(payload); | ||
| writer.flush(); | ||
| } catch (Exception e) { | ||
| if (LOGGER.isDebugEnabled()) { |
There was a problem hiding this comment.
The default Graphite client doesn't log anything itself. It simply counts failures and re-throws.
|
This is very useful for us too. Any chance getting this merged or at least looked at? |
|
It has been looked at. I'm in favor of this however I do need a chance to review the code further and test it. I do have concerns about extending GraphiteReporter, so perhaps I'll refactor that. (In any event, the fields should be protected, not package-private) |
|
The biggest reason I extended Graphite was to avoid changing GraphiteReporter. I suppose we could create an interface that Graphite and PickledGraphite both implement, that GraphiteReporter could use. Is that the refactor you were considering? |
|
Hi, just checking in to see if there was anything else you want me to do on this PR. |
|
Nothing at the moment. It will almost certainly be accepted, but v4 is a ways off and I'm pretty busy at the moment. |
|
This won't make it into a 3.x release? |
|
No, barring a catastrophic bug there will be no further v3 releases. However due to the nature of reporters it is trivial to separate it out into a new project/include it in your own codebase. |
|
I think @samperman hoped that he could move away from that with 3.x |
|
Part of this pull request was to relax some of the visibility of the metrics internals... so its a little more than trivial to have a separate project (unless I copy and paste the entire Graphite class) |
|
True. I'll seriously consider a 3.1 release, however it still wouldn't be out for a while. |
|
Moving to #639 |
This is based on https://github.com/BrightcoveOS/metrics-graphite-pickle but refactored for
metrics 3.