Add dimensional metrics to record applications that violate tls traci…

…ng assumptions (mismatched fds) (#1270)

Summary: Add dimensional metrics to record applications that violate tls
tracing assumptions (mismatched fds)

Relevant Issues: #692

Type of change: /kind feature

Test Plan: Verified that the metrics added do not clash with the non
dimensional metrics. Please see
e83443a
for the code that was used to produce this output
([P363](https://phab.corp.pixielabs.ai/P363))

---------

Signed-off-by: Dom Del Nano <ddelnano@pixielabs.ai>

src/common/metrics/metrics.h

@@ -30,6 +30,13 @@ prometheus::Registry& GetMetricsRegistry();
 // This function should only be called by testing code.
 void TestOnlyResetMetricsRegistry();
 
+// A convenience wrapper to return a counter with the specified name and help message when
+// dimensions aren't known at compile time. This should only be used when dimensional data is
+// very low cardinality.
+inline auto& BuildCounterFamily(const std::string& name, const std::string& help_message) {
+  return prometheus::BuildCounter().Name(name).Help(help_message).Register(GetMetricsRegistry());
+}
+
 // A convenience wrapper to return a counter with the specified name and help message.
 inline auto& BuildCounter(const std::string& name, const std::string& help_message) {
   return prometheus::BuildCounter()

src/stirling/source_connectors/socket_tracer/bcc_bpf/openssl_trace.c

@@ -34,6 +34,7 @@
 BPF_HASH(openssl_symaddrs_map, uint32_t, struct openssl_symaddrs_t);
 
 BPF_ARRAY(openssl_trace_state, int, 1);
+BPF_HASH(openssl_trace_state_debug, uint32_t, struct openssl_trace_state_debug_t);
 
 /***********************************************************
  * General helpers
@@ -120,9 +121,15 @@ static __inline int get_fd_and_eval_nested_syscall_detection(uint64_t pid_tgid)
   bool mismatched_fds = nested_syscall_fd_ptr->mismatched_fds;
 
   if (mismatched_fds) {
+    struct openssl_trace_state_debug_t debug = {};
+    bpf_get_current_comm(&debug.comm, sizeof(debug.comm));
+
     int error_status_idx = kOpenSSLTraceStatusIdx;
     int status_code = kOpenSSLMismatchedFDsDetected;
+    uint32_t tgid = pid_tgid >> 32;
+
     openssl_trace_state.update(&error_status_idx, &status_code);
+    openssl_trace_state_debug.update(&tgid, &debug);
   }
 
   int fd = nested_syscall_fd_ptr->fd;

src/stirling/source_connectors/socket_tracer/bcc_bpf_intf/symaddrs.h

@@ -263,6 +263,12 @@ struct openssl_symaddrs_t {
   int32_t RBIO_num_offset;  // 0x30 (openssl 1.1.1) or 0x28 (openssl 1.1.0)
 };
 
+#define MAX_CMD_SIZE 32
+
+struct openssl_trace_state_debug_t {
+  char comm[MAX_CMD_SIZE];
+};
+
 // For reading file descriptor from a TLSWrap pointer.
 struct node_tlswrap_symaddrs_t {
   // Offset of StreamListener base class of TLSWrap class.

src/stirling/source_connectors/socket_tracer/socket_trace_connector.cc

@@ -170,13 +170,16 @@ constexpr size_t kMaxHTTPHeadersBytes = 8192;
 // Protobuf printer will limit strings to this length.
 constexpr size_t kMaxPBStringLen = 64;
 
+constexpr char openssl_mismatched_fds_metric[] = "openssl_trace_mismatched_fds";
+constexpr char openssl_mismatched_fds_help[] =
+    "Count of the times a syscall's fd was mismatched when detecting fds from an active user space "
+    "call";
+
 SocketTraceConnector::SocketTraceConnector(std::string_view source_name)
     : SourceConnector(source_name, kTables),
       conn_stats_(&conn_trackers_mgr_),
-      openssl_trace_mismatched_fds_counter_(BuildCounter(
-          "openssl_trace_mismatched_fds",
-          "Count of the times a syscall's fd was mismatched when detecting fds from an "
-          "active user space call")),
+      openssl_trace_mismatched_fds_counter_family_(
+          BuildCounterFamily(openssl_mismatched_fds_metric, openssl_mismatched_fds_help)),
       uprobe_mgr_(this) {
   proc_parser_ = std::make_unique<system::ProcParser>();
   InitProtocolTransferSpecs();
@@ -495,6 +498,9 @@ Status SocketTraceConnector::InitImpl() {
 
   openssl_trace_state_ =
       std::make_unique<ebpf::BPFArrayTable<int>>(GetArrayTable<int>("openssl_trace_state"));
+  openssl_trace_state_debug_ =
+      std::make_unique<ebpf::BPFHashTable<uint32_t, struct openssl_trace_state_debug_t>>(
+          GetHashTable<uint32_t, openssl_trace_state_debug_t>("openssl_trace_state_debug"));
 
   return Status::OK();
 }
@@ -648,7 +654,18 @@ void SocketTraceConnector::CheckTracerState() {
   openssl_trace_state_->get_value(kOpenSSLTraceStatusIdx, error_code);
 
   if (error_code == kOpenSSLMismatchedFDsDetected) {
-    openssl_trace_mismatched_fds_counter_.Increment();
+    openssl_trace_mismatched_fds_counter_family_.Add({{"name", openssl_mismatched_fds_metric}})
+        .Increment();
+
+    // Record the offending applications and clear the BPF hash in the process.
+    auto table = openssl_trace_state_debug_->get_table_offline(true);
+    for (auto& entry : table) {
+      struct openssl_trace_state_debug_t debug = std::get<1>(entry);
+
+      openssl_trace_mismatched_fds_counter_family_
+          .Add({{"name", openssl_mismatched_fds_metric}, {"exe", debug.comm}})
+          .Increment();
+    }
   }
   DCHECK_EQ(error_code, kOpenSSLTraceOk);
 

src/stirling/source_connectors/socket_tracer/socket_trace_connector.h

@@ -232,7 +232,9 @@ class SocketTraceConnector : public SourceConnector, public bpf_tools::BCCWrappe
   ConnStats conn_stats_;
 
   std::unique_ptr<ebpf::BPFArrayTable<int>> openssl_trace_state_;
-  prometheus::Counter& openssl_trace_mismatched_fds_counter_;
+  std::unique_ptr<ebpf::BPFHashTable<uint32_t, struct openssl_trace_state_debug_t>>
+      openssl_trace_state_debug_;
+  prometheus::Family<prometheus::Counter>& openssl_trace_mismatched_fds_counter_family_;
 
   absl::flat_hash_set<int> pids_to_trace_disable_;