-
Notifications
You must be signed in to change notification settings - Fork 111
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
Propagate HTTP trace context in Go #491
Changes from all commits
c838139
1b4bc61
3f0fa38
45a68fb
fb84a58
827fe36
51bf0a1
4ae2488
2ce9f41
d57d21c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,8 +1,6 @@ | ||
#ifndef TRACE_UTIL_H | ||
#define TRACE_UTIL_H | ||
|
||
#include "utils.h" | ||
|
||
// 55+13 | ||
#define TRACE_PARENT_HEADER_LEN 68 | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,18 +1,41 @@ | ||
#ifndef TRACING_H | ||
#define TRACING_H | ||
#include "vmlinux.h" | ||
#include "trace_util.h" | ||
|
||
#define TRACE_ID_SIZE_BYTES 16 | ||
#define SPAN_ID_SIZE_BYTES 8 | ||
#define FLAGS_SIZE_BYTES 1 | ||
#define TRACE_ID_CHAR_LEN 32 | ||
#define SPAN_ID_CHAR_LEN 16 | ||
#define FLAGS_CHAR_LEN 2 | ||
#define TP_MAX_VAL_LENGTH 55 | ||
#define TP_MAX_KEY_LENGTH 11 | ||
|
||
typedef struct tp_info { | ||
unsigned char trace_id[TRACE_ID_SIZE_BYTES]; | ||
unsigned char span_id[SPAN_ID_SIZE_BYTES]; | ||
unsigned char parent_id[SPAN_ID_SIZE_BYTES]; | ||
u64 epoch; | ||
u8 flags; | ||
} tp_info_t; | ||
|
||
static __always_inline void make_tp_string(unsigned char *buf, tp_info_t *tp) { | ||
// Version | ||
*buf++ = '0'; *buf++ = '0'; *buf++ = '-'; | ||
|
||
// TraceID | ||
encode_hex(buf, tp->trace_id, TRACE_ID_SIZE_BYTES); | ||
buf += TRACE_ID_CHAR_LEN; | ||
*buf++ = '-'; | ||
|
||
// SpanID | ||
encode_hex(buf, tp->span_id, SPAN_ID_SIZE_BYTES); | ||
buf += SPAN_ID_CHAR_LEN; | ||
*buf++ = '-'; | ||
|
||
// Flags | ||
*buf++ = '0'; *buf = (tp->flags == 0) ? '0' : '1'; | ||
} | ||
|
||
#endif |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -27,6 +27,10 @@ | |
], | ||
"context.valueCtx": [ | ||
"val" | ||
], | ||
"bufio.Writer": [ | ||
"buf", | ||
"n" | ||
] | ||
} | ||
}, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -86,6 +86,7 @@ services: | |
- --config=/configs/beyla-config.yml | ||
volumes: | ||
- ./configs/:/configs | ||
- ./system/sys/kernel/security:/sys/kernel/security | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Will we have to document this e.g. to allow running Beyla in Kubernetes? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh this is a very good point. I need to document this. As it stands now, if security isn't there we'll assume we can propagate the context, since the security file is missing. But it could be because the users didn't mount it and the host machine doesn't allow it. I'll follow-up with a PR on this. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I opened an issue so I don't forget #502 |
||
container_name: demo-javabeyla | ||
privileged: true | ||
network_mode: "service:javatestserver" | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
none [integrity] confidentiality |
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.
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.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What happens if we don't do it now? Is the feature incomplete?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see it as incomplete in terms of Beyla instrumentation, but it would be nice to make Beyla work well for instrumented applications with the Go SDK. As it stands now, Beyla will correctly propagate the context, but if there's SDK instrumentation as well, the two spans will look parallel, just as it was before. If we overwrite the header value, we'll manage to nest them. So it's an extension to the feature to make it work with auto and manual instrumentation.