search

fleetdm / fleet

Open-source platform for IT, security, and infrastructure teams. (Linux, macOS, Chrome, Windows, cloud, data center)
https://fleetdm.com
Other
2.69k stars 383 forks source link

Some endpoints are not instrumented in the configured tracing tool (Elastic APM or OpenTelemetry) #19696

Open mna opened 1 month ago

mna commented 1 month ago

Fleet version: <!-- Copy this from the "My account" page in the Fleet UI, or run fleetctl --version --> 4.51.0

Web browser and operating system: N/A

💥  Actual behavior

APM (or opentelemetry) is setup for API routes here: https://github.com/fleetdm/fleet/blob/9867991c2f5328da882d5652569a1f862f91871a/server/service/handler.go#L140-L146

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

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:

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 commented 1 month ago

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

mna commented 4 weeks ago

@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 commented 3 weeks ago

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

sharon-fdm commented 3 weeks ago

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