Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: support auto-instrumentation when net/http.DefaultServeMux is used implicitly #440

Merged
merged 1 commit into from
Dec 4, 2024

Conversation

keisku
Copy link
Contributor

@keisku keisku commented Dec 2, 2024

Motivation

orchestrion doesn't do auto-instrumentation for this code because handler in http.ListenAndServe() is set to nil. Setting nil in http.ListenAndServe() is generally a common practice.

package main

import (
	"context"
	"log/slog"
	"net/http"
	"os"
	"os/signal"
)

func main() {
	ctx, cancel := signal.NotifyContext(context.Background(), os.Interrupt)
	defer cancel()
	http.HandleFunc("/hello", func(w http.ResponseWriter, r *http.Request) {
		slog.InfoContext(r.Context(), "Request received at /hello")
		resp, err := http.Get("https://example.com")
		if err != nil {
			http.Error(w, err.Error(), http.StatusInternalServerError)
			return
		}
		defer resp.Body.Close()
		slog.InfoContext(r.Context(), "Response received from example.com", slog.String("status", resp.Status))
		w.WriteHeader(http.StatusOK)
	})
	slog.Info("Starting server on port 8080")
	go http.ListenAndServe(":8080", nil)
	<-ctx.Done()
	slog.Info("Server stopped")
}

Workaround

Set http.DefaultServeMux to http.Server.Handler explicitly.

+	s := http.Server{
+		Addr:    ":8080",
+		Handler: http.DefaultServeMux,
+	}
+	go s.ListenAndServe()
-	go http.ListenAndServe(":8080", nil)

Generated code

srv.Handler is nil, when we set nil in http.ListenAndServe().

func (srv *Server) Serve(l net.Listener) error {
	//line <generated>
	{
		if srv.Handler == nil {
			srv.Handler = __dd_orchestrion_instrument_WrapHandler(DefaultServeMux)
		} else {
			srv.Handler = __dd_orchestrion_instrument_WrapHandler(srv.Handler)
		}
	}
	//line /snap/go/10743/src/net/http/server.go:3301
	if fn := testHookServerServe; fn != nil {
		fn(srv, l) // call hook with unwrapped listener
	}
	// ...skip....

How I checked generated code? Use fmt.Println(buf.String()) in /internal/injector/write.go

Replace github.com/DataDog/orchestrion with the project in my local.

module orchestrion-poc

go 1.23.3

require github.com/DataDog/orchestrion v1.0.1

replace github.com/DataDog/orchestrion => /home/ubuntu/workspace/orchestrion
....skip...

@keisku keisku marked this pull request as ready for review December 3, 2024 01:23
@keisku keisku requested a review from a team as a code owner December 3, 2024 01:23
@keisku keisku changed the title feat: support net/http.DefaultServeMux feat: support auto-instrumentation when net/http.DefaultServeMux is used implicitly Dec 3, 2024
@RomainMuller RomainMuller added this pull request to the merge queue Dec 3, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Dec 3, 2024
@RomainMuller RomainMuller added this pull request to the merge queue Dec 4, 2024
Merged via the queue into DataDog:main with commit cd22636 Dec 4, 2024
24 checks passed
@keisku keisku deleted the DefaultServeMux branch December 5, 2024 15:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants