Cluster-api-provider-aws: Metrics/Tracing

[randomvariable]

Would be good to get at least metrics added early on.

A cluster operator may want to metrics from the controllers available in their monitoring system of choice, e.g. Prometheus. I would be interested in metrics with regards to the creation, deletion, mutation and failure rates of AWS APIs, and the state of their objects at least to help diagnose when limits have been reached.

Specifically, adding https://opencensus.io/quickstart/go/metrics/ would do this in a vendor agnostic fashion and allow the choice of tracing / metrics system, incl. CloudWatch and AWS X-Ray.

[randomvariable]

if there's a strong preference for prometheus client_golang, also fine.

[vincepri]

We should have some umbrella issue to track this, not sure what the standards is across k8s components.

CC @detiber

[randomvariable]

Found them. https://github.com/kubernetes/community/blob/master/contributors/devel/instrumentation.md

https://docs.google.com/document/d/1U_h7osUDF6plKn_nkDF2Y-EjZA6BY8RR6LfTezrFBwM/edit?ts=5b926ad1 may also be useful.

[randomvariable]

From the new KEP, this seems to be important:

Kubernetes also makes extensive use of a global metrics registry to register metrics to be exposed. Aside from general shortcomings of global variables, Kubernetes is seeing actual effects of this, causing a number of components to use sync.Once or other mechanisms to ensure to not panic, when registering metrics. Instead a metrics registry should be passed to each component in order to explicitly register metrics instead of through init methods or other global, non-obvious executions.

While uncertain at this point, once cleaned up, this effort may put us a step closer to having stability guarantees for Kubernetes around metrics. Currently metrics are excluded from any kind of stability requirements.

As we're green field, we have an opportunity to do things right, any advice from @ehashman or @brancz would be appreciated.

[brancz]

The Prometheus client is currently what’s universally used in Kubernetes. Specifically regarding metrics more important is the standard wire format that both the Prometheus client as well as the opencensus library are going to support (this effort is called OpenMetrics). I have little experience with the opencensus project, it looks promising especially as all signals are instrumented with it, but also rather early stage. Given all of the above I’d recommend using the Prometheus client library at this point for metrics. Beyond that I recommend not to use the global metrics registry for the same reasons as any global variables (for the record we are actually seeing api metrics leaking into the scheduler in Kubernetes for example because of the use of the global metrics registry). Instead pass the registry down to components just like a logger.

[randomvariable]

Here's the extant AWS instrumentation in kubernetes/kubernetes
https://github.com/kubernetes/kubernetes/blob/master/pkg/cloudprovider/providers/aws/aws_metrics.go

[detiber]

@randomvariable I like this idea very much, should we also track this at the more generic cluster-api level as well?

[detiber]

@vincepri I think we can use this issue as the umbrella issue and spawn out more specific issues to address as identified.

[randomvariable]

/assign

[detiber]

/milestone Next

[randomvariable]

On the last attempt at this, it seemed like one of the best ways for instrumenting AWS calls is to use the WithContext methods in the AWS SDK, and add a hook to get stats. It required opening up the HTTP requests to do well, so left it last time.

[fejta-bot]

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

[vincepri]

/remove-lifecycle stale

[chuckha]

/assign @randomvariable

[randomvariable]

Will sync with SIG Instrumentation to get latest advice

[ncdc]

@randomvariable I assume you're not actively working on this?

[randomvariable]

Nope, would have lifecycled/active'd it if I were.

I didn't provide an update at the time because I think SIG Instrumentation was in the process of considering OpenCensus, which has been added to container runtime since.

[ncdc]

Do you still want to be assigned?

[randomvariable]

Sure. Now we're upgraded, this is implementable.

[fejta-bot]

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

[detiber]

/lifecycle frozen