cleanup(libsinsp): add sinsp_stats_v2_enabled flag

- Only add new flag - Keep m_sinsp_stats_v2 a simple struct always initialized to zero Co-authored-by: Federico Di Pierro <[email protected]> Co-authored-by: Grzegorz Nosek <[email protected]> Signed-off-by: Melissa Kilby <[email protected]>
falcosecurity · Oct 25, 2023 · 140f5d7 · 140f5d7
1 parent b47be2f
commit 140f5d7
Show file tree

Hide file tree

Showing 7 changed files with 42 additions and 25 deletions.
diff --git a/userspace/libsinsp/container.cpp b/userspace/libsinsp/container.cpp
@@ -88,15 +88,15 @@ bool sinsp_container_manager::remove_inactive_containers()
 		});
 
 		auto containers = m_containers.lock();
-		if (m_inspector != nullptr)
+		if (m_inspector != nullptr && m_inspector->m_sinsp_stats_v2_enabled)
 		{
 			m_inspector->m_sinsp_stats_v2.m_n_missing_container_images = 0;
 			m_inspector->m_sinsp_stats_v2.m_n_containers = containers->size();
 		}
 		for(auto it = containers->begin(); it != containers->end();)
 		{
 			sinsp_container_info::ptr_t container = it->second;
-			if (m_inspector != nullptr)
+			if (m_inspector != nullptr && m_inspector->m_sinsp_stats_v2_enabled)
 			{
 				auto container_info = container.get();
 				if (!container_info || (container_info && container_info->m_image.empty()))

diff --git a/userspace/libsinsp/fdinfo.cpp b/userspace/libsinsp/fdinfo.cpp
@@ -354,7 +354,7 @@ sinsp_fdinfo_t* sinsp_fdtable::find(int64_t fd)
 		//
 		if(m_last_accessed_fd != -1 && fd == m_last_accessed_fd)
 		{
-			if (m_inspector != nullptr)
+			if (m_inspector != nullptr && m_inspector->m_sinsp_stats_v2_enabled)
 			{
 				m_inspector->m_sinsp_stats_v2.m_n_cached_fd_lookups++;
 			}
@@ -368,15 +368,15 @@ sinsp_fdinfo_t* sinsp_fdtable::find(int64_t fd)
 
 		if(fdit == m_table.end())
 		{
-			if (m_inspector != nullptr)
+			if (m_inspector != nullptr && m_inspector->m_sinsp_stats_v2_enabled)
 			{
 				m_inspector->m_sinsp_stats_v2.m_n_failed_fd_lookups++;
 			}
 			return NULL;
 		}
 		else
 		{
-			if (m_inspector != nullptr)
+			if (m_inspector != nullptr && m_inspector->m_sinsp_stats_v2_enabled)
 			{
 				m_inspector->m_sinsp_stats_v2.m_n_noncached_fd_lookups++;
 			}
@@ -408,7 +408,7 @@ sinsp_fdinfo_t* sinsp_fdtable::add(int64_t fd, sinsp_fdinfo_t* fdinfo)
 			// No entry in the table, this is the normal case
 			//
 			m_last_accessed_fd = -1;
-			if (m_inspector != nullptr)
+			if (m_inspector != nullptr && m_inspector->m_sinsp_stats_v2_enabled)
 			{
 				m_inspector->m_sinsp_stats_v2.m_n_added_fds++;
 			}
@@ -480,15 +480,15 @@ void sinsp_fdtable::erase(int64_t fd)
 		// keep going.
 		//
 		ASSERT(false);
-		if (m_inspector != nullptr)
+		if (m_inspector != nullptr && m_inspector->m_sinsp_stats_v2_enabled)
 		{
 			m_inspector->m_sinsp_stats_v2.m_n_failed_fd_lookups++;
 		}
 	}
 	else
 	{
 		m_table.erase(fdit);
-		if (m_inspector != nullptr)
+		if (m_inspector != nullptr && m_inspector->m_sinsp_stats_v2_enabled)
 		{
 			m_inspector->m_sinsp_stats_v2.m_n_noncached_fd_lookups++;
 			m_inspector->m_sinsp_stats_v2.m_n_removed_fds++;

diff --git a/userspace/libsinsp/parsers.cpp b/userspace/libsinsp/parsers.cpp
@@ -780,7 +780,7 @@ bool sinsp_parser::reset(sinsp_evt *evt)
 			etype == PPME_SYSCALL_VFORK_20_X ||
 			etype == PPME_SYSCALL_CLONE3_X)
 		{
-			if (m_inspector != nullptr)
+			if (m_inspector != nullptr && m_inspector->m_sinsp_stats_v2_enabled)
 			{
 				m_inspector->m_sinsp_stats_v2.m_n_failed_thread_lookups--;
 			}
@@ -942,7 +942,7 @@ void sinsp_parser::store_event(sinsp_evt *evt)
 		// we won't be able to parse the corresponding exit event and we'll have
 		// to drop the information it carries.
 		//
-		if (m_inspector != nullptr)
+		if (m_inspector != nullptr && m_inspector->m_sinsp_stats_v2_enabled)
 		{
 			m_inspector->m_sinsp_stats_v2.m_n_store_evts_drops++;
 		}
@@ -978,7 +978,7 @@ void sinsp_parser::store_event(sinsp_evt *evt)
 	memcpy(tinfo->m_lastevent_data, evt->m_pevt, elen);
 	tinfo->m_lastevent_cpuid = evt->get_cpuid();
 
-	if (m_inspector != nullptr)
+	if (m_inspector != nullptr && m_inspector->m_sinsp_stats_v2_enabled)
 	{
 		m_inspector->m_sinsp_stats_v2.m_n_stored_evts++;
 	}
@@ -1003,7 +1003,7 @@ bool sinsp_parser::retrieve_enter_event(sinsp_evt *enter_evt, sinsp_evt *exit_ev
 		// This happen especially at the beginning of trace files, where events
 		// can be truncated
 		//
-		if (m_inspector != nullptr)
+		if (m_inspector != nullptr && m_inspector->m_sinsp_stats_v2_enabled)
 		{
 			m_inspector->m_sinsp_stats_v2.m_n_retrieve_evts_drops++;
 		}
@@ -1026,7 +1026,7 @@ bool sinsp_parser::retrieve_enter_event(sinsp_evt *enter_evt, sinsp_evt *exit_ev
 		&&
 		enter_evt->get_type() == PPME_SYSCALL_EXECVEAT_E)
 	{
-		if (m_inspector != nullptr)
+		if (m_inspector != nullptr && m_inspector->m_sinsp_stats_v2_enabled)
 		{
 			m_inspector->m_sinsp_stats_v2.m_n_retrieved_evts++;
 		}
@@ -1041,13 +1041,13 @@ bool sinsp_parser::retrieve_enter_event(sinsp_evt *enter_evt, sinsp_evt *exit_ev
 	{
 		//ASSERT(false);
 		exit_evt->m_tinfo->set_lastevent_data_validity(false);
-		if (m_inspector != nullptr)
+		if (m_inspector != nullptr && m_inspector->m_sinsp_stats_v2_enabled)
 		{
 			m_inspector->m_sinsp_stats_v2.m_n_retrieve_evts_drops++;
 		}
 		return false;
 	}
-	if (m_inspector != nullptr)
+	if (m_inspector != nullptr && m_inspector->m_sinsp_stats_v2_enabled)
 	{
 		m_inspector->m_sinsp_stats_v2.m_n_retrieved_evts++;
 	}
@@ -4156,13 +4156,13 @@ void sinsp_parser::parse_close_exit(sinsp_evt *evt)
 		// It is normal when a close fails that the fd lookup failed, so we revert the
 		// increment of m_n_failed_fd_lookups (for the enter event too if there's one).
 		//
-		if (m_inspector != nullptr)
+		if (m_inspector != nullptr && m_inspector->m_sinsp_stats_v2_enabled)
 		{
 			m_inspector->m_sinsp_stats_v2.m_n_failed_fd_lookups--;
 		}
 		if(evt->m_tinfo && evt->m_tinfo->is_lastevent_data_valid())
 		{
-			if (m_inspector != nullptr)
+			if (m_inspector != nullptr && m_inspector->m_sinsp_stats_v2_enabled)
 			{
 				m_inspector->m_sinsp_stats_v2.m_n_failed_fd_lookups--;
 			}

diff --git a/userspace/libsinsp/sinsp.cpp b/userspace/libsinsp/sinsp.cpp
@@ -353,7 +353,7 @@ void sinsp::init()
 	//
 	// Basic inits
 	//
-
+	m_sinsp_stats_v2_enabled = false;
 	m_sinsp_stats_v2.m_n_added_fds = 0;
 	m_sinsp_stats_v2.m_n_cached_fd_lookups = 0;
 	m_sinsp_stats_v2.m_n_failed_fd_lookups = 0;
@@ -2677,6 +2677,11 @@ void sinsp::set_proc_scan_log_interval_ms(uint64_t val)
 	m_proc_scan_log_interval_ms = val;
 }
 
+void sinsp::set_sinsp_stats_v2_enabled(bool sinsp_stats_v2_enabled)
+{
+	m_sinsp_stats_v2_enabled = sinsp_stats_v2_enabled;
+}
+
 ///////////////////////////////////////////////////////////////////////////////
 // Note: this is defined here so we can inline it in sinso::next
 ///////////////////////////////////////////////////////////////////////////////

diff --git a/userspace/libsinsp/sinsp.h b/userspace/libsinsp/sinsp.h
@@ -435,6 +435,12 @@ class SINSP_PUBLIC sinsp : public capture_stats_source
 	 */
 	void set_proc_scan_log_interval_ms(uint64_t val);
 
+	/*!
+	 * \brief sets sinsp_stats_v2_enabled, enabling sinsp state counters on the hot path.
+	 * \param sinsp_stats_v2_enabled if true, activates sinsp_stats_v2 counters on the hot path.
+	 */
+	void set_sinsp_stats_v2_enabled(bool sinsp_stats_v2_enabled);
+
 
 	/*!
 	  \brief Start writing the captured events to file.
@@ -1243,6 +1249,11 @@ VISIBILITY_PRIVATE
 	//
 	bool m_isdropping;
 
+	//
+	// Enable sinsp_stats_v2 on the hot path
+	//
+	bool m_sinsp_stats_v2_enabled;
+
 	//
 	// App events
 	//

diff --git a/userspace/libsinsp/test/sinsp_with_test_input.h b/userspace/libsinsp/test/sinsp_with_test_input.h
@@ -59,7 +59,8 @@ class sinsp_with_test_input : public ::testing::Test {
 	sinsp m_inspector;
 
 	void open_inspector(scap_mode_t mode = SCAP_MODE_TEST) {
-		m_inspector.open_test_input(m_test_data.get(), mode);
+		m_inspector.open_test_input(m_test_data.get(), mode);	
+		m_inspector.set_sinsp_stats_v2_enabled(true);
 	}
 
 	scap_evt* add_event(uint64_t ts, uint64_t tid, ppm_event_code event_type, uint32_t n, ...)

diff --git a/userspace/libsinsp/threadinfo.cpp b/userspace/libsinsp/threadinfo.cpp
@@ -1529,7 +1529,7 @@ std::unique_ptr<sinsp_threadinfo> sinsp_thread_manager::new_threadinfo() const
  */
 bool sinsp_thread_manager::add_thread(sinsp_threadinfo *threadinfo, bool from_scap_proctable)
 {
-	if (m_inspector != nullptr)
+	if (m_inspector != nullptr && m_inspector->m_sinsp_stats_v2_enabled)
 	{
 		m_inspector->m_sinsp_stats_v2.m_n_added_threads++;
 	}
@@ -1714,7 +1714,7 @@ void sinsp_thread_manager::remove_thread(int64_t tid)
 	/* This should never happen but just to be sure. */
 	if(thread_to_remove == nullptr)
 	{
-		if (m_inspector != nullptr)
+		if (m_inspector != nullptr && m_inspector->m_sinsp_stats_v2_enabled)
 		{
 			m_inspector->m_sinsp_stats_v2.m_n_failed_thread_lookups++;
 		}
@@ -1836,7 +1836,7 @@ void sinsp_thread_manager::remove_thread(int64_t tid)
 	 * the cache just to be sure.
 	 */
 	m_last_tid = -1;
-	if (m_inspector != nullptr)
+	if (m_inspector != nullptr && m_inspector->m_sinsp_stats_v2_enabled)
 	{
 		m_inspector->m_sinsp_stats_v2.m_n_removed_threads++;
 	}
@@ -2176,7 +2176,7 @@ threadinfo_map_t::ptr_t sinsp_thread_manager::find_thread(int64_t tid, bool look
 		thr = m_last_tinfo.lock();
 		if (thr)
 		{
-			if (m_inspector != nullptr)
+			if (m_inspector != nullptr && m_inspector->m_sinsp_stats_v2_enabled)
 			{
 				m_inspector->m_sinsp_stats_v2.m_n_cached_thread_lookups++;
 			}
@@ -2194,7 +2194,7 @@ threadinfo_map_t::ptr_t sinsp_thread_manager::find_thread(int64_t tid, bool look
 
 	if(thr)
 	{
-		if (m_inspector != nullptr)
+		if (m_inspector != nullptr && m_inspector->m_sinsp_stats_v2_enabled)
 		{
 			m_inspector->m_sinsp_stats_v2.m_n_noncached_thread_lookups++;
 		}
@@ -2209,7 +2209,7 @@ threadinfo_map_t::ptr_t sinsp_thread_manager::find_thread(int64_t tid, bool look
 	}
 	else
 	{
-		if (m_inspector != nullptr)
+		if (m_inspector != nullptr && m_inspector->m_sinsp_stats_v2_enabled)
 		{
 			m_inspector->m_sinsp_stats_v2.m_n_failed_thread_lookups++;
 		}