census-instrumentation/opencensus-go

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:

func isHealthEndpoint(path string) bool {
// Health checking is pretty frequent and
// traces collected for health endpoints
// can be extremely noisy and expensive.
// Disable canonical health checking endpoints
// like /healthz and /_ah/health for now.
if path == "/healthz" || path == "/_ah/health" {
return true

called from the server here:

func (h *Handler) startTrace(w http.ResponseWriter, r *http.Request) (*http.Request, func()) {
if isHealthEndpoint(r.URL.Path) {
return r, func() {}

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):

func TestEndToEnd(t *testing.T) {
tc := []struct {
name string
handler *Handler
transport *Transport
wantSameTraceID bool
wantLinks bool // expect a link between client and server span