analytics: use a tempfile for inter-process communication

dvc/analytics.py

@@ -3,31 +3,57 @@ import logging
 import platform
 import requests
 import sys
+import tempfile
 import uuid
 
 import distro
 
 from dvc import __version__
 from dvc.config import Config, to_bool
+from dvc.daemon import daemon
 from dvc.exceptions import NotDvcRepoError
 from dvc.lock import Lock, LockError
 from dvc.repo import Repo
 from dvc.scm import SCM
 from dvc.utils import env2bool, is_binary, makedirs
-from dvc.utils.compat import str, FileNotFoundError
 from dvc.utils.compat import str, FileNotFoundError, convert_to_unicode
 
 
 logger = logging.getLogger(__name__)
 
 
-def collect(cmd_class=None, return_code=None):
+def collect_and_send_report(args=None, return_code=None):
     """
-    Query the system to fill a report.
+    Collect information from the runtime/environment and the command
+    being executed into a report and send it over the network.
+
+    To prevent analytics from blocking the execution of the main thread,
+    sending the report is done in a separate process.
+
+    The inter-process communication happens through a file containing the
+    report as a JSON, where the _collector_ generates it and the _sender_
+    removes it after sending it.
+    """
+    report = runtime_info()
+
+    # Include command execution information on the report
+    report.update(
+        {
+            "cmd_class": args.func.__name__ if hasattr(args, "func") else None,
+            "cmd_return_code": return_code,
+        }
+    )
+
+    with tempfile.NamedTemporaryFile(delete=False, mode="w") as fobj:
+        json.dump(report, fobj)
+        daemon(["analytics", fobj.name])
+
+
+def runtime_info():
+    """
+    Gather information from the environment where DVC runs to fill a report.
     """
     return {
-        "cmd_class": cmd_class,
-        "cmd_return_code": return_code,
         "dvc_version": __version__,
         "is_binary": is_binary(),
         "scm_class": scm_in_use(),
@@ -53,7 +79,10 @@ def is_enabled():
 
 def send(report):
     url = "https://analytics.dvc.org"
-    requests.post(url, json=report, timeout=5)
+    headers = {"content-type": "application/json"}
+
+    with open(report, "rb") as fobj:
+        requests.post(url, data=fobj, headers=headers, timeout=5)
 
 
 def scm_in_use():

dvc/command/daemon.py

@@ -25,11 +25,12 @@ class CmdDaemonUpdater(CmdDaemonBase):
 
 class CmdDaemonAnalytics(CmdDaemonBase):
     def run(self):
-        """Collect and send analytics"""
         from dvc import analytics
 
-        report = analytics.collect(self.args.cmd_class, self.args.ret)
+        report = self.args.target
+
         analytics.send(report)
+        os.remove(report)
 
         return 0
 
@@ -58,17 +59,12 @@ def add_parser(subparsers, parent_parser):
     )
     daemon_updater_parser.set_defaults(func=CmdDaemonUpdater)
 
-    DAEMON_ANALYTICS_HELP = "Collect and send dvc usage analytics."
+    DAEMON_ANALYTICS_HELP = "Send dvc usage analytics."
     daemon_analytics_parser = daemon_subparsers.add_parser(
         "analytics",
         parents=[parent_parser],
         description=DAEMON_ANALYTICS_HELP,
         help=DAEMON_ANALYTICS_HELP,
     )
-    daemon_analytics_parser.add_argument(
-        "cmd_class", help="Class called through main"
-    )
-    daemon_analytics_parser.add_argument(
-        "ret", help="Return code from running such class"
-    )
+    daemon_analytics_parser.add_argument("target", help="Analytics file.")
     daemon_analytics_parser.set_defaults(func=CmdDaemonAnalytics)

dvc/main.py

@@ -6,7 +6,6 @@ import logging
 from dvc import analytics
 from dvc.cli import parse_args
 from dvc.config import ConfigError
-from dvc.daemon import daemon
 from dvc.exceptions import DvcParserError
 from dvc.exceptions import NotDvcRepoError
 from dvc.external_repo import clean_repos
@@ -83,9 +82,6 @@ def main(argv=None):
         logger.info(FOOTER)
 
     if analytics.is_enabled():
-        # Collect and send analytics report in a separate process,
-        # including the command class and return code on the report
-        cmd_class = args.func.__name__ if hasattr(args, "func") else None
-        daemon(["analytics", cmd_class, ret])
+        analytics.collect_and_send_report(args, ret)
 
     return ret

tests/func/test_analytics.py

@@ -1,11 +1,24 @@
 import mock
 
 from dvc.main import main
+from dvc.utils.compat import fspath
 
 
-@mock.patch("dvc.analytics.collect")
 @mock.patch("dvc.analytics.send")
-def test_daemon_analytics(mock_collect, mock_send, tmp_path):
-    assert 0 == main(["daemon", "analytics", None, 0])
-    assert mock_collect.called
-    assert mock_send.called
+@mock.patch("os.remove")
+def test_daemon_analytics(mock_remove, mock_send, tmp_path):
+    report = fspath(tmp_path)
+    assert 0 == main(["daemon", "analytics", report])
+
+    mock_send.assert_called_with(report)
+    mock_remove.assert_called_with(report)
+
+
+@mock.patch("dvc.daemon._spawn")
+@mock.patch("dvc.analytics.is_enabled", return_value=True)
+@mock.patch("dvc.analytics.runtime_info", return_value={})
+def test_main_analytics(mock_is_enabled, mock_daemon, mock_report, dvc_repo):
+    assert 0 == main(["add", "foo"])
+    assert mock_is_enabled.called
+    assert mock_report.called
+    assert mock_daemon.called

tests/unit/test_analytics.py

@@ -20,30 +20,32 @@ def tmp_global_config(tmp_path):
         yield
 
 
-def test_collect(tmp_global_config):
+def test_runtime_info(tmp_global_config):
     schema = Schema(
         {
-            "cmd_class": Any(str, None),
-            "cmd_return_code": Any(int, None),
             "dvc_version": Any(builtin_str, str),
             "is_binary": bool,
-            "scm_class": Any("Git", None),
+            "scm_class": Any("Git", "NoSCM"),
             "user_id": Any(builtin_str, str),
             "system_info": dict,
         }
     )
 
-    report = analytics.collect(return_code=0)
-    assert schema(report)
+    assert schema(analytics.runtime_info())
 
 
 @mock.patch("requests.post")
 def test_send(mock_post, tmp_path):
     url = "https://analytics.dvc.org"
     report = {"name": "dummy report"}
+    fname = tmp_path / "report"
 
-    analytics.send(report)
-    mock_post.assert_called_with(url, json=report, timeout=5)
+    with open(fname, "w") as fobj:
+        json.dump(report, fobj)
+
+    analytics.send(fname)
+    assert mock_post.called
+    assert mock_post.call_args.args[0] == url
 
 
 @pytest.mark.parametrize(