Implementation of all Vitess counters => Prom

[zmagg]

Implementing Prometheus-native metrics.

Types of metrics to implement

• Counters
• MultiCounters
• MultiCounterFunc
• Timings
• MultiTimings
• direct calls to NewInt()
• direct calls to stats.Publish(...IntFunc())

Other things to do

• Add tests for each of the types of metrics
• Get TravisCI working with the new packages I've added to the vendor file (promhttp)

Things not ported

• NewString / NewStringMap / PublishJSONFunc
  Prom. doesn't handle string values. Some of these could be converted into different types, if we'd like to export the metrics anyway.

• NewFloat
  This isn't called anywhere today. We could maybe export them anyway in case there are any callers in the future.

• direct calls to stats.Publish(...CounterFunc())
  I think this should get ported in a followup PR, possibly refactoring it to use MultiCounterFunc.

Additional things to do in follow-on PRs

• Visit Rates / DurationFunc callsites and instrument them so that we can derive the rate/duration in Prometheus at querytime.

Breaking changes

As much as possible, I tried to not change anything about the existing expvar stats. However, there were a few inconsistencies in namings that made it difficult to do the camelcase => snakecase transformation necessary for Prometheus exporting. Also, it's nice to have a consistent name for things.

The following are things that changed:

• VTGateWarnings => VtgateWarnings to match the rest of the things that have a Vtgate prefix.

[demmer]

Overall I think this is really good and you're definitely on the right track!

[demmer]

Better golang style would be to just call this function Init.

You also need a doc comment of the form "// Init initializes the backend with the given namespace" to appease the linter.

More to the point you should install govet / golint / etc and hook it up with your editor.

[zmagg]

ooh TIL.

[zmagg]

Ran govet & golint on all the packages I added. Is there a convention for what to do for files in a package that you edited that you didn't modify that fail? (stats/ring.go fails)

[demmer]

This doesn't seem to me something that warrants another public API function... better IMO would be to just fold this in as part of Init.

[demmer]

None of these NewXYZCollector functions should be exposed as part of the public API.

I think you should either make them newXYZCollector or just inline the struct creation into prombackend.NewMultiCounter

[zmagg]

This is great feedback, this is a lot clearer inlined.

[demmer]

It seems to me that if you stash the pointer to mc as part of the multiCountersCollector, then in the Collect implementation you don't need to go back again and call stats.GetMultiCounter(name) since you can just have the pointer to the underlying counter in the multiCountersCollector itself.

[demmer]

I feel like this can all be simplified a lot if we go with the approach I mentioned before where the pointer to the underlying counter / multicounter / multicounterfunc object is stored in the collector itself.

Then you wouldn't need all these separate locks -- you can have a single one for the registry of pull backends and pull metrics since the only coordination needs to be at initialization / registration time.

[demmer]

This looks like an erroneous typo

[demmer]

Missing godoc string

[demmer]

Another mistaken edit.

[demmer]

Well you put help in Counters already... I think both belong there.

[demmer]

I think you don't need this if the Help() method is defined on the Counters struct.
It also seems like we want a corresponding Name() function at the same level.

[zmagg]

I don't think we need Name() (or to store it), so I've gotten rid of it.

[zmagg]

Superceded by vitessio#3784