Feature/sudoless metrics (#2605)

* Add and use sudoless machine id getter for metrics

Step 2 of the metrics user ID improvement plan.

* Add machine-id v3 tests

Duplicates of the v1 tests, since those covered the cases that remain in v3,
and we will remove v1 and its tests later.

Also fix formatting of some python files.

* DRY out machine ID paths into global constants

lib/streamlit/cli.py

@@ -208,7 +208,7 @@ def main_run(target, args=None, **kwargs):
             raise click.BadArgumentUsage(
                 "Streamlit requires raw Python (.py) files, but the provided file has no extension.\nFor more information, please see https://docs.streamlit.io"
             )
-        else:    
+        else:
             raise click.BadArgumentUsage(
                 "Streamlit requires raw Python (.py) files, not %s.\nFor more information, please see https://docs.streamlit.io"
                 % extension

lib/streamlit/metrics_util.py

@@ -8,6 +8,9 @@
 
 from streamlit import file_util
 
+_ETC_MACHINE_ID_PATH = "/etc/machine-id"
+_DBUS_MACHINE_ID_PATH = "/var/lib/dbus/machine-id"
+
 
 def _get_machine_id():
     """Get the machine ID
@@ -25,18 +28,48 @@ def _get_machine_id():
     """
     if (
         platform.system() == "Linux"
-        and os.path.isfile("/etc/machine-id") == False
-        and os.path.isfile("/var/lib/dbus/machine-id") == False
+        and os.path.isfile(_ETC_MACHINE_ID_PATH) == False
+        and os.path.isfile(_DBUS_MACHINE_ID_PATH) == False
     ):
         subprocess.run(["sudo", "dbus-uuidgen", "--ensure"])
 
     machine_id = str(uuid.getnode())
-    if os.path.isfile("/etc/machine-id"):
-        with open("/etc/machine-id", "r") as f:
+    if os.path.isfile(_ETC_MACHINE_ID_PATH):
+        with open(_ETC_MACHINE_ID_PATH, "r") as f:
             machine_id = f.read()
 
-    elif os.path.isfile("/var/lib/dbus/machine-id"):
-        with open("/var/lib/dbus/machine-id", "r") as f:
+    elif os.path.isfile(_DBUS_MACHINE_ID_PATH):
+        with open(_DBUS_MACHINE_ID_PATH, "r") as f:
+            machine_id = f.read()
+
+    return machine_id
+
+
+def _get_machine_id_v3() -> str:
+    """Get the machine ID
+
+    This is a unique identifier for a user for tracking metrics in Segment,
+    that is broken in different ways in some Linux distros and Docker images.
+    - at times just a hash of '', which means many machines map to the same ID
+    - at times a hash of the same string, when running in a Docker container
+
+    This is a replacement for _get_machine_id() that doesn't try to use `sudo`
+    when there is no machine-id file, because it isn't available in all enviroments
+    and is a bad break in the user flow even when it is usable.
+
+    We'll track multiple IDs in Segment for a few months after which
+    we'll drop the others. The goal is to determine a ratio between them
+    that we can use to normalize our metrics.
+
+    """
+
+    machine_id = str(uuid.getnode())
+    if os.path.isfile(_ETC_MACHINE_ID_PATH):
+        with open(_ETC_MACHINE_ID_PATH, "r") as f:
+            machine_id = f.read()
+
+    elif os.path.isfile(_DBUS_MACHINE_ID_PATH):
+        with open(_DBUS_MACHINE_ID_PATH, "r") as f:
             machine_id = f.read()
 
     return machine_id
@@ -86,6 +119,9 @@ def __init__(self):
         self.installation_id_v2 = str(
             uuid.uuid5(uuid.NAMESPACE_DNS, _get_stable_random_id())
         )
+        self.installation_id_v3 = str(
+            uuid.uuid5(uuid.NAMESPACE_DNS, _get_machine_id_v3())
+        )
 
     @property
     def installation_id(self):

lib/streamlit/report_session.py

@@ -401,6 +401,7 @@ def _enqueue_new_report_message(self):
         imsg.user_info.installation_id = Installation.instance().installation_id
         imsg.user_info.installation_id_v1 = Installation.instance().installation_id_v1
         imsg.user_info.installation_id_v2 = Installation.instance().installation_id_v2
+        imsg.user_info.installation_id_v3 = Installation.instance().installation_id_v3
         if Credentials.get_current().activation:
             imsg.user_info.email = Credentials.get_current().activation.email
         else:

lib/tests/streamlit/metrics_util_test.py

@@ -75,6 +75,50 @@ def test_machine_id_from_node(self):
             machine_id = metrics_util._get_machine_id()
         self.assertEqual(machine_id, MAC)
 
+    def test_machine_id_v3_from_etc(self):
+        """Test getting the machine id from /etc"""
+        file_data = "etc"
+
+        with patch("streamlit.metrics_util.platform.system", return_value=False), patch(
+            "streamlit.metrics_util.uuid.getnode", return_value=MAC
+        ), patch(
+            "streamlit.metrics_util.open", mock_open(read_data=file_data), create=True
+        ), patch(
+            "streamlit.metrics_util.os.path.isfile"
+        ) as path_isfile:
+
+            path_isfile = lambda path: path == "/etc/machine-id"
+
+            machine_id = metrics_util._get_machine_id_v3()
+        self.assertEqual(machine_id, file_data)
+
+    def test_machine_id_v3_from_dbus(self):
+        """Test getting the machine id from /var/lib/dbus"""
+        file_data = "dbus"
+
+        with patch("streamlit.metrics_util.platform.system", return_value=False), patch(
+            "streamlit.metrics_util.uuid.getnode", return_value=MAC
+        ), patch(
+            "streamlit.metrics_util.open", mock_open(read_data=file_data), create=True
+        ), patch(
+            "streamlit.metrics_util.os.path.isfile"
+        ) as path_isfile:
+
+            path_isfile = lambda path: path == "/var/lib/dbus/machine-id"
+
+            machine_id = metrics_util._get_machine_id_v3()
+        self.assertEqual(machine_id, file_data)
+
+    def test_machine_id_v3_from_node(self):
+        """Test getting the machine id as the mac address"""
+
+        with patch("streamlit.metrics_util.platform.system", return_value=False), patch(
+            "streamlit.metrics_util.uuid.getnode", return_value=MAC
+        ), patch("streamlit.metrics_util.os.path.isfile", return_value=False):
+
+            machine_id = metrics_util._get_machine_id_v3()
+        self.assertEqual(machine_id, MAC)
+
     @patch("streamlit.metrics_util.file_util.get_streamlit_file_path", mock_get_path)
     def test_stable_id_not_exists(self):
         """Test creating a stable id """

proto/streamlit/proto/NewReport.proto

@@ -83,6 +83,7 @@ message UserInfo {
   string installation_id = 1;
   string installation_id_v1 = 3;
   string installation_id_v2 = 4;
+  string installation_id_v3 = 5;
   string email = 2;
 }