add PROVIDER prefix to each JSON events in h2olog quic
#22
Conversation
h2olog
Outdated
| @@ -93,7 +93,11 @@ quic_bpf = """ | |||
| #define MAX_HEADER_VALUE_LEN 128 | |||
| #define TOKEN_PREVIEW_LEN 8 | |||
|
|
|||
| int sprintf(char * restrict str, const char * restrict format, ...); | |||
| #include <linux/kernel.h> /* sprintf */ | |||
There was a problem hiding this comment.
Including the entire kernel header feels like overkill. If you insist on keeping it, my suggestion is to remove the comment.
There was a problem hiding this comment.
I see. I think including it is necessary when we try some other functions (e.g. snprintf, which doesn't compile, though).
So I've just removed it (8a8c600de5dc1d9a7f4b83269fddc80ebd3dd045).
h2olog
Outdated
| u.enable_probe(probe="quictrace_recv_ack_delay", fn_name="trace_quictrace_recv_ack_delay") | ||
| u.enable_probe(probe="quictrace_lost", fn_name="trace_quictrace_lost") | ||
| u.enable_probe(probe="send_response_header", fn_name="trace_send_response_header") | ||
| # NOTE: fully-specified probe name (e.g. "quicly:accept") is NOT available yet. |
There was a problem hiding this comment.
Ditto. I think this comment doesn't provide value within the source code. My suggestion is to instead open an issue as a future task.
|
Thank you! this work is great. It's great the you picked up #6 as well. Left couple of comments but other than that, I think we're looking good. |
| #include <linux/kernel.h> | ||
|
|
||
| #define INIT_EVENT_NAME(event) do { \\ | ||
| sprintf((event).type, __func__ + (sizeof("trace_") -1)); \\ |
There was a problem hiding this comment.
As a convention, strcpy should be used for copying strings. Use of sprintf is discouraged as the function expects more argument depending on the format being given (and therefore providing a variable format is not considered a good idea).
I also think that we have buffer overflow here. I presume that the longest string given as the second argument would be "quicly__stream_data_blocked_receive" which is 35 bytes long, whereas (event).type is 32 bytes long.
This is a part of #21 by namespacing event names. It also indicates we can improve the type filter to specify glob-patterns in the future, like
h2olog firehose -t "quicly:*".This PR also resolves #6.