Updated NATS exporter link

[bjflanne]

@brian-brazil - I've updated the NATS Exporter to be the one maintained by the NATS team, which was released this week. The previous link was to Lovoo's exporter - Markus (maintainer of Lovoo's) is aware of this change and fine with it.

[brian-brazil]

This new exporter appears to be talking to multiple NATS servers, whereas the current one has the one-to-one mapping we recommend in our guidelines for exporters. May I ask why you've chosen to ignore this guideline?

[brian-brazil]

Also as an exporter you should be using ConstMetrics rather than GaugeVec/CounterVec.

[ColinSullivan1]

@brian-brazil , Thank you for the comments.

Concerning the one-to-one mapping, a NATS server cluster (multiple NATS servers linked together) operates as one entity from perspective of NATS clients, so I designed the exporter to be able to mirror this for monitoring. That being said, one could easily configure a one-to-one mapping with multiple NATS exporter instances, and I'd be happy to describe that as a Prometheus recommended best practice in our own documentation.

We have had discussions about instrumenting the NATS server directly, which would certainly be a one-to-one mapping, but that is still in the design/discussion phase at this point.

I appreciate the note about ConstMetrics, and will certainly take a look at that. Do you see any other areas we can improve on?

Thanks,
Colin

[brian-brazil]

I designed the exporter to be able to mirror this for monitoring

How a user uses a system isn't always how it should be monitored, and we pretty strongly recommend against the exporter architecture you've implemented as service discovery is the responsibility of Prometheus - not of the exporter. If you wish to replace the existing exporter which is 1:1, you also need to be 1:1. See https://prometheus.io/docs/instrumenting/writing_exporters/#deployment

Do you see any other areas we can improve on?

Nothing else from a quick peek, bar the PrometheusMetricConfig struct being unused.

[ColinSullivan1]

@brian-brazil , I updated the README, usage, etc for the 1:1 usage pattern in the guidelines. Can you update the link at this point?

[brian-brazil]

As far as I can tell, it's still possible with the compiled binary. An internal API is fine, but we've found users repeatedly think their standard use case is advanced.

The tag parameter should also be removed from the CLI (or at least the docs), as that's the responsibility of Prometheus SD+relabelling.

[brian-brazil]

Thinking on this a bit more, removing the reference to the tag parameter would be sufficient as that's the only anti-pattern the readme currently mentions.

[ColinSullivan1]

@brian-brazil , Finally got back to this! I've removed all references to tags. Are we good to go once @bjflanne resolves conflicts?

Thanks,
Colin

[brian-brazil]

https://github.com/nats-io/prometheus-nats-exporter/blob/master/README.md#monitoring is still mentioning the server-id, which I understand won't be there now.

[brian-brazil]

It looks like the existing one listed is completely abandoned, so there's no much point continuing to list it even if it is technically more compliant. Thanks for the PR!