Revert "Add option for sampling idle recently active sessions (#19479)" (#19492)

This reverts commit 06d858a.

Author: nenadnoveljic

sqlserver/assets/configuration/spec.yaml

@@ -609,14 +609,6 @@ files:
           value:
             type: number
             example: 10
-        - name: sample_recently_active_idle_sessions
-          description: |
-            This option enables the sampling of idle sessions that were active since the last collection.
-            Recommended for APM customers to boost the APM/DBM correlation.
-          value:
-            type: boolean
-            example: true
-
     - name: stored_procedure_characters_limit
       description: |
         Limit the number of characters of the text of a stored procedure that is collected.

sqlserver/changelog.d/19479.added

@@ -91,11 +91,11 @@
 ).strip()
 
 # Turns out sys.dm_exec_requests does not contain idle sessions.
-# Inner joining dm_exec_sessions with dm_exec_requests will not return any idle sessions.
-# This prevent us reusing the same ACTIVITY_QUERY for regular activities and idle sessions.
+# Inner joining dm_exec_sessions with dm_exec_requests will not return any idle blocking sessions.
+# This prevent us reusing the same ACTIVITY_QUERY for regular activities and idle blocking sessions.
 # The query below is used for idle sessions and does not join with dm_exec_requests.
 # The last query execution on the connection is fetched from dm_exec_connections.most_recent_sql_handle.
