Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] Add metrics endpoint to watchdog #688

Closed
wants to merge 1 commit into from

Conversation

alexellis
Copy link
Member

@alexellis alexellis commented May 20, 2018

Signed-off-by: Alex Ellis (VMware) alexellis2@gmail.com

Description

Uses Prometheus client library. This commit is WIP and will need
to be altered to provide RED metrics in the format expected by
K8s HPAv2 / Weave Cloud / Prometheus operator.

By collecting metrics at the watchdog level we can enable Kubernetes HPAv2 to work on requests/per/second.

Motivation and Context

  • I have raised an issue to propose this change (required)

#583 #532

How Has This Been Tested?

WIP - shows that the metrics are showing up.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I've read the CONTRIBUTION guide
  • I have signed-off my commits with git commit -s
  • I have added tests to cover my changes.
  • All new and existing tests passed.

Uses Prometheus client library. This commit is WIP and will need
to be altered to provide RED metrics in the format expected by
K8s HPAv2 / Weave Cloud / Prometheus operator.

Signed-off-by: Alex Ellis (VMware) <alexellis2@gmail.com>
@alexellis
Copy link
Member Author

@stefanprodan can you link back to the RED metrics format with the histogram/counter expected by the tooling?

@alexellis alexellis changed the title Add metrics endpoint to watchdog [WIP] Add metrics endpoint to watchdog May 20, 2018
@stefanprodan
Copy link
Contributor

stefanprodan commented May 20, 2018

These are the Prometheus metrics names and format that the tooling expects:

HPA v2 http_requests_total

	counter := prometheus.NewCounterVec(
		prometheus.CounterOpts{
			Subsystem: "http",
			Name:      "requests_total",
			Help:      "The total number of HTTP requests.",
		},
		[]string{"status"},
	)

RED http_request_duration_seconds

	histogram := prometheus.NewHistogramVec(prometheus.HistogramOpts{
		Subsystem: "http",
		Name:      "request_duration_seconds",
		Help:      "Seconds spent serving HTTP requests.",
		Buckets:   prometheus.DefBuckets,
	}, []string{"method", "path", "status"})

A full example of a HTTP server instrumentation can be found here https://github.com/stefanprodan/k8s-podinfo/blob/master/pkg/server/instrument.go

}

promHTTP := prometheus.Handler()
http.Handle("/_/metrics", promHTTP)
Copy link
Contributor

@stefanprodan stefanprodan May 20, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The recommend way of exposing the metrics endpoint is with github.com/prometheus/client_golang/prometheus/promhttp. The handler should be http.Handle("/_/metrics", promhttp.Handler())

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was put out for discussion and you didn't raise an issue with it.

#547

We agreed to use a private namespace for the routes of /_/ so that it could not clash with the function in the container. Same for the health endpoint.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there was a reason I moved back to this version of the handler. How to the metrics get registered and associated with the first method/approach?

@LucasRoesler
Copy link
Member

@alexellis and @stefanprodan is this something we still want to do?

@alexellis
Copy link
Member Author

Yes

@alexellis alexellis mentioned this pull request Apr 3, 2019
11 tasks
@alexellis
Copy link
Member Author

Derek close: replaced by #1151

@derek derek bot closed this Apr 3, 2019
@alexellis alexellis deleted the add_metrics_to_watchdog branch October 8, 2019 16:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants