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

Refactor Go-DI template generation to use location expressions #31424

Open
wants to merge 44 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 34 commits
Commits
Show all changes
44 commits
Select commit Hold shift + click to select a range
ed0860d
Add more sample functions to sample service, add span creation
grantseltzer Nov 25, 2024
ac2a051
Break up bpf program into multiple headers
grantseltzer Nov 25, 2024
c111c5c
Refactor central logic of instrumentation to use location expressions
grantseltzer Nov 25, 2024
25b7519
Rename header file with type that gets generated for Go consumption
grantseltzer Nov 25, 2024
67bead1
Fix dynamic string reading
grantseltzer Nov 27, 2024
5438bcd
Change all Parameter uses to pointers
grantseltzer Dec 2, 2024
25b277f
Fix strings and slice headers such that length is encoded in headers …
grantseltzer Dec 4, 2024
6dc04b1
Update event parsing
grantseltzer Dec 5, 2024
d3ba30e
Fix strings, remove unneeded writing of length to output buffer
grantseltzer Dec 5, 2024
37304d5
Change logic for collection limit labels, panic checks
grantseltzer Dec 6, 2024
88d3eac
Add location expressions for comments and print statements
grantseltzer Dec 6, 2024
cf11f6e
Remove global map of seenTypes
grantseltzer Dec 9, 2024
7449788
Nil and length checks
grantseltzer Dec 10, 2024
eaa1cda
More nil checks, and print statement improvements
grantseltzer Dec 10, 2024
a1daad1
More nil checks
grantseltzer Dec 10, 2024
f06dba0
Restructure tests and update fixtures
grantseltzer Dec 10, 2024
62e389c
Fix location_expression_test.go to actually run (needs more testcases…
grantseltzer Dec 11, 2024
76e6744
Add Kafka go forwarder sample
grantseltzer Dec 11, 2024
b7e5ed3
Fix binary_inspection_test.go to actually run (needs more testcases t…
grantseltzer Dec 11, 2024
275e582
Update 3rdparty license file
grantseltzer Dec 11, 2024
a663e9b
Linting fixes
grantseltzer Dec 11, 2024
39f825a
Nil check for stack traces
grantseltzer Dec 12, 2024
f4519c3
Add circular reference sample, fixed logging on e2e test
grantseltzer Dec 12, 2024
c1fdf82
Fix circular reference type causing stack overflow
grantseltzer Dec 12, 2024
9fdff56
Remove rate limit for config probe
grantseltzer Dec 12, 2024
d2c7dc7
Remove redundant test
grantseltzer Dec 13, 2024
09b1a44
Remove kafka_go_forwarder sample go.sum/go.mod files
grantseltzer Dec 13, 2024
48ac32c
Update go.mod/go.sum for sample service
grantseltzer Dec 13, 2024
f89e905
switch from bpf_printk to log_debug
grantseltzer Dec 17, 2024
5ec3963
Fix the way limits are configured and applied, fix default value for …
grantseltzer Dec 17, 2024
656a3e0
rename variables in test
grantseltzer Dec 17, 2024
0cc21a0
Fix location expression for capturing slice length
grantseltzer Dec 17, 2024
350fbea
Style fixes
grantseltzer Dec 17, 2024
0717905
Remove span from sample service
grantseltzer Dec 17, 2024
bc87164
More comments/debugability
grantseltzer Dec 18, 2024
4374ca0
Make collection_limit a LRU
grantseltzer Dec 18, 2024
52ae648
Add comments above all location expressions
grantseltzer Dec 18, 2024
8b2e91b
Add start of param parsing test
grantseltzer Dec 18, 2024
f9091f9
Remove LRU
grantseltzer Dec 18, 2024
33d8015
Add logic to clear the bpf stack map at the end of invocations
brycekahle Oct 31, 2024
f07d69b
Fix expression header file macro name
grantseltzer Dec 18, 2024
d635dfb
Remove apache license copyright from top of bpf files
grantseltzer Dec 18, 2024
0eb45ce
Remove duplicate length check
grantseltzer Dec 18, 2024
b345511
Remove kafka go forwarder sample for now
grantseltzer Dec 18, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions LICENSE-3rdparty.csv
Original file line number Diff line number Diff line change
Expand Up @@ -2932,6 +2932,7 @@ core,gopkg.in/DataDog/dd-trace-go.v1/appsec/events,Apache-2.0,"Copyright 2016-Pr
core,gopkg.in/DataDog/dd-trace-go.v1/contrib/internal/httptrace,Apache-2.0,"Copyright 2016-Present Datadog, Inc."
core,gopkg.in/DataDog/dd-trace-go.v1/contrib/internal/options,Apache-2.0,"Copyright 2016-Present Datadog, Inc."
core,gopkg.in/DataDog/dd-trace-go.v1/contrib/net/http,Apache-2.0,"Copyright 2016-Present Datadog, Inc."
core,gopkg.in/DataDog/dd-trace-go.v1/datastreams,Apache-2.0,"Copyright 2016-Present Datadog, Inc."
core,gopkg.in/DataDog/dd-trace-go.v1/datastreams/options,Apache-2.0,"Copyright 2016-Present Datadog, Inc."
core,gopkg.in/DataDog/dd-trace-go.v1/ddtrace,Apache-2.0,"Copyright 2016-Present Datadog, Inc."
core,gopkg.in/DataDog/dd-trace-go.v1/ddtrace/ext,Apache-2.0,"Copyright 2016-Present Datadog, Inc."
Expand Down
13 changes: 12 additions & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -480,7 +480,7 @@ require (
github.com/mitchellh/hashstructure/v2 v2.0.2 // indirect
github.com/mitchellh/reflectwalk v1.0.2 // indirect
github.com/mkrautz/goar v0.0.0-20150919110319-282caa8bd9da // indirect
github.com/moby/buildkit v0.12.5 // indirect
github.com/moby/buildkit v0.14.1 // indirect
grantseltzer marked this conversation as resolved.
Show resolved Hide resolved
github.com/moby/locker v1.0.1 // indirect
github.com/moby/sys/signal v0.7.0 // indirect
github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect
Expand Down Expand Up @@ -616,6 +616,8 @@ require (
)

require (
github.com/Shopify/sarama v1.38.1
github.com/confluentinc/confluent-kafka-go/v2 v2.6.1
github.com/shirou/gopsutil/v4 v4.24.11
go.opentelemetry.io/collector/component/componenttest v0.115.0
)
Expand Down Expand Up @@ -838,6 +840,9 @@ require (
github.com/csaf-poc/csaf_distribution/v3 v3.0.0 // indirect
github.com/dennwc/varint v1.0.0 // indirect
github.com/digitalocean/godo v1.118.0 // indirect
github.com/eapache/go-resiliency v1.7.0 // indirect
github.com/eapache/go-xerial-snappy v0.0.0-20230731223053-c322873962e3 // indirect
github.com/eapache/queue v1.1.0 // indirect
github.com/eapache/queue/v2 v2.0.0-20230407133247-75960ed334e4 // indirect
github.com/ebitengine/purego v0.8.1 // indirect
github.com/elastic/go-grok v0.3.1 // indirect
Expand Down Expand Up @@ -877,6 +882,7 @@ require (
github.com/hashicorp/go-secure-stdlib/parseutil v0.1.8 // indirect
github.com/hashicorp/go-secure-stdlib/strutil v0.1.2 // indirect
github.com/hashicorp/go-sockaddr v1.0.6 // indirect
github.com/hashicorp/go-uuid v1.0.3 // indirect
github.com/hashicorp/nomad/api v0.0.0-20240717122358-3d93bd3778f3 // indirect
github.com/hetznercloud/hcloud-go/v2 v2.10.2 // indirect
github.com/iancoleman/strcase v0.3.0 // indirect
Expand All @@ -886,6 +892,11 @@ require (
github.com/jackc/puddle/v2 v2.2.1 // indirect
github.com/jaegertracing/jaeger v1.62.0 // indirect
github.com/jbenet/go-context v0.0.0-20150711004518-d14ea06fba99 // indirect
github.com/jcmturner/aescts/v2 v2.0.0 // indirect
github.com/jcmturner/dnsutils/v2 v2.0.0 // indirect
github.com/jcmturner/gofork v1.7.6 // indirect
github.com/jcmturner/gokrb5/v8 v8.4.4 // indirect
github.com/jcmturner/rpc/v2 v2.0.3 // indirect
github.com/jonboulle/clockwork v0.4.0 // indirect
github.com/jpillora/backoff v1.0.0 // indirect
github.com/kevinburke/ssh_config v1.2.0 // indirect
Expand Down
27 changes: 27 additions & 0 deletions go.sum

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

Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
#ifndef DI_TYPES_H
#define DI_TYPES_H
#ifndef DI_BASE_EVENT_H
#define DI_BASE_EVENT_H

#include "ktypes.h"

// NOTE: Be careful when adding fields, alignment should always be to 8 bytes
struct base_event {
grantseltzer marked this conversation as resolved.
Show resolved Hide resolved
char probe_id[304];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

consider adding a comment to explain the "magic numbers" for this struct's fields of array type (304 and 10)
even better replace it with #define

__u32 pid;
Expand Down
59 changes: 31 additions & 28 deletions pkg/dynamicinstrumentation/codegen/c/dynamicinstrumentation.c
Original file line number Diff line number Diff line change
Expand Up @@ -2,49 +2,30 @@
#include "bpf_tracing.h"
#include "kconfig.h"
#include <asm/ptrace.h>
#include "types.h"

#define MAX_STRING_SIZE {{ .InstrumentationInfo.InstrumentationOptions.StringMaxSize}}
#define PARAM_BUFFER_SIZE {{ .InstrumentationInfo.InstrumentationOptions.ArgumentsMaxSize}}
#define STACK_DEPTH_LIMIT 10
#define MAX_SLICE_SIZE 1800
#define MAX_SLICE_LENGTH 20

struct {
__uint(type, BPF_MAP_TYPE_RINGBUF);
__uint(max_entries, 1 << 24);
} events SEC(".maps");

struct {
__uint(type, BPF_MAP_TYPE_ARRAY);
__uint(key_size, sizeof(__u32));
__uint(value_size, sizeof(char[PARAM_BUFFER_SIZE]));
__uint(max_entries, 1);
} zeroval SEC(".maps");

struct event {
struct base_event base;
char output[PARAM_BUFFER_SIZE];
};
#include "base_event.h"
#include "macros.h"
#include "event.h"
#include "maps.h"
#include "expressions.h"

SEC("uprobe/{{.GetBPFFuncName}}")
int {{.GetBPFFuncName}}(struct pt_regs *ctx)
{
bpf_printk("{{.GetBPFFuncName}} probe in {{.ServiceName}} has triggered");
log_debug("{{.GetBPFFuncName}} probe in {{.ServiceName}} has triggered");

// reserve space on ringbuffer
struct event *event;
event = bpf_ringbuf_reserve(&events, sizeof(struct event), 0);
if (!event) {
bpf_printk("No space available on ringbuffer, dropping event");
log_debug("No space available on ringbuffer, dropping event");
return 0;
}

char* zero_string;
__u32 key = 0;
zero_string = bpf_map_lookup_elem(&zeroval, &key);
if (!zero_string) {
bpf_printk("couldn't lookup zero value in zeroval array map, dropping event for {{.GetBPFFuncName}}");
log_debug("couldn't lookup zero value in zeroval array map, dropping event for {{.GetBPFFuncName}}");
bpf_ringbuf_discard(event, 0);
return 0;
}
Expand Down Expand Up @@ -72,6 +53,8 @@ int {{.GetBPFFuncName}}(struct pt_regs *ctx)
__u64 ret_addr = PT_REGS_RET(ctx); // when bpf prog enters, the return address hasn't yet been written to the stack

int i;
int j;
__u16 n;
for (i = 1; i < STACK_DEPTH_LIMIT; i++)
{
if (bp == 0) {
Expand All @@ -83,11 +66,31 @@ int {{.GetBPFFuncName}}(struct pt_regs *ctx)
}

// Collect parameters
__u16 collectionMax = MAX_SLICE_LENGTH;
__u8 param_type;
__u16 param_size;
__u16 slice_length;
__u16 *collectionLimit;

int outputOffset = 0;
int chunk_size = 0;
__u64 outputOffset = 0;

// Set up temporary storage array which is used by some location expressions
// to have memory off the stack to work with
__u64 *temp_storage = bpf_map_lookup_elem(&temp_storage_array, &key) ;
if (!temp_storage) {
log_debug("could not lookup temporary storage array");
bpf_ringbuf_discard(event, 0);
return 0;
}

struct expression_context context = {
.ctx = ctx,
.output_offset = &outputOffset,
.event = event,
.temp_storage = temp_storage,
.zero_string = zero_string
};

{{ .InstrumentationInfo.BPFParametersSourceCode }}

Expand Down
19 changes: 19 additions & 0 deletions pkg/dynamicinstrumentation/codegen/c/event.h
grantseltzer marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
#ifndef DI_EVENT_H
#define DI_EVENT_H
grantseltzer marked this conversation as resolved.
Show resolved Hide resolved

#include "ktypes.h"

struct event {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: i would change all struct declarations to typedef

struct base_event base;
char output[PARAM_BUFFER_SIZE];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think you need to include macros.h for the PARAM_BUFFER_SIZE

};

struct expression_context {
__u64 *output_offset;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why this needs to be a pointer?

struct pt_regs *ctx;
struct event *event;
__u64 *temp_storage;
char *zero_string;
};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i assume those are the very core structs in your module, consider adding a short paragraph in code to explain their purpose (since the names of the structs are pretty generic)


#endif
Loading
Loading