Update to match db semconv #849
Conversation
| u8 p_type; | ||
| u8 dns_q; | ||
| u8 _pad1[1]; | ||
| u8 _pad1[2]; |
There was a problem hiding this comment.
I removed an unused field.
| // with other instrumented processes | ||
| pid_info pid; | ||
| unsigned char buf[k_dns_max_len]; | ||
| u8 _pad3[4]; |
There was a problem hiding this comment.
I had to clamp_max a size for the BPF buffer, so I needed a power of 2. I've switched to 512 and I've added a padding.
| bpf_clamp_umax(len, 512); | ||
| populate_dns_record(req, &p_conn, orig_dport, len, qr, hdr.id, conn_pid); | ||
|
|
||
| read_skb_bytes(skb, dns_off, req->buf, len); |
There was a problem hiding this comment.
This was a nasty bug. We were reading up to the max of req->buf, but read_skb_bytes failed to grab the last chunk of 16 bytes since we weren't specifically reading exactly as we should. This left us with garbage at the end.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #849 +/- ##
==========================================
- Coverage 55.42% 55.37% -0.05%
==========================================
Files 251 251
Lines 21400 21443 +43
==========================================
+ Hits 11861 11875 +14
- Misses 8731 8757 +26
- Partials 808 811 +3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| pid_connection_info_t *p_conn, | ||
| u16 orig_dport) { | ||
|
|
||
| if (size < sizeof(struct dnshdr)) { |
There was a problem hiding this comment.
I had to add an additional way to read the DNS traffic since if OBI is configured with host network, it may not be able to read some internal pod traffic, in this case it wasn't able to see the docker internal DNS traffic between the docker network and the service itself. I reproduced it in our test examples, with setting network_mode: host for OBI.
In this scenario, the socket_filter doesn't see the traffic, but the udp_sendmsg and sock_recvmsg kprobes fire. I added a userspace buffer helper here so we can capture those DNS requests too.
If OBI is running in the network of the target process, we'll see the event twice, once from the sock_filter another time by the kprobes. This isn't a problem because we deduplicate the DNS requests in user space and won't let them be emitted twice.
| return &InMemory{ | ||
| entries: map[string][]string{}, | ||
| func NewInMemory(cacheSize int) (*InMemory, error) { | ||
| cache, err := simplelru.NewLRU[string, []string](cacheSize, nil) |
There was a problem hiding this comment.
I changed this implementation to ensure this cache is memory capped.
3 participants
This PR fixes the DB and messaging semantic conventions for metrics to include the
server.addressfield. We are currently generating it as an IP address, so I used the new DNS code to implement an eBPF based RDNS for application metrics. I've only tested this in one scenario, the OATS SQL test, and we probably want to do more testing before we enable it by default.I'm leaving couple of comments in the PR code to explain some of the changes.
Closes #753