prometheus-nats-exporter
prometheus-nats-exporter copied to clipboard
proposal: refactory to use less/no reflection
Hi everyone, thanks for the great work on NATS and on this exporter!
So, I was looking in issues like #37 and #39, and I was looking forward into implementing it, but I found some issues that, while not blockers, would make the code smell bad.
The exporter currently uses reflection to determine the metrics, so, we have a generic exporter, to which we pass an endpoint and it exposes all metrics as gauges.
Some problems in there:
- we may have metrics better exposed as other types (eg: counters)
- the metrics don't have the
HELP
statement
This is kind of OK for the current scope, but, if we are going to expose streaming metrics as well, this will be a bigger issue.
On the reflection side, for example, if we want channel subscription metrics (streaming/channelsz?subs=1
), the actually metrics are under subscriptions
on the json, not in the root, so we will need to keep "going down" until we reach the actual metrics.
My proposal is:
- get rid of the reflection in favor of a simpler solution, parsing the JSON output into structs and etc
- add the missing HELPs
- finally, start adding the streaming metrics
We can still have plenty of "common code" (http request and etc), we just will have more declarative prometheus.Desc
and the parsing will be made to the appropriate structs instead of map[string]interface{}
.
What do you think? If you agree, I can work on a prototype...
I think this is a good idea. It'll require more maintenance as new metrics from servers are exposed, but will be the correct way to do this.
already did that on streaming and connz metrics.
#54 #75