SERVER-71103 Improve UX for failed metrics collection

6 files changed, 79 insertions, 19 deletions

diff --git a/buildscripts/metrics/resmoke_tooling_metrics.py b/buildscripts/metrics/resmoke_tooling_metrics.py
index 1b24160d26f..71c7b51bb49 100644
--- a/buildscripts/metrics/resmoke_tooling_metrics.py
+++ b/buildscripts/metrics/resmoke_tooling_metrics.py
@@ -2,14 +2,14 @@ from datetime import datetime
 import logging
 
 from buildscripts.metrics.metrics_datatypes import ToolingMetrics
-from buildscripts.metrics.tooling_metrics_utils import is_virtual_workstation, save_tooling_metrics
+from buildscripts.metrics.tooling_metrics_utils import save_tooling_metrics, should_collect_metrics
 
 logger = logging.getLogger('resmoke_tooling_metrics')
 
 
 def save_resmoke_tooling_metrics(utc_starttime: datetime):
     try:
-        if not is_virtual_workstation():
+        if not should_collect_metrics():
             return
         tooling_metrics = ToolingMetrics.get_resmoke_metrics(utc_starttime)
         save_tooling_metrics(tooling_metrics)
diff --git a/buildscripts/metrics/scons_tooling_metrics.py b/buildscripts/metrics/scons_tooling_metrics.py
index 0b02b6f55c3..cdece084631 100644
--- a/buildscripts/metrics/scons_tooling_metrics.py
+++ b/buildscripts/metrics/scons_tooling_metrics.py
@@ -5,7 +5,7 @@ import sys
 from typing import List
 
 from buildscripts.metrics.metrics_datatypes import ToolingMetrics
-from buildscripts.metrics.tooling_metrics_utils import is_virtual_workstation, save_tooling_metrics
+from buildscripts.metrics.tooling_metrics_utils import save_tooling_metrics, should_collect_metrics
 
 logger = logging.getLogger('scons_tooling_metrics')
 
@@ -39,7 +39,7 @@ def _save_scons_tooling_metrics(
 ):
     """Save SCons tooling metrics to atlas cluster."""
     try:
-        if not is_virtual_workstation():
+        if not should_collect_metrics():
             return
         tooling_metrics = ToolingMetrics.get_scons_metrics(utc_starttime, env_vars, env, parser,
                                                            args, exit_hook.exit_code)
diff --git a/buildscripts/metrics/tooling_metrics_utils.py b/buildscripts/metrics/tooling_metrics_utils.py
index 9e8b3256ecb..4a99f90c0f0 100644
--- a/buildscripts/metrics/tooling_metrics_utils.py
+++ b/buildscripts/metrics/tooling_metrics_utils.py
@@ -20,6 +20,10 @@ def _get_internal_tooling_metrics_client():
         host=INTERNAL_TOOLING_METRICS_HOSTNAME,
         username=INTERNAL_TOOLING_METRICS_USERNAME,
         password=INTERNAL_TOOLING_METRICS_PASSWORD,
+        socketTimeoutMS=1000,
+        serverSelectionTimeoutMS=1000,
+        connectTimeoutMS=1000,
+        waitQueueTimeoutMS=1000,
     )
 
 
@@ -39,11 +43,24 @@ def _git_user_exists() -> Optional[str]:
         return None
 
 
-def is_virtual_workstation() -> bool:
+def _is_virtual_workstation() -> bool:
     """Detect whether this is a MongoDB internal virtual workstation."""
     return _toolchain_exists() and _git_user_exists()
 
 
+TOOLING_METRICS_OPT_OUT = "TOOLING_METRICS_OPT_OUT"
+
+
+def _has_metrics_opt_out() -> bool:
+    """Check whether the opt out environment variable is set."""
+    return os.environ.get(TOOLING_METRICS_OPT_OUT, None) == '1'
+
+
+def should_collect_metrics() -> bool:
+    """Determine whether to collect tooling metrics."""
+    return _is_virtual_workstation() and not _has_metrics_opt_out()
+
+
 async def _save_metrics(metrics: ToolingMetrics) -> None:
     """Save tooling metrics data."""
     client = _get_internal_tooling_metrics_client()
