✨ pkg/manager,metrics: Expose ServingMetrics func

[lilic]

Exposes ServingMetrics() function and moves it to the metrics package, so serving metrics can be done independently of calling .Start().

Implemented as per the suggestion from this comment.

Closes #336

cc @DirectXMan12

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: LiliC
To fully approve this pull request, please assign additional approvers.
We suggest the following additional approver: directxman12

If they are not already assigned, you can assign the PR to them by writing /assign @directxman12 in a comment when ready.

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[lilic]

Noticed once the unit tests failed, but reran the job and they passed. Not sure if this is a documented flake? Don't think it's introduced with this PR? It was the source_integration_test.go test.

[grantr]

From #337 (comment):

Have a helper in pkg/metrics called ServeMetrics that takes in a net.Listener and does what .serveMetrics

I propose a slightly different interface: Create a PrometheusMetricsServer struct and have it implement manager.Runnable. Then the Manager doesn't need any special metrics server handling; the main method can just call manager.Add(metricsServer).

We've been experimenting with this in Knative here where we have two Runnable http servers (one for metrics) managed by the Manager. (note the use of a waitgroup is only required because of #350).

It's also relevant for #305, where we may have multiple metrics exporters running in the process.

[lilic]

I propose a slightly different interface:

That works for our use case very well for our use cases as well. Would be happy to implement that if everyone agrees on it. 👍

[DirectXMan12]

That seems pretty reasonable. I'd still like to have "metrics by default" where you can explicitly disable them, but making it a runnable seems fine.

[lilic]

That seems pretty reasonable.

Okay, will close this PR and we can discuss the details in the #336