net/http: Handler interface implementation is not always instrumented

[RomainMuller]

Version of orchestrion
0.7.x

Describe what happened:

The implementation of the ServerHTTP method does not get instrumented, because its aspect is gated by httpmode=report, which cannot be enabled externally.

// You can edit this code!
// Click here and start typing.
package main

import "net/http"

type Handler struct{}

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

func main() {
	// NB: http.Handle(...), using the default ServeMux, doesn't get instrumented either
	mu := http.NewServeMux()
	mu.Handle("/foo", Handler{})
	http.ListenAndServe(":8080", mu)
}

[RomainMuller]

The httpmode configuration is legacy and is supposed to be going away.

I think what we need to do is:

1. Remove all references to the httpmode configuration
2. Instrument ServeHTTP(http.ResponseWriter, *http.Request) methods using the report machinery
3. Since we'd then instrument http.HandlerFunc as well as http.Handler implementations, it likely becomes redundant to wrap values set on the Handler field of http.Server struct literals; and that aspect probably should be removed.

[nsrip-dd]

@RomainMuller to check in on this, here is what I have tried so far:

• Wrap http.HandlerFunc "calls" (really, type casts)
• Instrument ServeHTTP(http.ResponseWriter, *http.Request) implementations

And to your suggestions I added one more rule, since these things weren't matched by the above rules:

• Wrap the second parameter to http.HandleFunc(path, f) and http.(*ServeMux).HandleFunc(path, f), like done for http.HandlerFunc

My thinking was that it'd be better to make the rules more targeted, as opposed to doing something like instrumenting any function with the signature func(http.ResponseWriter, *http.Request). But, this is falling short because the "function-call" rule does not properly match http.(*ServeMux).HandleFunc calls. Basically, if you have something like this:

mux := http.NewServeMux()
mux.HandleFunc("/", ...)

then the mux.HandleFunc is a SelectorExpr, where the Sel.Path value is now blank since mux is a "local" identifier. I don't currently see a way to tell that we're matching a http.(*ServeMux).HandleFunc call.

How should I approach this?

[RomainMuller]

It sounds like you're trying to do call-site instrumentation of the HandleFunc method, but this is indeed going to be quite complicated to achieve with good selectivity (and the type-asserted chain is an interesting edge case I had never thought about).

I guess if you do callee instrumentation, things become a lot simpler... The main difference being in how //dd:ignore would work there:

• call-site instrumentation allows opting a specific call out of instrumentation
• callee instrumentation allows opting an entire implementation out of instrumentation

We can probably build round-about ways to achieve both declaratively; but this would be a feature "for later" (read: once someone actually has a compelling real-life use-case for this).