util: expose otel metrics through grpc and prometheus by jsternberg · Pull Request #4460 · moby/buildkit

jsternberg · 2023-12-04T22:39:29Z

This exposes the otel metrics through the environment variables for the
otel collector or through the prometheus /metrics endpoint through the
debug address.

This also adds metrics for the grpc endpoints from the opentelemetry
libraries.

For the opentelemetry collector, the metric temporality is delta which
reduces the amount of data that goes over the wire overall. Prometheus
metrics are cumulative because prometheus metrics require that.

crazy-max · 2023-12-05T18:55:55Z

+	go.opentelemetry.io/otel/sdk v1.21.0
+	go.opentelemetry.io/otel/trace v1.21.0
 	go.opentelemetry.io/proto/otlp v1.0.0
 	golang.org/x/crypto v0.14.0
 	golang.org/x/net v0.17.0
 	golang.org/x/sync v0.3.0
-	golang.org/x/sys v0.13.0
+	golang.org/x/sys v0.14.0
 	golang.org/x/time v0.3.0
-	google.golang.org/genproto/googleapis/rpc v0.0.0-20230711160842-782d3b101e98
-	google.golang.org/grpc v1.58.3
+	google.golang.org/genproto/googleapis/rpc v0.0.0-20230822172742-b8732ec3820d
+	google.golang.org/grpc v1.59.0


Is it necessary to bump otel deps for this feature? We tend to infer these updates through containerd, see #4318 (review)

We don't need to. I'm attempting to get this working with v1.19.0 since that was the first version where the metrics were considered stable. So that should be fine.

jsternberg · 2023-12-08T16:01:01Z

I have a PR almost ready for some cache metrics, but I also found that opentelemetry has grpc metrics built in so I included them in this PR so this PR actually uses the metrics.

crazy-max

Just some questions but overall LGTM

crazy-max · 2023-12-08T16:45:07Z

 		bklog.G(req.Context()).Debugf("triggered GC from debug endpoint")
 	}))

+	m.Handle("/metrics", promhttp.Handler())


I was wondering myself why not /debug/metrics as route but this is part of the debug handler so good to me.

It's mostly because /metrics is the typical endpoint for prometheus. I don't think it matters though so I can always move it to /debug/metrics.

The main reason why it's in the debug handler is because buildkit doesn't expose an HTTP server otherwise. At the same time, this pathway is only relevant for buildkit when it's run in isolation. If buildkit is embedded, such as moby, we'll piggyback on their own /metrics endpoint and be included there.

crazy-max · 2023-12-08T16:47:51Z

+		// Register the prometheus reader if there was no error.
+		// Don't fail if there was an error.
+		// TODO(jsternberg): Maybe log the error here?


Yes I think we should log the error using github.com/moby/buildkit/util/bklog

Forgot that I put this todo there. Good catch. I'll log the error as a warning.

crazy-max · 2023-12-08T16:52:40Z


-	sdktp := sdktrace.NewTracerProvider(sdktrace.WithSpanProcessor(sp), sdktrace.WithResource(res))
-	closers = append(closers, sdktp.Shutdown)
+	if r, err := prometheus.New(); err == nil {


IIUC prometheus is a pull metrics exporter right? If we fail to register it and a scraper tries to pull, what happens?

I believe it'll just include the defaults for prometheus. It just won't include any of our own metrics.

crazy-max · 2023-12-08T16:55:03Z

 }
