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.

Sign up for GitHub

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

Add support for FastCGI protocol #1417

Merged
merged 7 commits into from
Dec 2, 2024
Merged

Add support for FastCGI protocol #1417

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
898ea5f
Add support for FastCGI protocol
grcevski Nov 28, 2024
40917f5
Refactor black box propagation to support TCP
grcevski Nov 28, 2024
1a5aeca
Revert temp changes to tests
grcevski Nov 28, 2024
73d2091
Add unit tests
grcevski Nov 28, 2024
6f72cfd
add php-fpm integration test, more unit tests, look for Status
grcevski Nov 29, 2024
6a3ce17
Make kafka test more reliable
grcevski Nov 29, 2024
db9fa52
fix lint error
grcevski Nov 29, 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
2 changes: 1 addition & 1 deletion bpf/go_common.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,7 @@ server_trace_parent(void *goroutine_addr, tp_info_t *tp, tp_info_t *found_tp) {
} else {
// If not, we then look up the information in the black-box context map - same node.
bpf_dbg_printk("Looking up traceparent for connection info");
tp_info_pid_t *tp_p = trace_info_for_connection(&conn);
tp_info_pid_t *tp_p = trace_info_for_connection(&conn, TRACE_TYPE_CLIENT);
if (!disable_black_box_cp && tp_p) {
if (correlated_request_with_current(tp_p)) {
bpf_dbg_printk("Found traceparent from trace map, another process.");
Expand Down
2 changes: 1 addition & 1 deletion bpf/go_nethttp.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1107,7 +1107,7 @@ int uprobe_persistConnRoundTrip(struct pt_regs *ctx) {
// Must sort the connection info, this map is shared with kprobes which use sorted connection
// info always.
sort_connection_info(&conn);
set_trace_info_for_connection(&conn, &tp_p);
set_trace_info_for_connection(&conn, TRACE_TYPE_CLIENT, &tp_p);

// Setup information for the TC context propagation.
// We need the PID id to be able to query ongoing_http and update
Expand Down
1 change: 1 addition & 0 deletions bpf/http_types.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ typedef struct tp_info_pid {
tp_info_t tp;
u32 pid;
u8 valid;
u8 req_type;
} tp_info_pid_t;

// Here we keep the information that is sent on the ring buffer
Expand Down
7 changes: 4 additions & 3 deletions bpf/k_tracer.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -605,7 +605,7 @@ int socket__http_filter(struct __sk_buff *skb) {
partial->tcp_seq = tcp.seq;
__builtin_memcpy(partial->s_addr, conn.s_addr, sizeof(partial->s_addr));

tp_info_pid_t *trace_info = trace_info_for_connection(&conn);
tp_info_pid_t *trace_info = trace_info_for_connection(&conn, TRACE_TYPE_CLIENT);
if (trace_info) {
if (cookie) { // we have an actual socket associated
bpf_map_update_elem(&tcp_connection_map, partial, &conn, BPF_ANY);
Expand All @@ -614,14 +614,15 @@ int socket__http_filter(struct __sk_buff *skb) {
connection_info_t *prev_conn = bpf_map_lookup_elem(&tcp_connection_map, partial);

if (prev_conn) {
tp_info_pid_t *trace_info = trace_info_for_connection(prev_conn);
tp_info_pid_t *trace_info =
trace_info_for_connection(prev_conn, TRACE_TYPE_CLIENT);
if (trace_info) {
if (current_immediate_epoch(trace_info->tp.ts) ==
current_immediate_epoch(bpf_ktime_get_ns())) {
//bpf_dbg_printk("Found trace info on another interface, setting it up for this connection");
tp_info_pid_t other_info = {0};
__builtin_memcpy(&other_info, trace_info, sizeof(tp_info_pid_t));
set_trace_info_for_connection(&conn, &other_info);
set_trace_info_for_connection(&conn, TRACE_TYPE_CLIENT, &other_info);
}
}
}
Expand Down
3 changes: 2 additions & 1 deletion bpf/k_tracer_defs.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,8 @@ static __always_inline void handle_buf_with_args(void *ctx, call_protocol_args_t
tp_info_pid_t *existing = bpf_map_lookup_elem(&server_traces, &t_key);
if (existing) {
__builtin_memcpy(&existing->tp, &info->tp, sizeof(tp_info_t));
set_trace_info_for_connection(&args->pid_conn.conn, existing);
set_trace_info_for_connection(
&args->pid_conn.conn, TRACE_TYPE_SERVER, existing);
} else {
bpf_dbg_printk("Didn't find existing trace, this might be a bug!");
}
Expand Down
119 changes: 112 additions & 7 deletions bpf/protocol_http.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,110 @@ static __always_inline http_info_t *empty_http_info() {
return value;
}

static __always_inline u32 trace_type_from_meta(http_connection_metadata_t *meta) {
if (meta->type == EVENT_HTTP_CLIENT) {
return TRACE_TYPE_CLIENT;
}

return TRACE_TYPE_SERVER;
}

static __always_inline void http_get_or_create_trace_info(http_connection_metadata_t *meta,
u32 pid,
connection_info_t *conn,
void *u_buf,
int bytes_len,
s32 capture_header_buffer) {
tp_info_pid_t *tp_p = tp_buf();

if (!tp_p) {
return;
}

tp_p->tp.ts = bpf_ktime_get_ns();
tp_p->tp.flags = 1;
tp_p->valid = 1;
tp_p->pid = pid; // used for avoiding finding stale server requests with client port reuse
tp_p->req_type = (meta) ? meta->type : 0;

urand_bytes(tp_p->tp.span_id, SPAN_ID_SIZE_BYTES);

u8 found_tp = 0;

if (meta) {
if (meta->type == EVENT_HTTP_CLIENT) {
pid_connection_info_t p_conn = {.pid = pid};
__builtin_memcpy(&p_conn.conn, conn, sizeof(connection_info_t));
found_tp = find_trace_for_client_request(&p_conn, &tp_p->tp);
} else {
//bpf_dbg_printk("Looking up existing trace for connection");
//dbg_print_http_connection_info(conn);
Comment on lines +73 to +74
Copy link
Contributor

Choose a reason for hiding this comment

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

Just double checking you intended to leave those in.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah it's good for the future if we need to debug the code.


// For server requests, we first look for TCP info (setup by TC ingress) and then we fall back to black-box info.
found_tp = find_trace_for_server_request(conn, &tp_p->tp);
}
}

if (!found_tp) {
bpf_dbg_printk("Generating new traceparent id");
new_trace_id(&tp_p->tp);
__builtin_memset(tp_p->tp.parent_id, 0, sizeof(tp_p->tp.span_id));
} else {
bpf_dbg_printk("Using old traceparent id");
}

//unsigned char tp_buf[TP_MAX_VAL_LENGTH];
//make_tp_string(tp_buf, &tp_p->tp);
//bpf_dbg_printk("tp: %s", tp_buf);
Comment on lines +89 to +91
Copy link
Contributor

Choose a reason for hiding this comment

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

And these

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same, that's expensive but when I debug traceparent issues, I keep needing to write the code again.


#ifdef BPF_TRACEPARENT
Copy link
Contributor

Choose a reason for hiding this comment

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

Since we are moving this around, may I suggest the following?

// at the top of this file or some common header

#ifdef BPF_TRACEPARENT
enum { PARSE_TRACEPARENT = 1 };
#else
enum { PARSE_TRACEPARENT = 0 };
#endif 

// then here

if (PARSE_TRACEPARENT) { ... }

the rationale is that this ensures this code is always sieved through the compiler, thus allowing it to catch any compilation error even when the macro is used. The code will not be present in the final binary when PARSE_TRACEPARENT is 0 because the compiler will optimise it out anyway, so best of two worlds and less error-prone.

Another unrelated to the above thought: should this happen before the find_trace_for_xxx(...) code? This snippet seems to run irrespective of found_tp, so perhaps moving it before and bailing earlier if tp is found makes sense?

Or would it even make sense to only run this if found_tp == false, or is this some sort of unconditional fallback?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think the compilation is an issue, since we built all versions when compiling. Also the found_tp is not considered, because if we find something encoded in headers we don't care if we found something else before.

// The below buffer scan can be expensive on high volume of requests. We make it optional
// for customers to enable it. Off by default.
if (!capture_header_buffer) {
if (meta) {
u32 type = trace_type_from_meta(meta);
set_trace_info_for_connection(conn, type, tp_p);
server_or_client_trace(meta->type, conn, tp_p);
}
return;
}

unsigned char *buf = tp_char_buf();
if (buf) {
int buf_len = bytes_len;
bpf_clamp_umax(buf_len, TRACE_BUF_SIZE - 1);

bpf_probe_read(buf, buf_len, u_buf);
unsigned char *res = bpf_strstr_tp_loop(buf, buf_len);

if (res) {
bpf_dbg_printk("Found traceparent %s", res);
unsigned char *t_id = extract_trace_id(res);
unsigned char *s_id = extract_span_id(res);
unsigned char *f_id = extract_flags(res);

decode_hex(tp_p->tp.trace_id, t_id, TRACE_ID_CHAR_LEN);
decode_hex((unsigned char *)&tp_p->tp.flags, f_id, FLAGS_CHAR_LEN);
if (meta && meta->type == EVENT_HTTP_CLIENT) {
decode_hex(tp_p->tp.span_id, s_id, SPAN_ID_CHAR_LEN);
} else {
decode_hex(tp_p->tp.parent_id, s_id, SPAN_ID_CHAR_LEN);
}
} else {
bpf_dbg_printk("No traceparent, making a new trace_id", res);
}
} else {
return;
}
#endif

if (meta) {
u32 type = trace_type_from_meta(meta);
set_trace_info_for_connection(conn, type, tp_p);
server_or_client_trace(meta->type, conn, tp_p);
}
}

static __always_inline u8 is_http(const unsigned char *p, u32 len, u8 *packet_type) {
if (len < MIN_HTTP_SIZE) {
return 0;
Expand Down Expand Up @@ -250,15 +354,16 @@ int protocol_http(void *ctx) {
http_connection_metadata_t *meta =
connection_meta_by_direction(&args->pid_conn, args->direction, PACKET_TYPE_REQUEST);

get_or_create_trace_info(meta,
args->pid_conn.pid,
&args->pid_conn.conn,
(void *)args->u_buf,
args->bytes_len,
capture_header_buffer);
http_get_or_create_trace_info(meta,
args->pid_conn.pid,
&args->pid_conn.conn,
(void *)args->u_buf,
args->bytes_len,
capture_header_buffer);

if (meta) {
tp_info_pid_t *tp_p = trace_info_for_connection(&args->pid_conn.conn);
u32 type = trace_type_from_meta(meta);
tp_info_pid_t *tp_p = trace_info_for_connection(&args->pid_conn.conn, type);
if (tp_p) {
info->tp = tp_p->tp;
} else {
Expand Down
65 changes: 56 additions & 9 deletions bpf/protocol_tcp.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,56 @@ static __always_inline tcp_req_t *empty_tcp_req() {
return value;
}

static __always_inline void init_new_trace(tp_info_t *tp) {
new_trace_id(tp);
__builtin_memset(tp->parent_id, 0, sizeof(tp->span_id));
}

static __always_inline void
set_tcp_trace_info(u32 type, connection_info_t *conn, tp_info_t *tp, u32 pid) {
tp_info_pid_t *tp_p = tp_buf();

if (!tp_p) {
return;
}

tp_p->tp = *tp;
tp_p->tp.flags = 1;
tp_p->valid = 1;
tp_p->pid = pid; // used for avoiding finding stale server requests with client port reuse
tp_p->req_type = EVENT_TCP_REQUEST;

set_trace_info_for_connection(conn, type, tp_p);
bpf_dbg_printk("Set traceinfo for conn");
dbg_print_http_connection_info(conn);

server_or_client_trace(type, conn, tp_p);
}

static __always_inline void tcp_get_or_set_trace_info(tcp_req_t *req,
pid_connection_info_t *pid_conn) {
if (req->direction == TCP_SEND) { // Client
u8 found = find_trace_for_client_request(pid_conn, &req->tp);
bpf_dbg_printk("Looking up client trace info, found %d", found);
if (found) {
urand_bytes(req->tp.span_id, SPAN_ID_SIZE_BYTES);
} else {
init_new_trace(&req->tp);
}

set_tcp_trace_info(TRACE_TYPE_CLIENT, &pid_conn->conn, &req->tp, pid_conn->pid);
} else { // Server
u8 found = find_trace_for_server_request(&pid_conn->conn, &req->tp);
bpf_dbg_printk("Looking up server trace info, found %d", found);
if (found) {
urand_bytes(req->tp.span_id, SPAN_ID_SIZE_BYTES);
} else {
init_new_trace(&req->tp);
}
set_tcp_trace_info(TRACE_TYPE_SERVER, &pid_conn->conn, &req->tp, pid_conn->pid);
}
}

static __always_inline void handle_unknown_tcp_connection(pid_connection_info_t *pid_conn,
void *u_buf,
int bytes_len,
Expand All @@ -54,16 +104,11 @@ static __always_inline void handle_unknown_tcp_connection(pid_connection_info_t
task_pid(&req->pid);
bpf_probe_read(req->buf, K_TCP_MAX_LEN, u_buf);

tp_info_pid_t *server_tp = find_parent_trace(pid_conn);
req->tp.ts = bpf_ktime_get_ns();

if (server_tp && server_tp->valid && valid_trace(server_tp->tp.trace_id)) {
bpf_dbg_printk("Found existing server tp for client call");
__builtin_memcpy(
req->tp.trace_id, server_tp->tp.trace_id, sizeof(req->tp.trace_id));
__builtin_memcpy(
req->tp.parent_id, server_tp->tp.span_id, sizeof(req->tp.parent_id));
urand_bytes(req->tp.span_id, SPAN_ID_SIZE_BYTES);
}
bpf_dbg_printk("TCP request start, direction = %d, ssl = %d", direction, ssl);

tcp_get_or_set_trace_info(req, pid_conn);

bpf_map_update_elem(&ongoing_tcp_req, pid_conn, req, BPF_ANY);
}
Expand All @@ -90,6 +135,8 @@ static __always_inline void handle_unknown_tcp_connection(pid_connection_info_t
bpf_clamp_umax(off, (K_TCP_MAX_LEN / 2));
bpf_probe_read(existing->buf + off, (K_TCP_MAX_LEN / 2), u_buf);
existing->len += bytes_len;
} else {
existing->len += bytes_len;
}
}

Expand Down
Loading
Loading