Some endpoints are not instrumented in the configured tracing tool (OpenTelemetry)

[mna]

Fleet version:
4.51.0

Web browser and operating system:
N/A

---

Update (2024/11/18): The APM part of this issue is covered in #23902

💥  Actual behavior

APM (or opentelemetry) is setup for API routes here:

fleet/server/service/handler.go

Lines 140 to 146 in 9867991

	if config.Logging.TracingEnabled {
	if config.Logging.TracingType == "opentelemetry" {
	r.Use(otmiddleware.Middleware("fleet"))
	} else {
	apmgorilla.Instrument(r)
	}
	}

However, we have a number of routes that are handled outside of that Gorilla Mux:

• health check, version and assets:

  fleet/cmd/fleet/serve.go

  Lines 967 to 970 in 9867991

  	rootMux := http.NewServeMux()
  	rootMux.Handle("/healthz", service.PrometheusMetricsHandler("healthz", health.Handler(httpLogger, healthCheckers)))
  	rootMux.Handle("/version", service.PrometheusMetricsHandler("version", version.Handler()))
  	rootMux.Handle("/assets/", service.PrometheusMetricsHandler("static_assets", service.ServeStaticAssets("/assets/")))

• Prometheus metrics endpoint:

  fleet/cmd/fleet/serve.go

  Lines 1015 to 1028 in 9867991

  	if config.Prometheus.BasicAuth.Username != "" && config.Prometheus.BasicAuth.Password != "" {
  	rootMux.Handle("/metrics", basicAuthHandler(
  	config.Prometheus.BasicAuth.Username,
  	config.Prometheus.BasicAuth.Password,
  	service.PrometheusMetricsHandler("metrics", promhttp.Handler()),
  	))
  	} else {
  	if config.Prometheus.BasicAuth.Disable {
  	level.Info(logger).Log("msg", "metrics endpoint enabled with http basic auth disabled")
  	rootMux.Handle("/metrics", service.PrometheusMetricsHandler("metrics", promhttp.Handler()))
  	} else {
  	level.Info(logger).Log("msg", "metrics endpoint disabled (http basic auth credentials not set)")
  	}
  	}

• The frontend routes:

  fleet/cmd/fleet/serve.go

  Line 1066 in 9867991

  rootMux.Handle("/", frontendHandler)

• The debug routes:

  fleet/cmd/fleet/serve.go

  Line 1071 in 9867991

  rootMux.Handle("/debug/", debugHandler)

• The Apple MDM SCEP routes:

  fleet/server/service/handler.go

  Line 1085 in 9867991

  mux.Handle(apple_mdm.SCEPPath, scepHandler)

• The Apple MDM protocol routes:

  fleet/server/service/handler.go

  Line 1145 in 9867991

  mux.Handle(apple_mdm.MDMPath, mdmHandler)

This ticket is to determine which of those (all? all but frontend? just the Apple MDM ones?) should be instrumented, and to add that instrumentation accordingly (same-ish way it is done for the API routes at the moment).

🕯️ More info (optional)

It looks like both Elastic APM and OpenTelemetry have a package to wrap a standard net/http handler, similar to the one they provide to wrap the gorilla mux used in our API routes, so those could probably be used:

• Elastic APM: https://pkg.go.dev/go.elastic.co/apm/module/apmhttp/v2#Wrap
• OpenTelemetry: https://pkg.go.dev/go.opentelemetry.io/contrib/instrumentation/net/http#NewHandler

We should take the time to check that they are setup in a consistent way vs the Gorilla one. Elastic APM can be tested locally (see https://github.com/fleetdm/fleet/tree/main/tools/apm-elastic).

[sharon-fdm]

@mna can you please provide some incentives why it's a bug (not improvement) ?
cc: @lukeheath to prioritize if an improvement.

[mna]

@sharon-fdm @lukeheath Good question, I originally labeled it a bug as it looks like we wanted to instrument all routes and missed a few, but I could see it as an improvement too (we instrumented some endpoints, and now we want to extend that instrumentation).

I think for Fleet users that enable instrumentation, it would be surprising to only have a subset of endpoints show up in their dashboard - which makes it more a bug. But on the other hand I don't think we document much how to enable instrumentation (whether it's the open telemetry one or the Elastic APM one, I can't find any mention of how to enable them in https://fleetdm.com/docs/configuration/fleet-server-configuration).

[sharon-fdm]

@mna, OK. Will treat as a bug and will estimate next estimation session.

[sharon-fdm]

We estimate as 3 and think it's a feature that needs prioritization.
@mna Since this is more of a story, I'd like to see the user story (what's the pain/benefit?)
cc: @lukeheath