diff --git a/buildscripts/tests/tooling_metrics/test_resmoke_tooling_metrics.py b/buildscripts/tests/tooling_metrics/test_resmoke_tooling_metrics.py
index 3510184eb0c..cc3b909381f 100644
--- a/buildscripts/tests/tooling_metrics/test_resmoke_tooling_metrics.py
+++ b/buildscripts/tests/tooling_metrics/test_resmoke_tooling_metrics.py
@@ -22,13 +22,13 @@ if os.name == "nt":
 @patch("buildscripts.metrics.tooling_metrics_utils.INTERNAL_TOOLING_METRICS_HOSTNAME",
        TEST_INTERNAL_TOOLING_METRICS_HOSTNAME)
 @patch("buildscripts.resmokelib.logging.flush._FLUSH_THREAD", None)
-@patch("buildscripts.metrics.resmoke_tooling_metrics.is_virtual_workstation", return_value=True)
 class TestResmokeMetricsCollection(unittest.TestCase):
     @mongomock.patch(servers=((TEST_INTERNAL_TOOLING_METRICS_HOSTNAME), ))
+    @patch("buildscripts.metrics.resmoke_tooling_metrics.should_collect_metrics", return_value=True)
     @patch("sys.argv", ['buildscripts/resmoke.py', 'run', '--suite', 'buildscripts_test'])
     @patch("buildscripts.resmokelib.testing.executor.TestSuiteExecutor._run_tests",
            side_effect=Exception())
-    def test_resmoke_metrics_collection_exc(self, mock_executor_run, mock_is_virtual_workstation):
+    def test_resmoke_metrics_collection_exc(self, mock_executor_run, mock_should_collect_metrics):
         client = pymongo.MongoClient(host=TEST_INTERNAL_TOOLING_METRICS_HOSTNAME)
         assert not client.metrics.tooling_metrics.find_one()
         with self.assertRaises(SystemExit):
@@ -36,9 +36,20 @@ class TestResmokeMetricsCollection(unittest.TestCase):
         assert client.metrics.tooling_metrics.find_one()
 
     @mongomock.patch(servers=((TEST_INTERNAL_TOOLING_METRICS_HOSTNAME), ))
+    @patch("buildscripts.metrics.resmoke_tooling_metrics.should_collect_metrics", return_value=True)
     @patch("sys.argv", ['buildscripts/resmoke.py', 'list-suites'])
-    def test_resmoke_metrics_collection(self, mock_is_virtual_workstation):
+    def test_resmoke_metrics_collection(self, mock_should_collect_metrics):
         client = pymongo.MongoClient(host=TEST_INTERNAL_TOOLING_METRICS_HOSTNAME)
         assert not client.metrics.tooling_metrics.find_one()
         resmoke_entrypoint()
         assert client.metrics.tooling_metrics.find_one()
+
+    @mongomock.patch(servers=((TEST_INTERNAL_TOOLING_METRICS_HOSTNAME), ))
+    @patch("buildscripts.metrics.resmoke_tooling_metrics.should_collect_metrics",
+           return_value=False)
+    @patch("sys.argv", ['buildscripts/resmoke.py', 'list-suites'])
+    def test_no_resmoke_metrics_collection(self, mock_should_collect_metrics):
+        client = pymongo.MongoClient(host=TEST_INTERNAL_TOOLING_METRICS_HOSTNAME)
+        assert not client.metrics.tooling_metrics.find_one()
+        resmoke_entrypoint()
+        assert not client.metrics.tooling_metrics.find_one()
diff --git a/buildscripts/tests/tooling_metrics/test_scons_tooling_metrics.py b/buildscripts/tests/tooling_metrics/test_scons_tooling_metrics.py
index f0e020bf306..c50c89f85ba 100644
--- a/buildscripts/tests/tooling_metrics/test_scons_tooling_metrics.py
+++ b/buildscripts/tests/tooling_metrics/test_scons_tooling_metrics.py
@@ -23,11 +23,11 @@ if os.name == "nt":
     'buildscripts/scons.py', "CC=/opt/mongodbtoolchain/v3/bin/gcc",
     "CXX=/opt/mongodbtoolchain/v3/bin/g++", "NINJA_PREFIX=test_success", "--ninja"
 ])
