Enhance log module and enhance address resolver in access log module

.gitignore

@@ -14,4 +14,6 @@ pkg/**/bpf_*.go
 
 pkg/tools/btf/files/**/*.btf
 
-.config
+.config.omc/
+.omc/
+.config/

.golangci.yml

@@ -78,6 +78,17 @@ linters:
       - third_party$
       - builtin$
       - examples$
+    rules:
+      # test code is held to looser style rules: table-driven tests naturally repeat
+      # literal fixture values, use wide data rows, and keep fixed-argument helpers for
+      # readability, which the style linters below would otherwise flag.
+      - path: _test\.go
+        linters:
+          - goconst
+          - gocyclo
+          - lll
+          - unparam
+          - dupl
 formatters:
   enable:
     - gofmt

CHANGES.md

@@ -32,11 +32,19 @@ Release Notes.
 * Aggregate dropped perf event sample warnings periodically instead of logging every burst.
 * Lower the log level for connection query errors of short-lived exited processes.
 * Enable pprof by default but bind it to 127.0.0.1 so it is not exposed on the network.
+* Support collecting the ztunnel outbound connection mappings from the ztunnel admin config dump, as a symbol-independent fallback of the uprobe based events in the access log module.
+* Support cross-checking the ztunnel proxied connection count from the ztunnel prometheus metrics in the access log module.
+* Report a periodic info-level summary of un-resolved remote addresses, including the conntrack and ztunnel correlation statistics, in the access log module.
+* Attach all matched ztunnel `track_outbound` symbol copies when attaching the uprobe in the access log module.
+* Add success/failure counters to the conntrack real peer address queries.
 
 #### Bug Fixes
 * Fix the base image cannot run in the arm64.
 * Fix process fork tracepoint reporting thread TID instead of process TGID, causing repeated process detected/dead churn.
 * Fix panic in the access log module when handling HTTP/2 streams without a body.
+* Fix the ztunnel event reader and process finder not being registered when the ztunnel process starts after the rover.
+* Fix ghost connections created by failed accept syscalls with a negative socket fd in the access log module.
+* Remove the ineffective `ctnetlink_fill_info` kprobe and fix an always-true condition in the conntrack BPF program.
 
 #### Documentation
 * Add a dead link checker in the CI.

bpf/accesslog/ambient/ztunnel.c

@@ -41,12 +41,64 @@ int connection_manager_track_outbound(struct pt_regs* ctx) {
         return 0;
     }
     bool success = true;
-    success = get_socket_addr_ip_in_ztunnel(success, (void *)PT_REGS_PARM3(ctx), &event->orginal_src_ip, &event->src_port);
-    success = get_socket_addr_ip_in_ztunnel(success, (void *)PT_REGS_PARM4(ctx), &event->original_dst_ip, &event->dst_port);
-    success = get_socket_addr_ip_in_ztunnel(success, (void *)PT_REGS_PARM5(ctx), &event->lb_dst_ip, &event->lb_dst_port);
+    // track_outbound(&self, src, original_dst, actual_dst) returns a large ConnectionResult
+    // struct via a hidden sret pointer. On x86-64 SysV the sret pointer occupies the first
+    // integer arg register(PARM1), so &self is PARM2 and the three SocketAddr args are PARM3/4/5.
+    // On AArch64 AAPCS64 the sret pointer is passed in x8, which is OUTSIDE the PARM1..8 arg
+    // registers, so the arguments are NOT shifted: &self is PARM1 and the args are PARM2/3/4.
+    // original_dst is the service ClusterIP, actual_dst is the load balanced real pod.
+#if defined(bpf_target_x86)
+    void *src_arg = (void *)PT_REGS_PARM3(ctx);
+    void *original_dst_arg = (void *)PT_REGS_PARM4(ctx);
+    void *actual_dst_arg = (void *)PT_REGS_PARM5(ctx);
+#else
+    void *src_arg = (void *)PT_REGS_PARM2(ctx);
+    void *original_dst_arg = (void *)PT_REGS_PARM3(ctx);
+    void *actual_dst_arg = (void *)PT_REGS_PARM4(ctx);
+#endif
+    success = get_socket_addr_ip_in_ztunnel(success, src_arg, &event->orginal_src_ip, &event->src_port);
+    success = get_socket_addr_ip_in_ztunnel(success, original_dst_arg, &event->original_dst_ip, &event->dst_port);
+    success = get_socket_addr_ip_in_ztunnel(success, actual_dst_arg, &event->lb_dst_ip, &event->lb_dst_port);
     if (!success) {
         return 0;
     }
     bpf_perf_event_output(ctx, &ztunnel_lb_socket_mapping_event_queue, BPF_F_CURRENT_CPU, event, sizeof(*event));
     return 0;
 }
