Allow customization of health endpoints in ochttp server
tjcelaya opened this issue · 0 comments
This is part feature request and part bug. The automatic tracing includes a check where both the ochttp client and server components inspect the URL path before attaching a trace to the request being made. Unfortunately those paths to skip tracing are hardcoded:
opencensus-go/plugin/ochttp/trace.go
Lines 231 to 238 in 6531013
called from the server here:
opencensus-go/plugin/ochttp/server.go
Lines 89 to 91 in 6531013
Is your feature request related to a problem? Please describe.
The /health
and /metrics
endpoints in our server end up being sampled alongside service endpoints and are generating excessive traces.
Describe the solution you'd like
Enhance ochttp.Handler
to allow callers to supply a set of paths or path pattern to use for unconditionally skipping sampling. This could either be something like a HealthEndpoints []string
field for directly specifying paths, a HealthEndpointPattern *regex.Regex
for use with Match
/MatchString
. The default paths currently in use could be used as the default to provide backwards compatibility.
While a similar enhancement could also be introduced for the client this issue is focused on improving the server-side sampling logic.
Describe alternatives you've considered
Exposing health/metrics/etc endpoints through a different http.Server
instance is possible and could be used to bypass ochttp entirely but would have to listen on a different port and introduce the possibility of unexpected or divergent behavior from the state of the service's primary http.Server
(e.g. during server Shutdown or config reload).
Additional context
I could find time to work on a PR for this soon but would greatly appreciate if a maintainer could weigh in with a preferred approach and a general suggestion for what kinds of unit tests are expected. Perhaps a new unit test is desired or it might be sufficient to adds a few case to this test (or both):
opencensus-go/plugin/ochttp/trace_test.go
Lines 204 to 210 in 3da91ae