From b2d441b0474713f75f55aff0294bd2f2bb1929a4 Mon Sep 17 00:00:00 2001 From: JamieDanielson Date: Wed, 19 Apr 2023 15:25:03 -0400 Subject: [PATCH 01/22] adjust output for e2e test to remove empty span --- test/e2e/nethttp/traces.json | 21 --------------------- 1 file changed, 21 deletions(-) diff --git a/test/e2e/nethttp/traces.json b/test/e2e/nethttp/traces.json index ad20ef277..7d3508eb6 100644 --- a/test/e2e/nethttp/traces.json +++ b/test/e2e/nethttp/traces.json @@ -44,27 +44,6 @@ "spanId": "xxxxx", "status": {}, "traceId": "xxxxx" - }, - { - "attributes": [ - { - "key": "http.method", - "value": { - "stringValue": "" - } - }, - { - "key": "http.target", - "value": { - "stringValue": "" - } - } - ], - "kind": 2, - "parentSpanId": "", - "spanId": "", - "status": {}, - "traceId": "" } ] } From c1bc4d0d8be3666227389bdeeb7ed79b092c3bd9 Mon Sep 17 00:00:00 2001 From: JamieDanielson Date: Wed, 19 Apr 2023 16:01:08 -0400 Subject: [PATCH 02/22] only use one http function in http server instr --- .../bpf/net/http/server/probe.go | 50 +++++++------------ 1 file changed, 19 insertions(+), 31 deletions(-) diff --git a/pkg/instrumentors/bpf/net/http/server/probe.go b/pkg/instrumentors/bpf/net/http/server/probe.go index f72b2e9f6..467483caf 100644 --- a/pkg/instrumentors/bpf/net/http/server/probe.go +++ b/pkg/instrumentors/bpf/net/http/server/probe.go @@ -47,7 +47,7 @@ type HttpEvent struct { type httpServerInstrumentor struct { bpfObjects *bpfObjects - uprobes []link.Link + uprobe link.Link returnProbs []link.Link eventsReader *perf.Reader } @@ -61,7 +61,7 @@ func (h *httpServerInstrumentor) LibraryName() string { } func (h *httpServerInstrumentor) FuncNames() []string { - return []string{"net/http.(*ServeMux).ServeHTTP", "net/http.HandlerFunc.ServeHTTP"} + return []string{"net/http.HandlerFunc.ServeHTTP"} } func (h *httpServerInstrumentor) Load(ctx *context.InstrumentorContext) error { @@ -97,53 +97,41 @@ func (h *httpServerInstrumentor) Load(ctx *context.InstrumentorContext) error { return err } - for _, funcName := range h.FuncNames() { - h.registerProbes(ctx, funcName) - } + offset, err := ctx.TargetDetails.GetFunctionOffset(h.FuncNames()[0]) - rd, err := perf.NewReader(h.bpfObjects.Events, os.Getpagesize()) if err != nil { return err } - h.eventsReader = rd - - return nil -} - -func (h *httpServerInstrumentor) registerProbes(ctx *context.InstrumentorContext, funcName string) { - logger := log.Logger.WithName("net/http-instrumentor").WithValues("function", funcName) - offset, err := ctx.TargetDetails.GetFunctionOffset(funcName) - if err != nil { - logger.Error(err, "could not find function start offset. Skipping") - return - } - retOffsets, err := ctx.TargetDetails.GetFunctionReturns(funcName) - if err != nil { - logger.Error(err, "could not find function end offsets. Skipping") - return - } up, err := ctx.Executable.Uprobe("", h.bpfObjects.UprobeServerMuxServeHTTP, &link.UprobeOptions{ Address: offset, }) if err != nil { - logger.V(1).Info("could not insert start uprobe. Skipping", - "error", err.Error()) - return + return err } - h.uprobes = append(h.uprobes, up) + h.uprobe = up + retOffsets, err := ctx.TargetDetails.GetFunctionReturns(h.FuncNames()[0]) + if err != nil { + return err + } for _, ret := range retOffsets { retProbe, err := ctx.Executable.Uprobe("", h.bpfObjects.UprobeServerMuxServeHTTP_Returns, &link.UprobeOptions{ Address: ret, }) if err != nil { - logger.Error(err, "could not insert return uprobe. Skipping") - return + return err } h.returnProbs = append(h.returnProbs, retProbe) } + + rd, err := perf.NewReader(h.bpfObjects.Events, os.Getpagesize()) + if err != nil { + return err + } + h.eventsReader = rd + return nil } func (h *httpServerInstrumentor) Run(eventsChan chan<- *events.Event) { @@ -203,8 +191,8 @@ func (h *httpServerInstrumentor) Close() { h.eventsReader.Close() } - for _, r := range h.uprobes { - r.Close() + if h.uprobe != nil { + h.uprobe.Close() } for _, r := range h.returnProbs { From 63ec620fd418a6b18fab60a7087bf60975f9ff58 Mon Sep 17 00:00:00 2001 From: JamieDanielson Date: Wed, 19 Apr 2023 16:47:48 -0400 Subject: [PATCH 03/22] Revert "only use one http function in http server instr" This reverts commit c1bc4d0d8be3666227389bdeeb7ed79b092c3bd9. --- .../bpf/net/http/server/probe.go | 50 ++++++++++++------- 1 file changed, 31 insertions(+), 19 deletions(-) diff --git a/pkg/instrumentors/bpf/net/http/server/probe.go b/pkg/instrumentors/bpf/net/http/server/probe.go index 467483caf..f72b2e9f6 100644 --- a/pkg/instrumentors/bpf/net/http/server/probe.go +++ b/pkg/instrumentors/bpf/net/http/server/probe.go @@ -47,7 +47,7 @@ type HttpEvent struct { type httpServerInstrumentor struct { bpfObjects *bpfObjects - uprobe link.Link + uprobes []link.Link returnProbs []link.Link eventsReader *perf.Reader } @@ -61,7 +61,7 @@ func (h *httpServerInstrumentor) LibraryName() string { } func (h *httpServerInstrumentor) FuncNames() []string { - return []string{"net/http.HandlerFunc.ServeHTTP"} + return []string{"net/http.(*ServeMux).ServeHTTP", "net/http.HandlerFunc.ServeHTTP"} } func (h *httpServerInstrumentor) Load(ctx *context.InstrumentorContext) error { @@ -97,41 +97,53 @@ func (h *httpServerInstrumentor) Load(ctx *context.InstrumentorContext) error { return err } - offset, err := ctx.TargetDetails.GetFunctionOffset(h.FuncNames()[0]) + for _, funcName := range h.FuncNames() { + h.registerProbes(ctx, funcName) + } + rd, err := perf.NewReader(h.bpfObjects.Events, os.Getpagesize()) if err != nil { return err } + h.eventsReader = rd + + return nil +} + +func (h *httpServerInstrumentor) registerProbes(ctx *context.InstrumentorContext, funcName string) { + logger := log.Logger.WithName("net/http-instrumentor").WithValues("function", funcName) + offset, err := ctx.TargetDetails.GetFunctionOffset(funcName) + if err != nil { + logger.Error(err, "could not find function start offset. Skipping") + return + } + retOffsets, err := ctx.TargetDetails.GetFunctionReturns(funcName) + if err != nil { + logger.Error(err, "could not find function end offsets. Skipping") + return + } up, err := ctx.Executable.Uprobe("", h.bpfObjects.UprobeServerMuxServeHTTP, &link.UprobeOptions{ Address: offset, }) if err != nil { - return err + logger.V(1).Info("could not insert start uprobe. Skipping", + "error", err.Error()) + return } - h.uprobe = up - retOffsets, err := ctx.TargetDetails.GetFunctionReturns(h.FuncNames()[0]) - if err != nil { - return err - } + h.uprobes = append(h.uprobes, up) for _, ret := range retOffsets { retProbe, err := ctx.Executable.Uprobe("", h.bpfObjects.UprobeServerMuxServeHTTP_Returns, &link.UprobeOptions{ Address: ret, }) if err != nil { - return err + logger.Error(err, "could not insert return uprobe. Skipping") + return } h.returnProbs = append(h.returnProbs, retProbe) } - - rd, err := perf.NewReader(h.bpfObjects.Events, os.Getpagesize()) - if err != nil { - return err - } - h.eventsReader = rd - return nil } func (h *httpServerInstrumentor) Run(eventsChan chan<- *events.Event) { @@ -191,8 +203,8 @@ func (h *httpServerInstrumentor) Close() { h.eventsReader.Close() } - if h.uprobe != nil { - h.uprobe.Close() + for _, r := range h.uprobes { + r.Close() } for _, r := range h.returnProbs { From 47a47f25bcf5b223ad2afa57b2a4eedc91062560 Mon Sep 17 00:00:00 2001 From: JamieDanielson Date: Wed, 19 Apr 2023 16:49:16 -0400 Subject: [PATCH 04/22] only use one http function - keep refactor --- pkg/instrumentors/bpf/net/http/server/probe.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/instrumentors/bpf/net/http/server/probe.go b/pkg/instrumentors/bpf/net/http/server/probe.go index f72b2e9f6..3153297ad 100644 --- a/pkg/instrumentors/bpf/net/http/server/probe.go +++ b/pkg/instrumentors/bpf/net/http/server/probe.go @@ -61,7 +61,7 @@ func (h *httpServerInstrumentor) LibraryName() string { } func (h *httpServerInstrumentor) FuncNames() []string { - return []string{"net/http.(*ServeMux).ServeHTTP", "net/http.HandlerFunc.ServeHTTP"} + return []string{"net/http.HandlerFunc.ServeHTTP"} } func (h *httpServerInstrumentor) Load(ctx *context.InstrumentorContext) error { From f7605a2ca79e178682434557ab98bc3cb7ada1b5 Mon Sep 17 00:00:00 2001 From: JamieDanielson Date: Thu, 20 Apr 2023 10:48:07 -0400 Subject: [PATCH 05/22] keep both func names but only register 1 probe --- pkg/instrumentors/bpf/net/http/server/probe.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/instrumentors/bpf/net/http/server/probe.go b/pkg/instrumentors/bpf/net/http/server/probe.go index 3153297ad..f012dba65 100644 --- a/pkg/instrumentors/bpf/net/http/server/probe.go +++ b/pkg/instrumentors/bpf/net/http/server/probe.go @@ -61,7 +61,7 @@ func (h *httpServerInstrumentor) LibraryName() string { } func (h *httpServerInstrumentor) FuncNames() []string { - return []string{"net/http.HandlerFunc.ServeHTTP"} + return []string{"net/http.(*ServeMux).ServeHTTP","net/http.HandlerFunc.ServeHTTP"} } func (h *httpServerInstrumentor) Load(ctx *context.InstrumentorContext) error { @@ -97,9 +97,9 @@ func (h *httpServerInstrumentor) Load(ctx *context.InstrumentorContext) error { return err } - for _, funcName := range h.FuncNames() { - h.registerProbes(ctx, funcName) - } + // since both ServeMux.ServeHttp and HandlerFunc.ServeHttp functions are the same + // only one probe needs to be registered + h.registerProbes(ctx, (h.FuncNames()[0])) rd, err := perf.NewReader(h.bpfObjects.Events, os.Getpagesize()) if err != nil { From 28a5ac136808561227a75f4952955d1eee02f54f Mon Sep 17 00:00:00 2001 From: JamieDanielson Date: Thu, 20 Apr 2023 16:12:29 -0400 Subject: [PATCH 06/22] mirror changes to gorillamux from server #34 --- .../github.com/gorilla/mux/bpf/probe.bpf.c | 35 ++++++------ .../bpf/github.com/gorilla/mux/probe.go | 56 ++++++++++--------- 2 files changed, 49 insertions(+), 42 deletions(-) diff --git a/pkg/instrumentors/bpf/github.com/gorilla/mux/bpf/probe.bpf.c b/pkg/instrumentors/bpf/github.com/gorilla/mux/bpf/probe.bpf.c index 275510684..ee5239efc 100644 --- a/pkg/instrumentors/bpf/github.com/gorilla/mux/bpf/probe.bpf.c +++ b/pkg/instrumentors/bpf/github.com/gorilla/mux/bpf/probe.bpf.c @@ -18,25 +18,29 @@ char __license[] SEC("license") = "Dual MIT/GPL"; -#define MAX_SIZE 100 +#define PATH_MAX_LEN 100 +#define METHOD_MAX_LEN 6 // Longer method: DELETE #define MAX_CONCURRENT 50 struct http_request_t { u64 start_time; u64 end_time; - char method[MAX_SIZE]; - char path[MAX_SIZE]; + char method[METHOD_MAX_LEN]; + char path[PATH_MAX_LEN]; struct span_context sc; }; -struct { +// map key: pointer to the goroutine that handles the request +struct +{ __uint(type, BPF_MAP_TYPE_HASH); - __type(key, void*); + __type(key, void *); __type(value, struct http_request_t); __uint(max_entries, MAX_CONCURRENT); } context_to_http_events SEC(".maps"); -struct { +struct +{ __uint(type, BPF_MAP_TYPE_PERF_EVENT_ARRAY); } events SEC(".maps"); @@ -44,7 +48,6 @@ struct { volatile const u64 method_ptr_pos; volatile const u64 url_ptr_pos; volatile const u64 path_ptr_pos; -volatile const u64 ctx_ptr_pos; // This instrumentation attaches uprobe to the following function: // func (mux *ServeMux) ServeHTTP(w ResponseWriter, r *Request) @@ -77,14 +80,13 @@ int uprobe_GorillaMux_ServeHTTP(struct pt_regs *ctx) { path_size = path_size < path_len ? path_size : path_len; bpf_probe_read(&httpReq.path, path_size, path_ptr); - // Get Request.ctx - void *ctx_iface = 0; - bpf_probe_read(&ctx_iface, sizeof(ctx_iface), (void *)(req_ptr+ctx_ptr_pos+8)); + // Get goroutine pointer + void *goroutine = get_goroutine_address(ctx); // Write event httpReq.sc = generate_span_context(); - bpf_map_update_elem(&context_to_http_events, &ctx_iface, &httpReq, 0); - long res = bpf_map_update_elem(&spans_in_progress, &ctx_iface, &httpReq.sc, 0); + bpf_map_update_elem(&context_to_http_events, &goroutine, &httpReq, 0); + long res = bpf_map_update_elem(&spans_in_progress, &goroutine, &httpReq.sc, 0); return 0; } @@ -92,15 +94,14 @@ SEC("uprobe/GorillaMux_ServeHTTP") int uprobe_GorillaMux_ServeHTTP_Returns(struct pt_regs *ctx) { u64 request_pos = 4; void* req_ptr = get_argument(ctx, request_pos); - void *ctx_iface = 0; - bpf_probe_read(&ctx_iface, sizeof(ctx_iface), (void *)(req_ptr+ctx_ptr_pos+8)); + void *goroutine = get_goroutine_address(ctx); - void* httpReq_ptr = bpf_map_lookup_elem(&context_to_http_events, &ctx_iface); + void* httpReq_ptr = bpf_map_lookup_elem(&context_to_http_events, &goroutine); struct http_request_t httpReq = {}; bpf_probe_read(&httpReq, sizeof(httpReq), httpReq_ptr); httpReq.end_time = bpf_ktime_get_ns(); bpf_perf_event_output(ctx, &events, BPF_F_CURRENT_CPU, &httpReq, sizeof(httpReq)); - bpf_map_delete_elem(&context_to_http_events, &ctx_iface); - bpf_map_delete_elem(&spans_in_progress, &ctx_iface); + bpf_map_delete_elem(&context_to_http_events, &goroutine); + bpf_map_delete_elem(&spans_in_progress, &goroutine); return 0; } \ No newline at end of file diff --git a/pkg/instrumentors/bpf/github.com/gorilla/mux/probe.go b/pkg/instrumentors/bpf/github.com/gorilla/mux/probe.go index d10126f33..ac58dc5ef 100644 --- a/pkg/instrumentors/bpf/github.com/gorilla/mux/probe.go +++ b/pkg/instrumentors/bpf/github.com/gorilla/mux/probe.go @@ -40,14 +40,14 @@ import ( type HttpEvent struct { StartTime uint64 EndTime uint64 - Method [100]byte + Method [6]byte Path [100]byte SpanContext context.EbpfSpanContext } type gorillaMuxInstrumentor struct { bpfObjects *bpfObjects - uprobe link.Link + uprobes []link.Link returnProbs []link.Link eventsReader *perf.Reader } @@ -76,11 +76,6 @@ func (g *gorillaMuxInstrumentor) Load(ctx *context.InstrumentorContext) error { StructName: "net/http.Request", Field: "URL", }, - { - VarName: "ctx_ptr_pos", - StructName: "net/http.Request", - Field: "ctx", - }, { VarName: "path_ptr_pos", StructName: "net/url.URL", @@ -102,41 +97,52 @@ func (g *gorillaMuxInstrumentor) Load(ctx *context.InstrumentorContext) error { return err } - offset, err := ctx.TargetDetails.GetFunctionOffset(g.FuncNames()[0]) + for _, funcName := range g.FuncNames() { + g.registerProbes(ctx, funcName) + } + rd, err := perf.NewReader(g.bpfObjects.Events, os.Getpagesize()) if err != nil { return err } + g.eventsReader = rd + + return nil +} + +func (g *gorillaMuxInstrumentor) registerProbes(ctx *context.InstrumentorContext, funcName string) { + logger := log.Logger.WithName("gorilla/mux-instrumentor").WithValues("function", funcName) + offset, err := ctx.TargetDetails.GetFunctionOffset(funcName) + if err != nil { + logger.Error(err, "could not find function start offset. Skipping") + return + } + retOffsets, err := ctx.TargetDetails.GetFunctionReturns(funcName) + if err != nil { + logger.Error(err, "could not find function end offset. Skipping") + return + } up, err := ctx.Executable.Uprobe("", g.bpfObjects.UprobeGorillaMuxServeHTTP, &link.UprobeOptions{ Address: offset, }) if err != nil { - return err + logger.V(1).Info("could not insert start uprobe. Skipping", + "error", err.Error()) + return } - g.uprobe = up - retOffsets, err := ctx.TargetDetails.GetFunctionReturns(g.FuncNames()[0]) - if err != nil { - return err - } + g.uprobes = append(g.uprobes, up) for _, ret := range retOffsets { retProbe, err := ctx.Executable.Uprobe("", g.bpfObjects.UprobeGorillaMuxServeHTTP_Returns, &link.UprobeOptions{ Address: ret, }) if err != nil { - return err + logger.Error(err, "could not insert return uprobe. Skipping") + return } g.returnProbs = append(g.returnProbs, retProbe) } - - rd, err := perf.NewReader(g.bpfObjects.Events, os.Getpagesize()) - if err != nil { - return err - } - g.eventsReader = rd - - return nil } func (g *gorillaMuxInstrumentor) Run(eventsChan chan<- *events.Event) { @@ -196,8 +202,8 @@ func (g *gorillaMuxInstrumentor) Close() { g.eventsReader.Close() } - if g.uprobe != nil { - g.uprobe.Close() + for _, r := range g.uprobes { + r.Close() } for _, r := range g.returnProbs { From c84373173e5d576ce5c69c3bada09b4d85c5d5fb Mon Sep 17 00:00:00 2001 From: JamieDanielson Date: Thu, 20 Apr 2023 16:13:47 -0400 Subject: [PATCH 07/22] no more empty spans for gorillamux --- test/e2e/gorillamux/traces.json | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/test/e2e/gorillamux/traces.json b/test/e2e/gorillamux/traces.json index 0b0617cff..334e65aff 100644 --- a/test/e2e/gorillamux/traces.json +++ b/test/e2e/gorillamux/traces.json @@ -28,21 +28,22 @@ { "key": "http.method", "value": { - "stringValue": "" + "stringValue": "GET" } }, { "key": "http.target", "value": { - "stringValue": "" + "stringValue": "/users/foo" } } ], "kind": 2, + "name": "/users/foo", "parentSpanId": "", - "spanId": "", + "spanId": "xxxxx", "status": {}, - "traceId": "" + "traceId": "xxxxx" } ] }, From 853e5807e00080421ad0264c9123ff1236d8012b Mon Sep 17 00:00:00 2001 From: JamieDanielson Date: Thu, 20 Apr 2023 16:15:21 -0400 Subject: [PATCH 08/22] revert http funcname to 1, keep range for register --- pkg/instrumentors/bpf/net/http/server/probe.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/instrumentors/bpf/net/http/server/probe.go b/pkg/instrumentors/bpf/net/http/server/probe.go index f012dba65..3153297ad 100644 --- a/pkg/instrumentors/bpf/net/http/server/probe.go +++ b/pkg/instrumentors/bpf/net/http/server/probe.go @@ -61,7 +61,7 @@ func (h *httpServerInstrumentor) LibraryName() string { } func (h *httpServerInstrumentor) FuncNames() []string { - return []string{"net/http.(*ServeMux).ServeHTTP","net/http.HandlerFunc.ServeHTTP"} + return []string{"net/http.HandlerFunc.ServeHTTP"} } func (h *httpServerInstrumentor) Load(ctx *context.InstrumentorContext) error { @@ -97,9 +97,9 @@ func (h *httpServerInstrumentor) Load(ctx *context.InstrumentorContext) error { return err } - // since both ServeMux.ServeHttp and HandlerFunc.ServeHttp functions are the same - // only one probe needs to be registered - h.registerProbes(ctx, (h.FuncNames()[0])) + for _, funcName := range h.FuncNames() { + h.registerProbes(ctx, funcName) + } rd, err := perf.NewReader(h.bpfObjects.Events, os.Getpagesize()) if err != nil { From 205cfb70dea3ad05a31a7a0ad427ac44411da7af Mon Sep 17 00:00:00 2001 From: JamieDanielson Date: Thu, 20 Apr 2023 16:27:03 -0400 Subject: [PATCH 09/22] make generate --- pkg/instrumentors/bpf/github.com/gorilla/mux/bpf_bpfel.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pkg/instrumentors/bpf/github.com/gorilla/mux/bpf_bpfel.go b/pkg/instrumentors/bpf/github.com/gorilla/mux/bpf_bpfel.go index 56b9a71ee..9ff2a0497 100644 --- a/pkg/instrumentors/bpf/github.com/gorilla/mux/bpf_bpfel.go +++ b/pkg/instrumentors/bpf/github.com/gorilla/mux/bpf_bpfel.go @@ -16,9 +16,10 @@ import ( type bpfHttpRequestT struct { StartTime uint64 EndTime uint64 - Method [100]int8 + Method [6]int8 Path [100]int8 Sc bpfSpanContext + _ [6]byte } type bpfSpanContext struct { From 94be05f24c2ce510c9e0c1cc8c8819c2872abf7a Mon Sep 17 00:00:00 2001 From: JamieDanielson Date: Fri, 5 May 2023 14:38:14 -0400 Subject: [PATCH 10/22] fix error logger, merge some main changes --- .../bpf/github.com/gorilla/mux/bpf/probe.bpf.c | 8 ++++++-- pkg/instrumentors/bpf/github.com/gorilla/mux/probe.go | 10 +++++++--- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/pkg/instrumentors/bpf/github.com/gorilla/mux/bpf/probe.bpf.c b/pkg/instrumentors/bpf/github.com/gorilla/mux/bpf/probe.bpf.c index ac14f3210..c854ffb84 100644 --- a/pkg/instrumentors/bpf/github.com/gorilla/mux/bpf/probe.bpf.c +++ b/pkg/instrumentors/bpf/github.com/gorilla/mux/bpf/probe.bpf.c @@ -46,6 +46,7 @@ struct { volatile const u64 method_ptr_pos; volatile const u64 url_ptr_pos; volatile const u64 path_ptr_pos; +volatile const u64 ctx_ptr_pos; // This instrumentation attaches uprobe to the following function: // func (mux *ServeMux) ServeHTTP(w ResponseWriter, r *Request) @@ -84,14 +85,17 @@ int uprobe_GorillaMux_ServeHTTP(struct pt_regs *ctx) { // Write event httpReq.sc = generate_span_context(); bpf_map_update_elem(&context_to_http_events, &goroutine, &httpReq, 0); - bpf_map_update_elem(&spans_in_progress, &goroutine, &httpReq.sc, 0); + long res = bpf_map_update_elem(&spans_in_progress, &goroutine, &httpReq.sc, 0); return 0; } SEC("uprobe/GorillaMux_ServeHTTP") int uprobe_GorillaMux_ServeHTTP_Returns(struct pt_regs *ctx) { + u64 request_pos = 4; void *goroutine = get_goroutine_address(ctx, ctx_ptr_pos); - void *httpReq_ptr = bpf_map_lookup_elem(&context_to_http_events, &goroutine); + void* req_ptr = get_argument(ctx, request_pos); + + void* httpReq_ptr = bpf_map_lookup_elem(&context_to_http_events, &goroutine); struct http_request_t httpReq = {}; bpf_probe_read(&httpReq, sizeof(httpReq), httpReq_ptr); httpReq.end_time = bpf_ktime_get_ns(); diff --git a/pkg/instrumentors/bpf/github.com/gorilla/mux/probe.go b/pkg/instrumentors/bpf/github.com/gorilla/mux/probe.go index 364591bc8..b6b57669f 100644 --- a/pkg/instrumentors/bpf/github.com/gorilla/mux/probe.go +++ b/pkg/instrumentors/bpf/github.com/gorilla/mux/probe.go @@ -86,6 +86,11 @@ func (g *Instrumentor) Load(ctx *context.InstrumentorContext) error { StructName: "net/http.Request", Field: "URL", }, + { + VarName: "ctx_ptr_pos", + StructName: "net/http.Request", + Field: "ctx", + }, { VarName: "path_ptr_pos", StructName: "net/url.URL", @@ -119,7 +124,7 @@ func (g *Instrumentor) Load(ctx *context.InstrumentorContext) error { return nil } -func (g *gorillaMuxInstrumentor) registerProbes(ctx *context.InstrumentorContext, funcName string) { +func (g *Instrumentor) registerProbes(ctx *context.InstrumentorContext, funcName string) { logger := log.Logger.WithName("gorilla/mux-instrumentor").WithValues("function", funcName) offset, err := ctx.TargetDetails.GetFunctionOffset(funcName) if err != nil { @@ -136,8 +141,7 @@ func (g *gorillaMuxInstrumentor) registerProbes(ctx *context.InstrumentorContext Address: offset, }) if err != nil { - logger.V(1).Info("could not insert start uprobe. Skipping", - "error", err.Error()) + logger.Error(err, "could not insert start uprobe. Skipping") return } From ce13c1d5197f300aeee8762d25999f792c8141cc Mon Sep 17 00:00:00 2001 From: JamieDanielson Date: Fri, 5 May 2023 15:06:34 -0400 Subject: [PATCH 11/22] update changelog --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index a84fdaa82..29898b9eb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,10 @@ OpenTelemetry Go Automatic Instrumentation adheres to [Semantic Versioning](http ## [Unreleased] +### Added + +- Fix missing spans in gorillamux instrumentation ([#86](https://github.com/open-telemetry/opentelemetry-go-instrumentation/pull/86)) + ## [v0.2.0-alpha] - 2023-05-03 ### Added From 2bd277bc05bf9c24a9d8146ce799ed9551a9f557 Mon Sep 17 00:00:00 2001 From: Jamie Danielson Date: Tue, 9 May 2023 12:51:47 -0400 Subject: [PATCH 12/22] Update CHANGELOG.md Co-authored-by: Tyler Yahn --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 29898b9eb..76bf719a5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,7 +10,7 @@ OpenTelemetry Go Automatic Instrumentation adheres to [Semantic Versioning](http ### Added -- Fix missing spans in gorillamux instrumentation ([#86](https://github.com/open-telemetry/opentelemetry-go-instrumentation/pull/86)) +- Fix missing spans in gorillamux instrumentation. ([#86](https://github.com/open-telemetry/opentelemetry-go-instrumentation/pull/86)) ## [v0.2.0-alpha] - 2023-05-03 From 806a608ba224c70bb5f53756fb83d4876a7b76e9 Mon Sep 17 00:00:00 2001 From: Jamie Danielson Date: Tue, 9 May 2023 12:52:00 -0400 Subject: [PATCH 13/22] Update CHANGELOG.md Co-authored-by: Tyler Yahn --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 76bf719a5..c18dc7606 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,7 +8,7 @@ OpenTelemetry Go Automatic Instrumentation adheres to [Semantic Versioning](http ## [Unreleased] -### Added +### Fixed - Fix missing spans in gorillamux instrumentation. ([#86](https://github.com/open-telemetry/opentelemetry-go-instrumentation/pull/86)) From 6c651e80bec4bd1bbd3e10687c9fe5eca0367688 Mon Sep 17 00:00:00 2001 From: Jamie Danielson Date: Tue, 9 May 2023 14:45:44 -0400 Subject: [PATCH 14/22] Update pkg/instrumentors/bpf/github.com/gorilla/mux/bpf/probe.bpf.c Co-authored-by: Mike Goldsmith --- pkg/instrumentors/bpf/github.com/gorilla/mux/bpf/probe.bpf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/instrumentors/bpf/github.com/gorilla/mux/bpf/probe.bpf.c b/pkg/instrumentors/bpf/github.com/gorilla/mux/bpf/probe.bpf.c index c854ffb84..eabaa7f4f 100644 --- a/pkg/instrumentors/bpf/github.com/gorilla/mux/bpf/probe.bpf.c +++ b/pkg/instrumentors/bpf/github.com/gorilla/mux/bpf/probe.bpf.c @@ -85,7 +85,7 @@ int uprobe_GorillaMux_ServeHTTP(struct pt_regs *ctx) { // Write event httpReq.sc = generate_span_context(); bpf_map_update_elem(&context_to_http_events, &goroutine, &httpReq, 0); - long res = bpf_map_update_elem(&spans_in_progress, &goroutine, &httpReq.sc, 0); + bpf_map_update_elem(&spans_in_progress, &goroutine, &httpReq.sc, 0); return 0; } From 5a075a4d8e4254a9e727e7f559d34e347e4bb37f Mon Sep 17 00:00:00 2001 From: JamieDanielson Date: Tue, 9 May 2023 14:48:37 -0400 Subject: [PATCH 15/22] set Method size to 7 in gorillamux probe --- pkg/instrumentors/bpf/github.com/gorilla/mux/probe.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/instrumentors/bpf/github.com/gorilla/mux/probe.go b/pkg/instrumentors/bpf/github.com/gorilla/mux/probe.go index b6b57669f..7cd5c3a14 100644 --- a/pkg/instrumentors/bpf/github.com/gorilla/mux/probe.go +++ b/pkg/instrumentors/bpf/github.com/gorilla/mux/probe.go @@ -44,7 +44,7 @@ import ( type Event struct { StartTime uint64 EndTime uint64 - Method [6]byte + Method [7]byte Path [100]byte SpanContext context.EBPFSpanContext } From 32cc57bb0e095b64ca5a5978740b9659e61069c9 Mon Sep 17 00:00:00 2001 From: JamieDanielson Date: Tue, 9 May 2023 14:48:47 -0400 Subject: [PATCH 16/22] set Method size to 7 in gin-gonic probe --- pkg/instrumentors/bpf/github.com/gin-gonic/gin/probe.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/instrumentors/bpf/github.com/gin-gonic/gin/probe.go b/pkg/instrumentors/bpf/github.com/gin-gonic/gin/probe.go index 7c05651b6..117ec7bf6 100644 --- a/pkg/instrumentors/bpf/github.com/gin-gonic/gin/probe.go +++ b/pkg/instrumentors/bpf/github.com/gin-gonic/gin/probe.go @@ -45,7 +45,7 @@ import ( type Event struct { StartTime uint64 EndTime uint64 - Method [6]byte + Method [7]byte Path [100]byte SpanContext context.EBPFSpanContext } From f8d5608209fa4dd4cd00a0f972a85a77fc32095c Mon Sep 17 00:00:00 2001 From: JamieDanielson Date: Tue, 9 May 2023 14:49:03 -0400 Subject: [PATCH 17/22] set Method size to 7 in nethttp probe --- pkg/instrumentors/bpf/net/http/server/probe.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/instrumentors/bpf/net/http/server/probe.go b/pkg/instrumentors/bpf/net/http/server/probe.go index 5ae733999..7064115cf 100644 --- a/pkg/instrumentors/bpf/net/http/server/probe.go +++ b/pkg/instrumentors/bpf/net/http/server/probe.go @@ -44,7 +44,7 @@ import ( type Event struct { StartTime uint64 EndTime uint64 - Method [6]byte + Method [7]byte Path [100]byte SpanContext context.EBPFSpanContext } From 1ea0e48f011c0545100b4b3ec3f060362893a90f Mon Sep 17 00:00:00 2001 From: JamieDanielson Date: Tue, 9 May 2023 16:37:01 -0400 Subject: [PATCH 18/22] updated traces.json after span name pr merged --- test/e2e/gorillamux/traces.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/e2e/gorillamux/traces.json b/test/e2e/gorillamux/traces.json index 0ac5cedaf..26027ede2 100644 --- a/test/e2e/gorillamux/traces.json +++ b/test/e2e/gorillamux/traces.json @@ -45,7 +45,7 @@ } ], "kind": 2, - "name": "/users/foo", + "name": "GET /users/foo", "parentSpanId": "", "spanId": "xxxxx", "status": {}, From 2c9a55674523e94664ef9715d640014667c875ff Mon Sep 17 00:00:00 2001 From: JamieDanielson Date: Tue, 9 May 2023 16:37:49 -0400 Subject: [PATCH 19/22] also set method len to 7 in gorillamux probe c --- pkg/instrumentors/bpf/github.com/gorilla/mux/bpf/probe.bpf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/instrumentors/bpf/github.com/gorilla/mux/bpf/probe.bpf.c b/pkg/instrumentors/bpf/github.com/gorilla/mux/bpf/probe.bpf.c index eabaa7f4f..e20865fe9 100644 --- a/pkg/instrumentors/bpf/github.com/gorilla/mux/bpf/probe.bpf.c +++ b/pkg/instrumentors/bpf/github.com/gorilla/mux/bpf/probe.bpf.c @@ -19,7 +19,7 @@ char __license[] SEC("license") = "Dual MIT/GPL"; #define PATH_MAX_LEN 100 -#define METHOD_MAX_LEN 6 // Longer method: DELETE +#define METHOD_MAX_LEN 7 #define MAX_CONCURRENT 50 struct http_request_t { From 6ade2a27147f6640e6fc796055089a2d7d07a477 Mon Sep 17 00:00:00 2001 From: JamieDanielson Date: Tue, 9 May 2023 16:38:27 -0400 Subject: [PATCH 20/22] also set method len to 7 in gin-gonic probe c --- pkg/instrumentors/bpf/github.com/gin-gonic/gin/bpf/probe.bpf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/instrumentors/bpf/github.com/gin-gonic/gin/bpf/probe.bpf.c b/pkg/instrumentors/bpf/github.com/gin-gonic/gin/bpf/probe.bpf.c index 0c595e789..a068fe450 100644 --- a/pkg/instrumentors/bpf/github.com/gin-gonic/gin/bpf/probe.bpf.c +++ b/pkg/instrumentors/bpf/github.com/gin-gonic/gin/bpf/probe.bpf.c @@ -19,7 +19,7 @@ char __license[] SEC("license") = "Dual MIT/GPL"; #define PATH_MAX_LEN 100 -#define METHOD_MAX_LEN 6 // Longer method: DELETE +#define METHOD_MAX_LEN 7 #define MAX_CONCURRENT 50 struct http_request_t { From 4701f500f184da4c15d61bda8998a9371aa2da84 Mon Sep 17 00:00:00 2001 From: JamieDanielson Date: Tue, 9 May 2023 16:38:36 -0400 Subject: [PATCH 21/22] also set method len to 7 in nethttp probe c --- pkg/instrumentors/bpf/net/http/server/bpf/probe.bpf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/instrumentors/bpf/net/http/server/bpf/probe.bpf.c b/pkg/instrumentors/bpf/net/http/server/bpf/probe.bpf.c index bb724666b..bde32a61b 100644 --- a/pkg/instrumentors/bpf/net/http/server/bpf/probe.bpf.c +++ b/pkg/instrumentors/bpf/net/http/server/bpf/probe.bpf.c @@ -19,7 +19,7 @@ char __license[] SEC("license") = "Dual MIT/GPL"; #define PATH_MAX_LEN 100 -#define METHOD_MAX_LEN 6 // Longer method: DELETE +#define METHOD_MAX_LEN 7 #define MAX_CONCURRENT 50 struct http_request_t From 4f2f16f59c9ee98c858431ff80f42b081ae5d8ad Mon Sep 17 00:00:00 2001 From: JamieDanielson Date: Tue, 9 May 2023 16:44:49 -0400 Subject: [PATCH 22/22] make docker-generate --- .../bpf/github.com/gin-gonic/gin/bpf_bpfel_arm64.go | 4 ++-- .../bpf/github.com/gin-gonic/gin/bpf_bpfel_x86.go | 4 ++-- .../bpf/github.com/gorilla/mux/bpf_bpfel_arm64.go | 4 ++-- pkg/instrumentors/bpf/github.com/gorilla/mux/bpf_bpfel_x86.go | 4 ++-- pkg/instrumentors/bpf/net/http/server/bpf_bpfel_arm64.go | 4 ++-- pkg/instrumentors/bpf/net/http/server/bpf_bpfel_x86.go | 4 ++-- 6 files changed, 12 insertions(+), 12 deletions(-) diff --git a/pkg/instrumentors/bpf/github.com/gin-gonic/gin/bpf_bpfel_arm64.go b/pkg/instrumentors/bpf/github.com/gin-gonic/gin/bpf_bpfel_arm64.go index be4cd2a39..3532cd519 100644 --- a/pkg/instrumentors/bpf/github.com/gin-gonic/gin/bpf_bpfel_arm64.go +++ b/pkg/instrumentors/bpf/github.com/gin-gonic/gin/bpf_bpfel_arm64.go @@ -16,10 +16,10 @@ import ( type bpfHttpRequestT struct { StartTime uint64 EndTime uint64 - Method [6]int8 + Method [7]int8 Path [100]int8 Sc bpfSpanContext - _ [6]byte + _ [5]byte } type bpfSpanContext struct { diff --git a/pkg/instrumentors/bpf/github.com/gin-gonic/gin/bpf_bpfel_x86.go b/pkg/instrumentors/bpf/github.com/gin-gonic/gin/bpf_bpfel_x86.go index d57a435b0..787c38522 100644 --- a/pkg/instrumentors/bpf/github.com/gin-gonic/gin/bpf_bpfel_x86.go +++ b/pkg/instrumentors/bpf/github.com/gin-gonic/gin/bpf_bpfel_x86.go @@ -16,10 +16,10 @@ import ( type bpfHttpRequestT struct { StartTime uint64 EndTime uint64 - Method [6]int8 + Method [7]int8 Path [100]int8 Sc bpfSpanContext - _ [6]byte + _ [5]byte } type bpfSpanContext struct { diff --git a/pkg/instrumentors/bpf/github.com/gorilla/mux/bpf_bpfel_arm64.go b/pkg/instrumentors/bpf/github.com/gorilla/mux/bpf_bpfel_arm64.go index 3d8c80d1d..e64f9a790 100644 --- a/pkg/instrumentors/bpf/github.com/gorilla/mux/bpf_bpfel_arm64.go +++ b/pkg/instrumentors/bpf/github.com/gorilla/mux/bpf_bpfel_arm64.go @@ -16,10 +16,10 @@ import ( type bpfHttpRequestT struct { StartTime uint64 EndTime uint64 - Method [6]int8 + Method [7]int8 Path [100]int8 Sc bpfSpanContext - _ [6]byte + _ [5]byte } type bpfSpanContext struct { diff --git a/pkg/instrumentors/bpf/github.com/gorilla/mux/bpf_bpfel_x86.go b/pkg/instrumentors/bpf/github.com/gorilla/mux/bpf_bpfel_x86.go index 50f832a31..2af10aa70 100644 --- a/pkg/instrumentors/bpf/github.com/gorilla/mux/bpf_bpfel_x86.go +++ b/pkg/instrumentors/bpf/github.com/gorilla/mux/bpf_bpfel_x86.go @@ -16,10 +16,10 @@ import ( type bpfHttpRequestT struct { StartTime uint64 EndTime uint64 - Method [6]int8 + Method [7]int8 Path [100]int8 Sc bpfSpanContext - _ [6]byte + _ [5]byte } type bpfSpanContext struct { diff --git a/pkg/instrumentors/bpf/net/http/server/bpf_bpfel_arm64.go b/pkg/instrumentors/bpf/net/http/server/bpf_bpfel_arm64.go index 8b017c446..8d06beb63 100644 --- a/pkg/instrumentors/bpf/net/http/server/bpf_bpfel_arm64.go +++ b/pkg/instrumentors/bpf/net/http/server/bpf_bpfel_arm64.go @@ -16,10 +16,10 @@ import ( type bpfHttpRequestT struct { StartTime uint64 EndTime uint64 - Method [6]int8 + Method [7]int8 Path [100]int8 Sc bpfSpanContext - _ [6]byte + _ [5]byte } type bpfSpanContext struct { diff --git a/pkg/instrumentors/bpf/net/http/server/bpf_bpfel_x86.go b/pkg/instrumentors/bpf/net/http/server/bpf_bpfel_x86.go index 8d70f5d5e..d6cb3da28 100644 --- a/pkg/instrumentors/bpf/net/http/server/bpf_bpfel_x86.go +++ b/pkg/instrumentors/bpf/net/http/server/bpf_bpfel_x86.go @@ -16,10 +16,10 @@ import ( type bpfHttpRequestT struct { StartTime uint64 EndTime uint64 - Method [6]int8 + Method [7]int8 Path [100]int8 Sc bpfSpanContext - _ [6]byte + _ [5]byte } type bpfSpanContext struct {