fix(contrib/net/http): when Wrapping use the WrappedHandler

[jacobmikesell]

What does this PR do?

Uses a previously unused type to wrap the httphandler. On the surface this is a no-op, but in practice the enables targeted changes to http tracing that orchestrion does.

Motivation

We've been using orchestrion (to great effect) but had noticed a memory leak due to tracing in one of our services. This service exposed both rest and grpc using https://pkg.go.dev/golang.org/x/net/http2/h2c. Orchestrion happily wraps the serve mux, but under the hood h2c hijacks the incoming connection for grpc calls and fails to "complete" the parent trace. This ends up leading to a "forever parent" that accumulates references to children until the app OOM's

Orchestrion does not support overriding the behavior but it does skip wrapping if the type it sees is already one of the wrapped types, which means we can manually instrument just the part of the h2c server thats problematic to handle this case!

Open to other ideas to solve this, but the type was there and already in the orchestrion logic so this seemed like a reasonable line!

sample configuration (in case you want to reproduce)

	// HTTP handler: log request details and return 200
	httpHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
		body, _ := io.ReadAll(r.Body)
		log.Printf("%s %s host=%s remote=%s body=%q", r.Method, r.URL.String(), r.Host, r.RemoteAddr, string(body))
		w.WriteHeader(http.StatusOK)
		_, _ = w.Write([]byte("ok\n"))
	})
         // create a minimal grpc server
	grpcServer := grpc.NewServer()
	healthpb.RegisterHealthServer(grpcServer, health.NewServer())
	reflection.Register(grpcServer)

        dualHandler := func(w http.ResponseWriter, r *http.Request) {
	if r.ProtoMajor == 2 && strings.Contains(r.Header.Get("Content-Type"), "application/grpc") {
		grpcServer.ServeHTTP(w, r)
	} else {
		httpHandler.ServeHTTP(w, r)
	}
h := http.HandlerFunc(dualHandler)
	h2s := &http2.Server{}
	h2cHandler := h2c.NewHandler(h, h2s)
	httpsvr := http.Server{Handler: h2cHandler}
}

	if err := http2.ConfigureServer(&httpsvr, h2s); err != nil {
		return fmt.Errorf("error configuring http2: %v", err)
	}
	lis, err := net.Listen("tcp", netutil.ListenAddrFromEnv())
	if err != nil {
		return fmt.Errorf("listen error: %v", err)
	}
if err := httpsvr.Serve(lis); err != nil && !errors.Is(err, http.ErrServerClosed) {
			return err
		}

after!

	// HTTP handler: log request details and return 200
	httpHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
		body, _ := io.ReadAll(r.Body)
		log.Printf("%s %s host=%s remote=%s body=%q", r.Method, r.URL.String(), r.Host, r.RemoteAddr, string(body))
		w.WriteHeader(http.StatusOK)
		_, _ = w.Write([]byte("ok\n"))
	})
         // create a minimal grpc server
	grpcServer := grpc.NewServer()
	healthpb.RegisterHealthServer(grpcServer, health.NewServer())
	reflection.Register(grpcServer)

        dualHandler := func(w http.ResponseWriter, r *http.Request) {
	if r.ProtoMajor == 2 && strings.Contains(r.Header.Get("Content-Type"), "application/grpc") {
		grpcServer.ServeHTTP(w, r)
	} else {
		httpHandler.ServeHTTP(w, r)
	}
h := http.HandlerFunc(dualHandler)
	h2s := &http2.Server{}
	h2cHandler := h2c.NewHandler(h, h2s)
	httpsvr := http.Server{Handler: http_trace.WrapHandler(h2cHandler, "", "/", http_trace.WithIgnoreRequest(func(request *http.Request) bool {
		return true
	}))}
}

	if err := http2.ConfigureServer(&httpsvr, h2s); err != nil {
		return fmt.Errorf("error configuring http2: %v", err)
	}
	lis, err := net.Listen("tcp", netutil.ListenAddrFromEnv())
	if err != nil {
		return fmt.Errorf("listen error: %v", err)
	}
if err := httpsvr.Serve(lis); err != nil && !errors.Is(err, http.ErrServerClosed) {
			return err
		}

Reviewer's Checklist

• Changed code has unit tests for its functionality at or near 100% coverage.
• System-Tests covering this feature have been added and enabled with the va.b.c-dev version tag.
• There is a benchmark for any new code, or changes to existing code.
• If this interacts with the agent in a new way, a system test has been added.
• New code is free of linting errors. You can check this by running ./scripts/lint.sh locally.
• Add an appropriate team label so this PR gets put in the right place for the release notes.
• Non-trivial go.mod changes, e.g. adding new modules, are reviewed by @DataDog/dd-trace-go-guild.

Unsure? Have a question? Request a review!

[darccio]

Thanks @jacobmikesell for your contribution. Let me ping the original implementer - @RomainMuller - to double check. I think that the purpose of this was exactly what you are doing to ensure that it isn't double instrumented nor left hanging.