Prometheus metrics

[kminehart]

Related issue

#1176
ory/x#49

Proposed changes

I've changed my approach since the last PR that was made.

This is in-tandem with ory/x#49.

The general idea is to wrap the central types with a Metrics... struct, which can be used interchangeably with whatever it wraps.

A good example is seen with the consent/metrics.go or client/metrics.go file. This nice thing about this approach is that it keeps the metric collection fairly central, though there are a few places where WithMetrics(NewSQLManager) must be used instead of just NewSQLManager(...).

Not all the metrics requested were added in here. I figured since i was changing directions so many times, it would be worth checking out the approach for the current metrics I have now before I proceeded to implement this for the Strategy interface.

Checklist

• I have read the contributing guidelines
• I confirm that this pull request does not address a security vulnerability. If this pull request addresses a security
  vulnerability, I confirm that I got green light (please contact hi@ory.sh) from the maintainers to push the changes.
• I have added tests that prove my fix is effective or that my feature works
  • Tests are definitely doable here but will probably cause a lot of headache. I'll keep looking into this.
• I have added necessary documentation within the code base (if appropriate)
• I have documented my changes in the developer guide (if appropriate)

Further comments

See the other discussion comments in ory/x#49 for more implementation details about how metrics are collected.

[aeneasr]

I love this! This is amazing, so clear and clean. Just some minor things but it's pretty much good to go IMO!

[ghost]

I think this should fix current undefined: metricsx.Observer errors on the broken tests:

diff --git a/client/metrics.go b/client/metrics.go
index 9404ac25..a4b2d3db 100644
--- a/client/metrics.go
+++ b/client/metrics.go
@@ -11,7 +11,6 @@ import (

 type Metrics struct {
        prometheus.Collector
-       metricsx.Observer
        Manager

        Clients              prometheus.Gauge
diff --git a/consent/metrics.go b/consent/metrics.go
index 6fd01aee..48fa4eb9 100644
--- a/consent/metrics.go
+++ b/consent/metrics.go
@@ -11,7 +11,6 @@ import (

 type MetricsManager struct {
        Manager
-       metricsx.Observer
        ConsentRequestsHandled  metricsx.CounterVec
        ConsentRequestScopes    metricsx.CounterVec
        ConsentRequestAudiences metricsx.CounterVec
diff --git a/jwk/metrics.go b/jwk/metrics.go
index 123c1262..d8fb9482 100644
--- a/jwk/metrics.go
+++ b/jwk/metrics.go
@@ -10,7 +10,6 @@ import (

 type MetricManager struct {
        Manager
-       metricsx.Observer
        JWKs prometheus.Gauge
 }

[kminehart]

I didn't even look at the tests since they require this PR to be merged in first:

ory/x#49

which adds those undefined types.

[tutman96]

Is there an update to when we can expect this to land in master? More insights into the running instance of Hydra would be very helpful to narrow down some issues.

[aeneasr]

Is there an update to when we can expect this to land in master? More insights into the running instance of Hydra would be very helpful to narrow down some issues.

Unfortunately not right now. We support Jaeger / OpenTracing though which should probably help more than these prometheus stats :) Basic prometheus exporter is also supported in Hydra already.

[tutman96]

I am looking for more per-client metrics (number of token requests vs failed token requests) etc more than pure timings for the different paths. The metrics that are already exposed, while helpful, only show Go stats, and nothing specific to Hydra.