WIP: instrument net/http.ServeHTTP implementations

[nsrip-dd]

What does this PR do?

TODO - tests

Makes sure we instrument net/http handlers and servers.

Background

There are two general ways to start an HTTP server with net/http:

• Package-level functions:
  • http.ListenAndServe
  • http.ListenAndServeTLS
  • http.Serve
  • http.ServeTLS
• Via the net/http.Server methods:
  • http.(*Server).ListenAndServe
  • http.(*Server).ListenAndServeTLS
  • http.(*Server).Serve
  • http.(*Server).ServeTLS

The package-level functions create a Server and dispatch to the matching methods.

All of these work with the http.Handler interface, which defines the "application" logic for handling an HTTP request. The Server type has a Handler field, and all of the package-level functions take an http.Handler argument. Many packages, like github.com/gorilla/mux, provide http.Handler implementations.

The net/http package has two http.Handler implementations: net/http.ServeMux, and net/http.HandlerFunc.

What this PR actually does

Before, we instrumented the Handler field of Server literals, and function literals cast to http.HandlerFunc. But this could lead to redundant instrumentation, and also didn't cover common patterns. For example, this doesn't get instrumented today:

package main

import "net/http"

type Handler struct{}

func (Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) {}

func main() {
	mu := http.NewServeMux()
	mu.Handle("/foo", Handler{})
	http.ListenAndServe(":8080", mu)
}

For another example, we get redundant instrumentation for this github.com/gorilla/mux example:

mux := mux.NewRouter()
tc.Server = &http.Server{
	Addr:    "127.0.0.1:" + utils.GetFreePort(t),
	Handler: mux,
}

mux.HandleFunc("/ping", func(w http.ResponseWriter, _ *http.Request) {
	w.Header().Set("Content-Type", "application/json")
	w.WriteHeader(http.StatusOK)
	_, err := io.WriteString(w, `{"message": "pong"}`)
	assert.NoError(t, err)
}).Methods("GET")

The Handler field of the Server literal gets instrumented, and the http.HandlerFunc literal is instrumented, and the router returned via mux.NewRouter is instrumented. We have at least one too many layers of instrumentation.

So this PR takes this approach:

• We define a new MaybeWrapHandler function. It takes an http.Handler and returns an http.Handler. If the input handler is one of the net/http handler types, we wrap it with tracing instrumentation. Otherwise we pass it on unchanged under the assumption it is already instrumented.
• We use this function to maybe wrap the http.Handler passed to the package-level functions.
• We also use this function to maybe wrap the Handler field of a Server literal, similar to what we did before.

This doesn't provide complete coverage. For example, the user could do something like s := new(http.Server); s.Handler = foo and we wouldn't catch it. But this hopefully improves our coverage significantly and avoids some redundant instrumentation.

Motivation

Fixes #173

Reviewer's Checklist

• Changed code has unit tests for its functionality.

[nsrip-dd]

Closing in favor of #403

[codecov]

Codecov Report

Attention: Patch coverage is 0% with 8 lines in your changes missing coverage. Please review.

Project coverage is 56.48%. Comparing base (0acfd19) to head (71230cd).
Report is 51 commits behind head on main.

Files with missing lines	Patch %	Lines
instrument/http.go	0.00%	8 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #175      +/-   ##
==========================================
- Coverage   58.16%   56.48%   -1.68%     
==========================================
  Files         151      105      -46     
  Lines       10976     5051    -5925     
==========================================
- Hits         6384     2853    -3531     
+ Misses       4124     1873    -2251     
+ Partials      468      325     -143     

Components	Coverage Δ
Generators	78.38% <ø> (+1.40%)	⬆️
Instruments	61.33% <0.00%> (-26.73%)	⬇️
Go Driver	48.55% <ø> (-24.26%)	⬇️
Toolexec Driver	42.84% <ø> (-28.05%)	⬇️
Aspects	71.34% <ø> (+0.49%)	⬆️
Injector	72.71% <ø> (-0.44%)	⬇️
Job Server	54.18% <ø> (-9.85%)	⬇️
Integration Test Suite	38.68% <ø> (-9.81%)	⬇️
Other	56.48% <0.00%> (-1.68%)	⬇️

Files with missing lines	Coverage Δ
instrument/http.go	0.00% <0.00%> (-100.00%)	⬇️

... and 136 files with indirect coverage changes

---- 🚨 Try these New Features:

• Flaky Tests Detection - Detect and resolve failed and flaky tests