-@patch("buildscripts.metrics.scons_tooling_metrics.is_virtual_workstation", return_value=True)
+@patch("buildscripts.metrics.scons_tooling_metrics.should_collect_metrics", return_value=True)
 @patch("atexit.register")
 class TestSconsAtExitMetricsCollection(unittest.TestCase):
     def test_scons_at_exit_metrics_collection(self, mock_atexit_register,
-                                              mock_is_virtual_workstation):
+                                              mock_should_collect_metrics):
         with self.assertRaises(SystemExit) as context:
             scons_entrypoint()
         assert context.exception.code == 0
@@ -36,7 +36,7 @@ class TestSconsAtExitMetricsCollection(unittest.TestCase):
 
     @patch("buildscripts.moduleconfig.get_module_sconscripts", side_effect=Exception())
     def test_scons_at_exit_metrics_collection_exc(self, mock_method, mock_atexit_register,
-                                                  mock_is_virtual_workstation):
+                                                  mock_should_collect_metrics):
         with self.assertRaises(SystemExit) as context:
             scons_entrypoint()
         assert context.exception.code == 2
@@ -46,19 +46,29 @@ class TestSconsAtExitMetricsCollection(unittest.TestCase):
 
 @patch("buildscripts.metrics.tooling_metrics_utils.INTERNAL_TOOLING_METRICS_HOSTNAME",
        TEST_INTERNAL_TOOLING_METRICS_HOSTNAME)
-@patch("buildscripts.metrics.scons_tooling_metrics.is_virtual_workstation", return_value=True)
-class TestSconsMetricsPersist(unittest.TestCase):
+class TestSconsMetricsCollection(unittest.TestCase):
     @mongomock.patch(servers=((TEST_INTERNAL_TOOLING_METRICS_HOSTNAME), ))
-    def test_scons_metrics_collection_success(self, mock_is_virtual_workstation):
+    @patch("buildscripts.metrics.scons_tooling_metrics.should_collect_metrics", return_value=True)
+    def test_scons_metrics_collection_success(self, mock_should_collect_metrics):
         client = pymongo.MongoClient(host=TEST_INTERNAL_TOOLING_METRICS_HOSTNAME)
         assert not client.metrics.tooling_metrics.find_one()
         under_test._save_scons_tooling_metrics(CURRENT_DATE_TIME, None, None, None, None,
                                                MagicMock(exit_code=0))
         assert client.metrics.tooling_metrics.find_one()
 
+    @patch("buildscripts.metrics.scons_tooling_metrics.should_collect_metrics", return_value=True)
     @mongomock.patch(servers=((TEST_INTERNAL_TOOLING_METRICS_HOSTNAME), ))
-    def test_scons_metrics_collection_fail(self, mock_is_virtual_workstation):
+    def test_scons_metrics_collection_fail(self, mock_should_collect_metrics):
         client = pymongo.MongoClient(host=TEST_INTERNAL_TOOLING_METRICS_HOSTNAME)
         assert not client.metrics.tooling_metrics.find_one()
         under_test._save_scons_tooling_metrics(None, None, None, None, None, None)
         assert not client.metrics.tooling_metrics.find_one()
