Micrometer: SignalFx support

[jkschneider]

This originally started in #147, but SignalFx suggests that custom metrics should go direct to API rather than through the collectd agent.

[jkschneider]

Direct-to-API docs for SignalFx: https://developers.signalfx.com/docs/metric-data-overview

[jkschneider]

SignalFx provides a java client as well: https://github.com/signalfx/signalfx-java. It has dependencies on Jackson 2.2 (Boot 2 is on Jackson 2.9) and Guava 20.

[codefromthecrypt]

cc @mpetazzoni

[mpetazzoni]

Thanks, I definitely want to take a look at this.

First comments I would make:

• I would strongly recommend that https://github.com/signalfx/signalfx-java be used behind the scenes. In particular, it will guarantee that ProtocolBuffers are used as the wire format (instead of the less efficient JSON). It will also properly handle connection management and pooling. Finally, it will make this shim easier to maintain going forward as SignalFx will take care of any changes that need to happen to signalfx-java if anything changes on the SignalFx API side;
• we prefer SignalFx to be written exactly with that capitalization, and not "SignalFX" or "Signalfx" (both in code and documentation/issues/PRs) :-)

[jkschneider]

@mpetazzoni I'd also prefer to use the signalfx-java client if possible for all the reasons you mentioned. At the very least, we need it to not use Guava (or shade it if you must) and Jackson. These dependencies are poison to downstream consumers depending on different versions (Guava because of its aggressive deprecation cycle and Jackson because of version alignment issues).

[jkschneider]

@mpetazzoni Also, curious what you think about this discussion in Prometheus land about their protobuf wire format. It is being removed because they found the savings to be negligible compared with gzipped text. Naturally JSON is a little more verbose than the Prom line format, but I wouldn't be surprised if the same held for SignalFx. Also, not sure if your API accepts a gzipped payload or not; haven't tried it myself yet.

[mpetazzoni]

Re: shading of Guava and Jackson. We've had this discussion internally recently and I think that's something we'll very likely do in a future released version of signalfx-java. If this is a blocker for you, we can prioritize accordingly (/cc @molner).

Re: Protobuf vs JSON. I agree that the savings of Protobuf are negligible compared to gzipped JSON, but only if you care about size on the wire. Where Protobuf is really making a difference is on the amount of CPU needed to process incoming data on our ingest service, with JSON being much more expensive.

Re: API accepting gzipped payloads, I'm not sure. @mdubbyap can probably shine some light on it.

[codefromthecrypt]

>

Re: Protobuf vs JSON. I agree that the savings of Protobuf are negligible
compared to gzipped JSON, but only if you care about size on the wire.
Where Protobuf is really making a difference is on the amount of CPU needed
to process incoming data on our ingest service, with JSON being much more
expensive.

this part makes sense, a hand-written json encoder can be made to run at
the same order of magnitude as a binary one (in zipkin we hand-write
encoders to make json perf less crap in apps). OTOH it is usually an order
of magnitude or approaching that on the receiving side due to the
complexity of unwinding potentially malformed json. Not sure the trees
burnt is worth always doing binary in the middle, as that depends on your
customers. Zipkin we almost always prefer json just because support is such
hell and people can't reliably paste binary etc.

[mpetazzoni]

Agreed, and that's why our API will always support both. But between our public signalfx-java library and SignalFx, there's no reason not to use the most efficient transport all around :)

[codefromthecrypt]

Agreed, and that's why our API will always support both. But between our
public signalfx-java library and SignalFx, there's no reason not to use
the most efficient transport all around :)

heh well.. there's at least one reason, because of support. If you don't
experience support issues where users struggle to understand what their app
sent etc (because they can't or don't know how to switch from json), then
that reason, is not one you have. Efficiency is one thing to optimize for
and in the case of protocols optimizes against text-based protocol support.
That's all I meant to bring up.

[jkschneider]

@mpetazzoni If we can address the dependency issues, I'd be happy to leave transport decisions up to your team.

I know it's a lot to ask, but I have to go 1.0 GA by January 24 because I'll be blocking Boot's RC1. If you can't cut a release without those deps before then, I'm fine leaving signalfx-java out just for now, with an eventual plan to replace our manual API calls with it later in Micrometer 1.1.0.

[jkschneider]

Initial support was released with rc7. Discussion can continue here or in a new issue, either way.

[mpetazzoni]

I'll discuss with the team on Tuesday. If they agree, I'm pretty confident we can get you a release by the end of the week, giving you enough time before January 24th to switch over.

[mpetazzoni]

FYI, PR for shading signalfx-java's dependencies is out at https://github.com/signalfx/signalfx-java/pull/53. Once merged, we'll release v0.0.40 and you can use that. I'll let you know here when that's ready!

[mpetazzoni]

@jkschneider I've just released 0.0.40:

<dependency>
  <groupId>com.signalfx.public</groupId>
  <artifactId>signalfx-java</artifactId>
  <version>0.0.40</version>
</dependency>

It may take a little bit to appear on Maven Central.

[jkschneider]

@mpetazzoni FYI, your build process synced a snapshot to Maven Central:

[mpetazzoni]

@jkschneider yeah, that was an earlier mistake made by someone on the team. Unfortunately it cannot be removed :( The 0.0.40-SNAPSHOT artifact on there is actually the same as the 0.0.39. But the 0.0.40 pushed yesterday is good:

We've improved our tooling as well to make sure this doesn't happen in the future.

[jkschneider]

@mpetazzoni Yeah, sorry. I hadn't noticed the date on that artifact initially 🙄. I also wasn't sure whether Gradle's various dynamic constraints would select the snapshot or the release (0.+ ,etc), but looks like all variants select the release. All is good!