+
+func otlpMetricExporter() (sdkmetric.Exporter, error) {
+	set := os.Getenv("OTEL_METRICS_EXPORTER") == "otlp" || os.Getenv("OTEL_EXPORTER_OTLP_ENDPOINT") != "" || os.Getenv("OTEL_EXPORTER_OTLP_METRICS_ENDPOINT") != ""


Not for this PR but I think we should start writing a dedicated "observability" docs? Atm we just have

buildkit/README.md

Lines 765 to 777 in c2baaa9

## OpenTelemetry support

BuildKit supports [OpenTelemetry](https://opentelemetry.io/) for buildkitd gRPC

API and buildctl commands. To capture the trace to

[Jaeger](https://github.com/jaegertracing/jaeger), set `JAEGER_TRACE`

environment variable to the collection address.

```bash

docker run -d -p6831:6831/udp -p16686:16686 jaegertracing/all-in-one:latest

export JAEGER_TRACE=0.0.0.0:6831

# restart buildkitd and buildctl so they know JAEGER_TRACE

# any buildctl command should be traced to http://127.0.0.1:16686/

```

And https://docs.docker.com/build/building/opentelemetry/

Yea I think that would probably help.

tonistiigi

overall SGTM

This exposes the otel metrics through the environment variables for the otel collector or through the prometheus `/metrics` endpoint through the debug address. This also adds metrics for the grpc endpoints from the opentelemetry libraries. For the opentelemetry collector, the metric temporality is delta which reduces the amount of data that goes over the wire overall. Prometheus metrics are cumulative because prometheus metrics require that. Signed-off-by: Jonathan A. Sternberg <jonathan.sternberg@docker.com>

moby/buildkit#4460 tries and fails to send metrics if OTEL_METRICS_EXPORTER is 'none' Signed-off-by: Tom Plant <21111317+pl4nty@users.noreply.github.com>

jsternberg force-pushed the otel-metrics branch 3 times, most recently from f0f15ef to c233e37 Compare December 5, 2023 18:39

crazy-max reviewed Dec 5, 2023

View reviewed changes

jsternberg force-pushed the otel-metrics branch 2 times, most recently from 8586a33 to a54d044 Compare December 5, 2023 19:33

jsternberg marked this pull request as ready for review December 5, 2023 20:13

jsternberg force-pushed the otel-metrics branch 2 times, most recently from f025d9c to c52719b Compare December 8, 2023 15:59

jsternberg requested review from crazy-max and tonistiigi December 8, 2023 16:01

crazy-max reviewed Dec 8, 2023

View reviewed changes

jsternberg force-pushed the otel-metrics branch from c52719b to 378b588 Compare December 8, 2023 17:12

jsternberg requested a review from crazy-max December 8, 2023 17:12

crazy-max approved these changes Dec 8, 2023

View reviewed changes

tonistiigi reviewed Dec 12, 2023

View reviewed changes

Comment thread util/tracing/detect/otlp.go Outdated

jsternberg force-pushed the otel-metrics branch from 378b588 to 7de2e4f Compare December 12, 2023 14:59

jsternberg requested a review from tonistiigi December 12, 2023 14:59

tonistiigi approved these changes Dec 12, 2023

View reviewed changes

tonistiigi merged commit f84cfe3 into moby:master Dec 12, 2023

jsternberg deleted the otel-metrics branch December 12, 2023 22:07

crazy-max mentioned this pull request Dec 19, 2023

deps: update buildkit, vendor changes docker/buildx#2170

Merged

pl4nty added a commit to pl4nty/homelab that referenced this pull request Dec 24, 2023

fix: pin buildkitd to v0.12.4

d6dc981

moby/buildkit#4460 tries and fails to send metrics if OTEL_METRICS_EXPORTER is 'none' Signed-off-by: Tom Plant <21111317+pl4nty@users.noreply.github.com>

	## OpenTelemetry support

	BuildKit supports [OpenTelemetry](https://opentelemetry.io/) for buildkitd gRPC
	API and buildctl commands. To capture the trace to
	[Jaeger](https://github.com/jaegertracing/jaeger), set `JAEGER_TRACE`
	environment variable to the collection address.

	```bash
	docker run -d -p6831:6831/udp -p16686:16686 jaegertracing/all-in-one:latest
	export JAEGER_TRACE=0.0.0.0:6831
	# restart buildkitd and buildctl so they know JAEGER_TRACE
	# any buildctl command should be traced to http://127.0.0.1:16686/
	```

Conversation

jsternberg commented Dec 4, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jsternberg commented Dec 8, 2023

Uh oh!

crazy-max left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tonistiigi left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

jsternberg commented Dec 4, 2023 •

edited

Loading