Skip to content

Commit

Permalink
Fix missing spans in gorillamux instrumentation (#86)
Browse files Browse the repository at this point in the history
* adjust output for e2e test to remove empty span

* only use one http function in http server instr

* Revert "only use one http function in http server instr"

This reverts commit c1bc4d0.

* only use one http function - keep refactor

* keep both func names but only register 1 probe

* mirror changes to gorillamux from server #34

* no more empty spans for gorillamux

* revert http funcname to 1, keep range for register

* make generate

* fix error logger, merge some main changes

* update changelog

* Update CHANGELOG.md

Co-authored-by: Tyler Yahn <MrAlias@users.noreply.github.com>

* Update CHANGELOG.md

Co-authored-by: Tyler Yahn <MrAlias@users.noreply.github.com>

* Update pkg/instrumentors/bpf/github.com/gorilla/mux/bpf/probe.bpf.c

Co-authored-by: Mike Goldsmith <goldsmith.mike@gmail.com>

* set Method size to 7 in gorillamux probe

* set Method size to 7 in gin-gonic probe

* set Method size to 7 in nethttp probe

* updated traces.json after span name pr merged

* also set method len to 7 in gorillamux probe c

* also set method len to 7 in gin-gonic probe c

* also set method len to 7 in nethttp probe c

* make docker-generate

---------

Co-authored-by: Mike Goldsmith <goldsmith.mike@gmail.com>
Co-authored-by: Tyler Yahn <MrAlias@users.noreply.github.com>
  • Loading branch information
3 people authored May 12, 2023
1 parent 4de1a1f commit 5bd910d
Show file tree
Hide file tree
Showing 14 changed files with 81 additions and 38 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ OpenTelemetry Go Automatic Instrumentation adheres to [Semantic Versioning](http
### Fixed

- Fix gRPC instrumentation memory access issue on newer kernels. ([#150](https://github.com/open-telemetry/opentelemetry-go-instrumentation/pull/150))
- Fix missing spans in gorillamux instrumentation. ([#86](https://github.com/open-telemetry/opentelemetry-go-instrumentation/pull/86))

### Changed

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion pkg/instrumentors/bpf/github.com/gin-gonic/gin/probe.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ import (
type Event struct {
StartTime uint64
EndTime uint64
Method [6]byte
Method [7]byte
Path [100]byte
SpanContext context.EBPFSpanContext
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -91,8 +91,11 @@ int uprobe_GorillaMux_ServeHTTP(struct pt_regs *ctx) {

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();
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

50 changes: 30 additions & 20 deletions pkg/instrumentors/bpf/github.com/gorilla/mux/probe.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,15 +45,15 @@ import (
type Event struct {
StartTime uint64
EndTime uint64
Method [100]byte
Method [7]byte
Path [100]byte
SpanContext context.EBPFSpanContext
}

// Instrumentor is the gorilla/mux instrumentor.
type Instrumentor struct {
bpfObjects *bpfObjects
uprobe link.Link
uprobes []link.Link
returnProbs []link.Link
eventsReader *perf.Reader
}
Expand Down Expand Up @@ -113,41 +113,51 @@ func (g *Instrumentor) 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 *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 {
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.Error(err, "could not insert start uprobe. Skipping")
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
}

// Run runs the events processing loop.
Expand Down Expand Up @@ -210,8 +220,8 @@ func (g *Instrumentor) Close() {
g.eventsReader.Close()
}

if g.uprobe != nil {
g.uprobe.Close()
for _, r := range g.uprobes {
r.Close()
}

for _, r := range g.returnProbs {
Expand Down
2 changes: 1 addition & 1 deletion pkg/instrumentors/bpf/net/http/server/bpf/probe.bpf.c
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions pkg/instrumentors/bpf/net/http/server/bpf_bpfel_arm64.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions pkg/instrumentors/bpf/net/http/server/bpf_bpfel_x86.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion pkg/instrumentors/bpf/net/http/server/probe.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ import (
type Event struct {
StartTime uint64
EndTime uint64
Method [6]byte
Method [7]byte
Path [100]byte
SpanContext context.EBPFSpanContext
}
Expand Down
29 changes: 29 additions & 0 deletions test/e2e/gorillamux/traces.json
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,35 @@
]
},
"scopeSpans": [
{
"scope": {
"name": "github.com/gorilla/mux"
},
"spans": [
{
"attributes": [
{
"key": "http.method",
"value": {
"stringValue": "GET"
}
},
{
"key": "http.target",
"value": {
"stringValue": "/users/foo"
}
}
],
"kind": 2,
"name": "GET /users/foo",
"parentSpanId": "",
"spanId": "xxxxx",
"status": {},
"traceId": "xxxxx"
}
]
},
{
"scope": {
"name": "net/http"
Expand Down

0 comments on commit 5bd910d

Please sign in to comment.