+
+    @patch("buildscripts.metrics.scons_tooling_metrics.should_collect_metrics", return_value=False)
+    @mongomock.patch(servers=((TEST_INTERNAL_TOOLING_METRICS_HOSTNAME), ))
+    def test_no_scons_metrics_collection(self, mock_should_collect_metrics):
+        client = pymongo.MongoClient(host=TEST_INTERNAL_TOOLING_METRICS_HOSTNAME)
+        assert not client.metrics.tooling_metrics.find_one()
+        under_test._save_scons_tooling_metrics(CURRENT_DATE_TIME, None, None, None, None,
+                                               MagicMock(exit_code=0))
+        assert not client.metrics.tooling_metrics.find_one()
diff --git a/buildscripts/tests/tooling_metrics/test_tooling_metrics_utils.py b/buildscripts/tests/tooling_metrics/test_tooling_metrics_utils.py
index c1c768e50d2..0b64c6bb05a 100644
--- a/buildscripts/tests/tooling_metrics/test_tooling_metrics_utils.py
+++ b/buildscripts/tests/tooling_metrics/test_tooling_metrics_utils.py
@@ -60,19 +60,41 @@ class TestIsVirtualWorkstation(unittest.TestCase):
     @patch("buildscripts.metrics.tooling_metrics_utils._toolchain_exists", return_value=False)
     @patch("buildscripts.metrics.tooling_metrics_utils._git_user_exists", return_value=True)
     def test_no_toolchain_has_email(self, mock_git_user_exists, mock_toolchain_exists):
-        assert not under_test.is_virtual_workstation()
+        assert not under_test._is_virtual_workstation()
 
     @patch("buildscripts.metrics.tooling_metrics_utils._toolchain_exists", return_value=True)
     @patch("buildscripts.metrics.tooling_metrics_utils._git_user_exists", return_value=True)
     def test_has_toolchain_has_email(self, mock_git_user_exists, mock_toolchain_exists):
-        assert under_test.is_virtual_workstation()
+        assert under_test._is_virtual_workstation()
 
     @patch("buildscripts.metrics.tooling_metrics_utils._toolchain_exists", return_value=True)
     @patch("buildscripts.metrics.tooling_metrics_utils._git_user_exists", return_value=False)
     def test_has_toolchain_no_email(self, mock_git_user_exists, mock_toolchain_exists):
-        assert not under_test.is_virtual_workstation()
+        assert not under_test._is_virtual_workstation()
 
     @patch("buildscripts.metrics.tooling_metrics_utils._toolchain_exists", return_value=False)
     @patch("buildscripts.metrics.tooling_metrics_utils._git_user_exists", return_value=False)
     def test_no_toolchain_no_email(self, mock_git_user_exists, mock_toolchain_exists):
-        assert not under_test.is_virtual_workstation()
+        assert not under_test._is_virtual_workstation()
+
+
+class TestHasMetricsOptOut(unittest.TestCase):
+    @patch("os.environ.get", return_value='1')
+    def test_opt_out(self, mock_environ_get):
+        assert under_test._has_metrics_opt_out()
+
+    @patch("os.environ.get", return_value=None)
+    def test_no_opt_out(self, mock_environ_get):
+        assert not under_test._has_metrics_opt_out()
+
+
+class TestShouldCollectMetrics(unittest.TestCase):
+    @patch("buildscripts.metrics.tooling_metrics_utils._is_virtual_workstation", return_value=True)
+    @patch("buildscripts.metrics.tooling_metrics_utils._has_metrics_opt_out", return_value=False)
+    def test_should_collect_metrics(self, mock_opt_out, mock_is_virtual_env):
+        assert under_test.should_collect_metrics()
+
+    @patch("buildscripts.metrics.tooling_metrics_utils._is_virtual_workstation", return_value=True)
+    @patch("buildscripts.metrics.tooling_metrics_utils._has_metrics_opt_out", return_value=True)
+    def test_no_collect_metrics_opt_out(self, mock_opt_out, mock_is_virtual_env):
+        assert not under_test.should_collect_metrics()