-IDLE_SESSIONS_QUERY = re.sub(
+IDLE_BLOCKING_SESSIONS_QUERY = re.sub(
     r'\s+',
     ' ',
     """\
@@ -114,25 +114,17 @@
     c.client_net_address as client_address,
     sess.host_name as host_name,
     sess.program_name as program_name,
-    sess.is_user_process as is_user_process,
-    sess.context_info as context_info
+    sess.is_user_process as is_user_process
 FROM sys.dm_exec_sessions sess
     INNER JOIN sys.dm_exec_connections c
         ON sess.session_id = c.session_id
     CROSS APPLY sys.dm_exec_sql_text(c.most_recent_sql_handle) lqt
-WHERE
+WHERE sess.status = 'sleeping'
+    AND sess.session_id IN ({blocking_session_ids})
+    AND c.session_id IN ({blocking_session_ids})
 """,
 ).strip()
 
-IDLE_BLOCKERS_FILTER = """(sess.status = 'sleeping'
-    AND sess.session_id IN ({blocking_session_ids})
-    AND c.session_id IN ({blocking_session_ids}))"""
-
-IDLE_RECENTLY_ACTIVE_FILTER = (
-    "(sess.status = 'sleeping' AND sess.last_request_start_time > DATEADD(SECOND, - ?, GETDATE()))"
-)
-
-
 # enumeration of the columns we collect
 # from sys.dm_exec_requests
 DM_EXEC_REQUESTS_COLS = [
@@ -203,10 +195,6 @@ def __init__(self, check, config: SQLServerConfig):
         self._conn_key_prefix = "dbm-activity-"
         self._activity_payload_max_bytes = MAX_PAYLOAD_BYTES
         self._exec_requests_cols_cached = None
-        self._sample_recently_active_idle_sessions = is_affirmative(
-            self._config.activity_config.get('sample_recently_active_idle_sessions', False)
-        )
-        self._time_since_last_activity_event = 0
 
         self._collect_raw_query_statement = self._config.collect_raw_query_statement.get("enabled", False)
         self._raw_statement_text_cache = RateLimitingTTLCache(
@@ -231,34 +219,15 @@ def _get_active_connections(self, cursor):
         self.log.debug("loaded sql server current connections len(rows)=%s", len(rows))
         return rows
 
-    def _is_sample_idle_recently_active_sessions(self) -> bool:
-        return self._sample_recently_active_idle_sessions and self._time_since_last_activity_event
-
     @tracked_method(agent_check_getter=agent_check_getter, track_result_length=True)
-    def _get_idle_sessions(self, cursor, blocking_session_ids):
-        query = IDLE_SESSIONS_QUERY.format(
+    def _get_idle_blocking_sessions(self, cursor, blocking_session_ids):
+        # The IDLE_BLOCKING_SESSIONS_QUERY contains minimum information on idle blocker
+        query = IDLE_BLOCKING_SESSIONS_QUERY.format(
+            blocking_session_ids=",".join(map(str, blocking_session_ids)),
             proc_char_limit=self._config.stored_procedure_characters_limit,
         )
-        idle_filter = None
-
-        # a stateful closure that appends filters to the query
-        def _append_filter(filter: str) -> str:
-            if idle_filter:
-                return f"{idle_filter} OR {filter}"
-            return filter
-
-        if blocking_session_ids:
-            idle_filter = _append_filter(
-                IDLE_BLOCKERS_FILTER.format(blocking_session_ids=",".join(map(str, blocking_session_ids)))
-            )
-        if self._is_sample_idle_recently_active_sessions():
-            idle_filter = _append_filter(IDLE_RECENTLY_ACTIVE_FILTER)
-        query += idle_filter
         self.log.debug("Running query [%s]", query)
-        if self._is_sample_idle_recently_active_sessions():
-            cursor.execute(query, (int(time.time() - self._time_since_last_activity_event),))
-        else:
-            cursor.execute(query)
+        cursor.execute(query)
         columns = [i[0] for i in cursor.description]
         rows = [dict(zip(columns, row)) for row in cursor.fetchall()]
         return rows
@@ -284,9 +253,9 @@ def _get_activity(self, cursor, exec_request_columns, input_buffer_columns, inpu
         blocking_session_ids = {r['blocking_session_id'] for r in rows if r['blocking_session_id']}
         # if there are blocking sessions and some of the session(s) are not captured in the activity query
         idle_blocking_session_ids = blocking_session_ids - session_ids
-        if idle_blocking_session_ids or self._is_sample_idle_recently_active_sessions():
-            idle_sessions = self._get_idle_sessions(cursor, idle_blocking_session_ids)
-            rows.extend(idle_sessions)
+        if idle_blocking_session_ids:
+            idle_blocking_sessions = self._get_idle_blocking_sessions(cursor, idle_blocking_session_ids)
+            rows.extend(idle_blocking_sessions)
         return rows
 
     def _normalize_queries_and_filter_rows(self, rows, max_bytes_limit):
@@ -464,7 +433,6 @@ def _get_estimated_row_size_bytes(row):
         return len(str(row))
 
     def _create_activity_event(self, active_sessions, active_connections):
-        self._time_since_last_activity_event = time.time()
         event = {
             "host": self._check.resolved_hostname,
             "ddagentversion": datadog_agent.get_version(),
@@ -485,7 +453,7 @@ def _create_activity_event(self, active_sessions, active_connections):
     @tracked_method(agent_check_getter=agent_check_getter)
     def collect_activity(self):
         """
-        Collects all current activity for the SQLServer instance.
+        Collects all current activity for the SQLServer intance.
         :return:
         """
 

sqlserver/datadog_checks/sqlserver/activity.py

@@ -311,7 +311,6 @@ class QueryActivity(BaseModel):
     )
     collection_interval: Optional[float] = None
     enabled: Optional[bool] = None
-    sample_recently_active_idle_sessions: Optional[bool] = None
 
 
 class QueryMetrics(BaseModel):

sqlserver/datadog_checks/sqlserver/config_models/instance.py

@@ -419,12 +419,6 @@ instances:
         #
         # collection_interval: 10
 
-        ## @param sample_recently_active_idle_sessions - boolean - optional - default: true
-        ## This option enables the sampling of idle sessions that were active since the last collection.
-        ## Recommended for APM customers to boost the APM/DBM correlation.
-        #
-        # sample_recently_active_idle_sessions: true
-
     ## @param stored_procedure_characters_limit - integer - optional - default: 500
     ## Limit the number of characters of the text of a stored procedure that is collected.
     ## The characters limit is applicable to both query metrics and query samples.

sqlserver/datadog_checks/sqlserver/data/conf.yaml.example

@@ -57,17 +57,6 @@ def dbm_instance(instance_docker):
     return copy(instance_docker)
 
 
-@pytest.mark.usefixtures('dd_environment')
-def test_idle_sessions_sampling(dbm_instance, dd_run_check):
-    check = SQLServer(CHECK_NAME, {}, [dbm_instance])
-    assert (
-        not check.activity._sample_recently_active_idle_sessions
-    ), "Sample recently active idle sessions switched off by default"
-    check.activity._sample_recently_active_idle_sessions = True
-    check.activity._time_since_last_activity_event = 10
-    dd_run_check(check)
-
-
 @pytest.mark.flaky
 @pytest.mark.integration
 @pytest.mark.usefixtures('dd_environment')