riemann-scala-client icon indicating copy to clipboard operation
riemann-scala-client copied to clipboard

Metric deserialization does not roundtrip long values

Open mallman opened this issue 8 years ago • 4 comments

I've run into an issue with roundtripping a Riemann event with a metric of type Long. The scenario is roughly as follows:

  1. I create and send an Event with a metric of type Long.
  2. I query for that event and receive a metric of type Float.
  3. :(

I've debugged the issue and it appears to be caused by a difference in the way riemann-scala-client transcodes metrics versus https://github.com/aphyr/riemann-java-client. Note in particular that when writing a metric value of type Long, the Riemann Java client sets both the int64 and float metric values on the underlying protobuf. Why? I don't know. But it does. Therefore, when the Riemann server answers a query for an event with a long metric, it sets the float value as well.

When riemann-scala-client decodes the event protobuf, it looks for a float value before a long: https://github.com/benmur/riemann-scala-client/blob/master/src/main/scala/net/benmur/riemann/client/Serializers.scala#L53. And therein lies the confusion.

I looked carefully but I actually couldn't find a method in the Riemann Java client code which decodes an event protobuf's metric into a single canonical value. I guess it's up to the user of the library to decide how to decode metric values. However, I did find code within the Riemann Ruby client codebase which decodes an event protobuf's metric: https://github.com/aphyr/riemann-ruby-client/blob/master/lib/riemann/event.rb#L192. It looks for a double value first, then a long, then a float. In that scheme, a long metric encoded by the java client would indeed be decoded as a long (even though the float metric value exists as well in the underlying protobuf).

I've thought about how to rectify this. The easiest fix (by far) is to simply change the metric decoding to work like the java client. However, this would introduce an incompatibility with previous versions. (Perhaps a warning and a bump in version to 0.4.0 would suffice.)

A more ambitious fix would be to support both decoding schemes, at least as a bridge to cross-compatibility: a "legacy" mode preserving the previous decoding behavior and a "standard" or "java" mode emulating the java client behavior. This is something I've sketched out, but it's pretty ambitious and touches a lot of code.

What do you think?

mallman avatar Oct 20 '15 05:10 mallman