+
+// ConnectionResult::new(src: SocketAddr, dst: SocketAddr, hbone_target, ...) is an
+// associated function(no &self) that ztunnel constructs UNCONDITIONALLY for every proxied
+// connection - including the outbound legs that skip track_outbound through an early-return
+// in proxy_to - so it is a strictly-higher-coverage, log-level-independent source(the same
+// data ztunnel would print as the "connection complete"/"connection opened" access log, but
+// captured at construction time regardless of the log level). It returns a large struct via
+// the hidden sret pointer, which on x86-64 occupies PARM1 and shifts the arguments by one
+// (src=PARM2, dst=PARM3); on AArch64 the sret pointer is in x8(not a PARM) so the arguments
+// are not shifted(src=PARM1, dst=PARM2). src is the downstream app addr and dst is the REAL
+// backend pod addr. There is no service ClusterIP among the arguments, so this mapping is
+// keyed by the source address alone in user space(the app's ephemeral src port is unique per
+// connection). original_dst_ip is left zero to mark this event as a "src-only" mapping.
+SEC("uprobe/connection_result_new")
+int connection_result_new(struct pt_regs* ctx) {
+    struct ztunnel_socket_mapping_t *event = create_ztunnel_socket_mapping_event();
+    if (event == NULL) {
+        return 0;
+    }
+    bool success = true;
+#if defined(bpf_target_x86)
+    void *src_arg = (void *)PT_REGS_PARM2(ctx);
+    void *dst_arg = (void *)PT_REGS_PARM3(ctx);
+#else
+    void *src_arg = (void *)PT_REGS_PARM1(ctx);
+    void *dst_arg = (void *)PT_REGS_PARM2(ctx);
+#endif
+    success = get_socket_addr_ip_in_ztunnel(success, src_arg, &event->orginal_src_ip, &event->src_port);
+    success = get_socket_addr_ip_in_ztunnel(success, dst_arg, &event->lb_dst_ip, &event->lb_dst_port);
+    if (!success) {
+        return 0;
+    }
+    event->original_dst_ip = 0;
+    event->dst_port = 0;
+    bpf_perf_event_output(ctx, &ztunnel_lb_socket_mapping_event_queue, BPF_F_CURRENT_CPU, event, sizeof(*event));
+    return 0;
+}

bpf/accesslog/syscalls/connect_conntrack.c

@@ -93,7 +93,7 @@ static __always_inline int nf_conn_aware(struct pt_regs* ctx, struct nf_conn *ct
     }
 
     // already contains the remote address
