From 9c11bcef1f80da68b53ab4de64a4b75e884977b4 Mon Sep 17 00:00:00 2001
From: Struan Shrimpton <sshrimp@google.com>
Date: Wed, 4 Jun 2025 14:15:56 -0700
Subject: [PATCH] telemetry: Collect until opt-out

The pdd says we will collect by default:
https://eldar.corp.google.com/assessments/570486509/revisions/1/sections/550004#questions/550404/revisions/2

Also reduce the notice count to 9 since it notices on "0 remaining"

Bug: 326277821
Change-Id: I4de584c36868b45cefaa5cea42f88d245485dce7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/6616983
Reviewed-by: Ben Pastene <bpastene@chromium.org>
Commit-Queue: Struan Shrimpton <sshrimp@google.com>
---
 infra_lib/telemetry/__init__.py        |  5 ++---
 infra_lib/telemetry/config.py          | 14 +++++++++-----
 infra_lib/telemetry/config_unittest.py | 22 ++++++++++------------
 3 files changed, 21 insertions(+), 20 deletions(-)

diff --git a/infra_lib/telemetry/__init__.py b/infra_lib/telemetry/__init__.py
index cad9b3806..97c6aacfd 100644
--- a/infra_lib/telemetry/__init__.py
+++ b/infra_lib/telemetry/__init__.py
@@ -76,6 +76,8 @@ def initialize(service_name,
         return
 
     cfg = config.Config(cfg_file)
+    if cfg.disabled():
+        return
 
     if not cfg.trace_config.has_enabled():
         if cfg.root_config.notice_countdown > -1:
@@ -89,9 +91,6 @@ def initialize(service_name,
 
         cfg.flush()
 
-    if not cfg.trace_config.enabled:
-        return
-
     default_resource = otel_resources.Resource.create({
         otel_resources.SERVICE_NAME:
         service_name,
diff --git a/infra_lib/telemetry/config.py b/infra_lib/telemetry/config.py
index 24b0a8639..2c0c5bcab 100644
--- a/infra_lib/telemetry/config.py
+++ b/infra_lib/telemetry/config.py
@@ -23,7 +23,7 @@ ENABLED_REASON_KEY = "enabled_reason"
 TRACE_SECTION_KEY = "trace"
 DEFAULT_CONFIG = {
     ROOT_SECTION_KEY: {
-        NOTICE_COUNTDOWN_KEY: 10
+        NOTICE_COUNTDOWN_KEY: 9
     },
     TRACE_SECTION_KEY: {},
 }
@@ -42,12 +42,13 @@ class TraceConfig:
     def __init__(self, config: configparser.ConfigParser) -> None:
         self._config = config
 
+        if not self.has_enabled() or self.enabled:
+            self.gen_id()
+
     def update(self, enabled: bool, reason: Literal["AUTO", "USER"]) -> None:
         """Update the config."""
         self._config[TRACE_SECTION_KEY][ENABLED_KEY] = str(enabled)
         self._config[TRACE_SECTION_KEY][ENABLED_REASON_KEY] = reason
-        if enabled:
-            self.gen_id()
 
     def gen_id(self, regen=False) -> None:
         """[Re]generate UUIDs."""
@@ -89,6 +90,10 @@ class TraceConfig:
         """Checks if the enabled property exists in config."""
         return ENABLED_KEY in self._config[TRACE_SECTION_KEY]
 
+    def disabled(self) -> bool:
+        """Checks if the enabled probperty exists and is false"""
+        return self.trace_config.has_enabled() and not self.trace_config.enabled
+
     @property
     def enabled(self) -> bool:
         """Value of trace.enabled property in telemetry.cfg."""
@@ -131,8 +136,7 @@ class RootConfig:
     @property
     def notice_countdown(self) -> int:
         """Value for root.notice_countdown property in telemetry.cfg."""
-
-        return self._config[ROOT_SECTION_KEY].getint(NOTICE_COUNTDOWN_KEY, 10)
+        return self._config[ROOT_SECTION_KEY].getint(NOTICE_COUNTDOWN_KEY, 9)
 
 
 class Config:
diff --git a/infra_lib/telemetry/config_unittest.py b/infra_lib/telemetry/config_unittest.py
index c167888aa..54f3f5836 100644
--- a/infra_lib/telemetry/config_unittest.py
+++ b/infra_lib/telemetry/config_unittest.py
@@ -21,12 +21,12 @@ class ConfigTest(unittest.TestCase):
             cfg = config.Config(path)
 
             with open(path, 'r') as f:
-                self.assertEqual(
-                    f.read(), "[root]\nnotice_countdown = 10\n\n[trace]\n\n")
+                self.assertEqual(f.read(),
+                                 "[root]\nnotice_countdown = 9\n\n[trace]\n\n")
             self.assertFalse(cfg.trace_config.enabled)
             self.assertFalse(cfg.trace_config.has_enabled())
             self.assertEqual("AUTO", cfg.trace_config.enabled_reason)
-            self.assertEqual(10, cfg.root_config.notice_countdown)
+            self.assertEqual(9, cfg.root_config.notice_countdown)
 
     def test_load_config_file(self) -> None:
         """Test Config to load config file."""
@@ -60,19 +60,17 @@ class ConfigTest(unittest.TestCase):
             cfg.flush()
 
             with open(path, 'r') as f:
-                self.assertEqual(
-                    f.read(),
+                file = f.read()
+                self.assertIn(
                     "\n".join([
                         "[root]",
                         "notice_countdown = 9",
-                        "",
-                        "[trace]",
-                        "enabled = False",
-                        "enabled_reason = AUTO",
-                        "",
-                        "",
                     ]),
+                    file,
                 )
+                self.assertIn("[trace]", file)
+                self.assertIn("enabled = False", file)
+                self.assertIn("enabled_reason = AUTO", file)
 
 
 def test_default_trace_config() -> None:
@@ -111,7 +109,7 @@ def test_default_root_config() -> None:
     cfg[config.ROOT_SECTION_KEY] = {}
     root_config = config.RootConfig(cfg)
 
-    assert root_config.notice_countdown == 10
+    assert root_config.notice_countdown == 9
 
 
 def test_root_config_update() -> None: