Use Metrics libraries for HTTP serving and JSON serialization #9
Conversation
This replaces the old Rack based HTTP server with a Jetty-based one. A Rack app is still exposed, but the handling of it is left to the user. This way, the Rack app can be mounted anywhere in an existing application. Moreover, and more importantly, is that both of these now use the Metrics JSON serializations library, which means that it is possible to get very consistent reporting even with applications using vanilla Metrics.
I assume that #to_h is only used as an intermediate step in the serialization for most users, why I think it may be ok to skip it. By using Metrics JSON library, it's easier to maintain a consistent format across applications. Or maybe rather; it is difficult not to.
lib/multimeter/rack.rb
Outdated
There was a problem hiding this comment.
The code above is duplicated between json.rb and here.
There was a problem hiding this comment.
I realized that as soon as I shut my laptop yesterday as well. Will fix.
|
I think it would be good to make some of the dependencies optional. For example: if you don't install Jetty you don't get the standalone HTTP serving. |
|
Yes, I've thought about that as well. I was thinking about having separate gems, e.g. multimeter and multimeter-http. Then one could follow the same convention if we ever want to add support for the health checks and other Metrics features without cluttering the original gem with tons of stuff that the average user won't use. But such an approach requires more maintenance from the contributors and might be confusing for users. What do you think? |
|
I like the idea of having multiple gems. |
|
Multiple gems looks good. This would have to be released as a new major version since it changes the API in a backwards-incompatible way. If there are any other API changes you would like to see, now would be a good time. |
lib/multimeter/http.rb
Outdated
|
As we talked about: move as much of the JSON-code into Java. |
Rather than converting data back and forth
|
I've re-implemented |
To support Java 7
|
I now see that I've realized that the tests are failing twice, both when writing the last comment and when writing the PR description :) It appears that Jetty 9.3 only works with Java 8, so I downgraded to 9.2 to support Java 7 (http://www.eclipse.org/jetty/documentation/current/what-jetty-version.html). We don't need to support Java 6, right? |
I really like the direction in which Multimeter seems to be heading, in that it is moving closer and closer to Metrics without sacrificing the Rubyesqueness. This is an attempt to move even further in that direction. As Multimeter has moved towards Metrics, I've found that the hash representations of the metrics feels a bit arbitrary and the HTTP interface a bit clumsy. To make the serializable representations of the metrics more consistent and the HTTP interface a bit cleaner I've imported JSON serialization and servlet libraries from Metrics and replaced
#to_hwith#to_jsonand serve the metrics using Jetty instead. It is still possible to use a Rack handler, but the user can now mount the Rack app as she pleases rather than having everything setup automatically.This PR is still to be considered to be a work in progress, as well as a learning experience for my part. I'd primarily like feedback on the idea of using these extra libraries and pretty much breaking all backwards compatibility.
PS. the tests are failing due to an old Java version. I haven't really looked into exactly why yet.