-    if (connect_args->has_remote && &(connect_args->remote) != NULL) {
+    if (connect_args->has_remote) {
         return 0;
     }
 
@@ -142,7 +142,3 @@ int nf_confirm(struct pt_regs* ctx) {
     return nf_conn_aware(ctx, (struct nf_conn*)PT_REGS_PARM3(ctx));
 }
 
-SEC("kprobe/ctnetlink_fill_info")
-int nf_ctnetlink_fill_info(struct pt_regs* ctx) {
-    return nf_conn_aware(ctx, (struct nf_conn*)PT_REGS_PARM5(ctx));
-}

configs/rover_configs.yaml

@@ -18,6 +18,11 @@
 logger:
   # The lowest level of printing allowed.
   level: ${ROVER_LOGGER_LEVEL:INFO}
+  # Comma separated list of modules(name prefix) to elevate to the debug level
+  # regardless of the level above, e.g. "accesslog.collector.ztunnel". Keeps the
+  # high volume modules quiet(bounding the logging allocation) while still getting
+  # debug detail for the modules under investigation.
+  debug_modules: ${ROVER_LOGGER_DEBUG_MODULES:}
 
 core:
   # The name of the cluster.

pkg/accesslog/collector/collector.go

@@ -19,12 +19,9 @@ package collector
 
 import (
 	"github.com/apache/skywalking-rover/pkg/accesslog/common"
-	"github.com/apache/skywalking-rover/pkg/logger"
 	"github.com/apache/skywalking-rover/pkg/module"
 )
 
-var log = logger.GetLogger("accesslog", "collector")
-
 type Collector interface {
 	Start(mgr *module.Manager, context *common.AccessLogContext) error
 	Stop()

pkg/accesslog/collector/connection.go

@@ -125,9 +125,6 @@ func (c *ConnectCollector) Start(m *module.Manager, ctx *common.AccessLogContext
 	_ = ctx.BPF.AddLinkOrError(link.Kprobe, map[string]*ebpf.Program{
 		"nf_confirm": ctx.BPF.NfConfirm,
 	})
-	_ = ctx.BPF.AddLinkOrError(link.Kprobe, map[string]*ebpf.Program{
-		"ctnetlink_fill_info": ctx.BPF.NfCtnetlinkFillInfo,
-	})
 	return nil
 }
 
@@ -159,6 +156,13 @@ func (c *ConnectionPartitionContext) Consume(data interface{}) {
 			"pid: %d, fd: %d, role: %s: func: %s, family: %d, success: %d, conntrack exist: %t",
 			event.ConID, event.RandomID, event.PID, event.SocketFD, enums.ConnectionRole(event.Role), enums.SocketFunctionName(event.FuncName),
 			event.SocketFamily, event.ConnectSuccess, event.ConnTrackUpstreamPort != 0)
+		// a negative fd means the syscall failed(e.g. non-blocking accept returns -EAGAIN),
+		// there is no real connection behind it, so ignore the event
+		if int32(event.SocketFD) < 0 {
+			connectionLogger.Debugf("ignore the connect event with negative socket fd, connection ID: %d, randomID: %d, "+
+				"pid: %d, fd: %d", event.ConID, event.RandomID, event.PID, int32(event.SocketFD))
+			return
+		}
 		socketPair := c.BuildSocketFromConnectEvent(event)
 		if socketPair == nil {
 			connectionLogger.Debugf("cannot found the socket paire from connect event, connection ID: %d, randomID: %d",
@@ -271,6 +275,9 @@ func (c *ConnectionPartitionContext) BuildSocketPair(event *events.SocketConnect
 			if !ip.ShouldIgnoreConntrack(remoteAddr, conntrackIP, uint16(event.ConnTrackUpstreamPort)) {
 				result.DestIP = conntrackIP
 				result.DestPort = uint16(event.ConnTrackUpstreamPort)
+				// the BPF conntrack path rewrote the dest to the real peer; mark it so the
+				// resolve summary counts it as conntrack-resolved instead of unresolved
+				result.ConnTrackResolved = true
 				ignoredConntrack = false
 			}
 
@@ -308,6 +315,9 @@ func (c *ConnectionPartitionContext) BuildSocketPair(event *events.SocketConnect
 			if !ip.ShouldIgnoreConntrack(remoteAddr, conntrackIP, uint16(event.ConnTrackUpstreamPort)) {
 				result.DestIP = conntrackIP
 				result.DestPort = uint16(event.ConnTrackUpstreamPort)
+				// the BPF conntrack path rewrote the dest to the real peer; mark it so the
+				// resolve summary counts it as conntrack-resolved instead of unresolved
+				result.ConnTrackResolved = true
 				ignoredConntrack = false
 			}