SONARPY-2464 Add safety around the telemetry mechanism

Author: ghislainpiot

sonar-python-plugin/src/main/java/org/sonar/plugins/python/IPynbSensor.java

@@ -92,18 +92,18 @@ private void processNotebooksFiles(List<PythonInputFile> pythonFiles, SensorCont
     PythonIndexer pythonIndexer = new SonarQubePythonIndexer(pythonFiles, cacheContext, context);
     PythonScanner scanner = new PythonScanner(context, checks, fileLinesContextFactory, noSonarFilter, PythonParser.createIPythonParser(), pythonIndexer);
     scanner.execute(pythonFiles, context);
-    sensorTelemetryStorage.updateMetric(SensorTelemetryStorage.MetricKey.NOTEBOOK_RECOGNITION_ERROR_KEY, String.valueOf(scanner.getRecognitionErrorCount()));
+    sensorTelemetryStorage.updateMetric(TelemetryMetricKey.NOTEBOOK_RECOGNITION_ERROR_KEY, scanner.getRecognitionErrorCount());
   }
 
   private List<PythonInputFile> parseNotebooks(List<PythonInputFile> pythonFiles, SensorContext context) {
     List<PythonInputFile> generatedIPythonFiles = new ArrayList<>();
 
-    sensorTelemetryStorage.updateMetric(SensorTelemetryStorage.MetricKey.NOTEBOOK_TOTAL_KEY, String.valueOf(pythonFiles.size()));
+    sensorTelemetryStorage.updateMetric(TelemetryMetricKey.NOTEBOOK_TOTAL_KEY, pythonFiles.size());
     var numberOfExceptions = 0;
 
     for (PythonInputFile inputFile : pythonFiles) {
       try {
-        sensorTelemetryStorage.updateMetric(SensorTelemetryStorage.MetricKey.NOTEBOOK_PRESENT_KEY, "1");
+        sensorTelemetryStorage.updateMetric(TelemetryMetricKey.NOTEBOOK_PRESENT_KEY, "1");
         var result = IpynbNotebookParser.parseNotebook(inputFile);
         result.ifPresent(generatedIPythonFiles::add);
       } catch (Exception e) {
@@ -114,7 +114,7 @@ private List<PythonInputFile> parseNotebooks(List<PythonInputFile> pythonFiles,
       }
     }
 
-    sensorTelemetryStorage.updateMetric(SensorTelemetryStorage.MetricKey.NOTEBOOK_EXCEPTION_KEY, String.valueOf(numberOfExceptions));
+    sensorTelemetryStorage.updateMetric(TelemetryMetricKey.NOTEBOOK_EXCEPTION_KEY, String.valueOf(numberOfExceptions));
     return generatedIPythonFiles;
   }
 

sonar-python-plugin/src/main/java/org/sonar/plugins/python/SensorTelemetryStorage.java

@@ -16,42 +16,43 @@
  */
 package org.sonar.plugins.python;
 
-import java.util.HashMap;
+import java.util.EnumMap;
 import java.util.Map;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.sonar.api.batch.sensor.SensorContext;
+import org.sonar.api.utils.Version;
 
 public class SensorTelemetryStorage {
   private static final Logger LOG = LoggerFactory.getLogger(SensorTelemetryStorage.class);
 
-  private final Map<String, String> data = new HashMap<>();
+  private final Map<TelemetryMetricKey, String> data = new EnumMap<>(TelemetryMetricKey.class);
 
   public void send(SensorContext sensorContext) {
-    data.forEach((k, v) -> {
-      LOG.info("Metrics property: {}={}", k, v);
-      sensorContext.addTelemetryProperty(k, v);
-    });
+    // This try/catch block should be useless, as in the worst case it should be a no-op depending on the SensorContext implementation
+    // It exists to be extra sure for the LTA
+    try {
+      var apiVersion = sensorContext.runtime().getApiVersion();
+      if (apiVersion.isGreaterThanOrEqual(Version.create(10, 9))) {
+        data.forEach((k, v) -> {
+          LOG.info("Collected metric: {}={}", k, v);
+          sensorContext.addTelemetryProperty(k.key(), v);
+        });
+
+      } else {
+        LOG.info("Skipping sending metrics because the plugin API version is {}", apiVersion);
+      }
+    } catch (Exception e) {
+      LOG.error("Failed to send metrics", e);
+    }
   }
 
-  public void updateMetric(MetricKey key, String value) {
-    data.put(key.key(), value);
+  public void updateMetric(TelemetryMetricKey key, String value) {
+    data.put(key, value);
   }
 
-  public enum MetricKey {
-    NOTEBOOK_PRESENT_KEY("python.notebook.present"),
-    NOTEBOOK_TOTAL_KEY("python.notebook.total"),
-    NOTEBOOK_RECOGNITION_ERROR_KEY("python.notebook.recognition_error"),
-    NOTEBOOK_EXCEPTION_KEY("python.notebook.exceptions");
-
-    private final String key;
-
-    MetricKey(String key) {
-      this.key = key;
-    }
-
-    public String key() {
-      return key;
-    }
+  public void updateMetric(TelemetryMetricKey key, int value) {
+    data.put(key, String.valueOf(value));
   }
+
 }

sonar-python-plugin/src/main/java/org/sonar/plugins/python/TelemetryMetricKey.java

@@ -0,0 +1,34 @@
+/*
+ * SonarQube Python Plugin
+ * Copyright (C) 2011-2024 SonarSource SA
+ * mailto:info AT sonarsource DOT com
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the Sonar Source-Available License Version 1, as published by SonarSource SA.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
+ * See the Sonar Source-Available License for more details.
+ *
+ * You should have received a copy of the Sonar Source-Available License
+ * along with this program; if not, see https://sonarsource.com/license/ssal/
+ */
+package org.sonar.plugins.python;
+
+public enum TelemetryMetricKey {
+  NOTEBOOK_PRESENT_KEY("python.notebook.present"),
+  NOTEBOOK_TOTAL_KEY("python.notebook.total"),
+  NOTEBOOK_RECOGNITION_ERROR_KEY("python.notebook.recognition_error"),
+  NOTEBOOK_EXCEPTION_KEY("python.notebook.exceptions");
+
+  private final String key;
+
+  TelemetryMetricKey(String key) {
+    this.key = key;
+  }
+
+  public String key() {
+    return key;
+  }
+}

sonar-python-plugin/src/test/java/org/sonar/plugins/python/IPynbSensorTest.java

@@ -218,10 +218,10 @@ void test_notebook_sensor_is_executed_on_json_file() {
     var sensor = spy(notebookSensor());
     var contextSpy = spy(context);
     assertDoesNotThrow(() -> sensor.execute(contextSpy));
-    verify(contextSpy, Mockito.times(1)).addTelemetryProperty(SensorTelemetryStorage.MetricKey.NOTEBOOK_PRESENT_KEY.key(), "1");
-    verify(contextSpy, Mockito.times(1)).addTelemetryProperty(SensorTelemetryStorage.MetricKey.NOTEBOOK_RECOGNITION_ERROR_KEY.key(), "0");
-    verify(contextSpy, Mockito.times(1)).addTelemetryProperty(SensorTelemetryStorage.MetricKey.NOTEBOOK_TOTAL_KEY.key(), "1");
-    verify(contextSpy, Mockito.times(1)).addTelemetryProperty(SensorTelemetryStorage.MetricKey.NOTEBOOK_EXCEPTION_KEY.key(), "0");
+    verify(contextSpy, Mockito.times(1)).addTelemetryProperty(TelemetryMetricKey.NOTEBOOK_PRESENT_KEY.key(), "1");
+    verify(contextSpy, Mockito.times(1)).addTelemetryProperty(TelemetryMetricKey.NOTEBOOK_RECOGNITION_ERROR_KEY.key(), "0");
+    verify(contextSpy, Mockito.times(1)).addTelemetryProperty(TelemetryMetricKey.NOTEBOOK_TOTAL_KEY.key(), "1");
+    verify(contextSpy, Mockito.times(1)).addTelemetryProperty(TelemetryMetricKey.NOTEBOOK_EXCEPTION_KEY.key(), "0");
   }
 
   @Test
@@ -243,9 +243,9 @@ void test_notebook_sensor_parse_error_on_valid_line() {
     sensor.execute(contextSpy);
     var logs = String.join("", logTester.logs());
     assertThat(logs).contains("Unable to parse file: notebook_parse_error.ipynbParse error at line 1");
-    verify(contextSpy, Mockito.times(1)).addTelemetryProperty(SensorTelemetryStorage.MetricKey.NOTEBOOK_PRESENT_KEY.key(), "1");
-    verify(contextSpy, Mockito.times(1)).addTelemetryProperty(SensorTelemetryStorage.MetricKey.NOTEBOOK_RECOGNITION_ERROR_KEY.key(), "1");
-    verify(contextSpy, Mockito.times(1)).addTelemetryProperty(SensorTelemetryStorage.MetricKey.NOTEBOOK_TOTAL_KEY.key(), "1");
-    verify(contextSpy, Mockito.times(1)).addTelemetryProperty(SensorTelemetryStorage.MetricKey.NOTEBOOK_EXCEPTION_KEY.key(), "0");
+    verify(contextSpy, Mockito.times(1)).addTelemetryProperty(TelemetryMetricKey.NOTEBOOK_PRESENT_KEY.key(), "1");
+    verify(contextSpy, Mockito.times(1)).addTelemetryProperty(TelemetryMetricKey.NOTEBOOK_RECOGNITION_ERROR_KEY.key(), "1");
+    verify(contextSpy, Mockito.times(1)).addTelemetryProperty(TelemetryMetricKey.NOTEBOOK_TOTAL_KEY.key(), "1");
+    verify(contextSpy, Mockito.times(1)).addTelemetryProperty(TelemetryMetricKey.NOTEBOOK_EXCEPTION_KEY.key(), "0");
   }
 }

sonar-python-plugin/src/test/java/org/sonar/plugins/python/SensorTelemetryStorageTest.java

@@ -0,0 +1,80 @@
+/*
+ * SonarQube Python Plugin
+ * Copyright (C) 2011-2024 SonarSource SA
+ * mailto:info AT sonarsource DOT com
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the Sonar Source-Available License Version 1, as published by SonarSource SA.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
+ * See the Sonar Source-Available License for more details.
+ *
+ * You should have received a copy of the Sonar Source-Available License
+ * along with this program; if not, see https://sonarsource.com/license/ssal/
+ */
+package org.sonar.plugins.python;
+
+import java.io.File;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.RegisterExtension;
+import org.slf4j.event.Level;
+import org.sonar.api.SonarEdition;
+import org.sonar.api.SonarQubeSide;
+import org.sonar.api.batch.sensor.SensorContext;
+import org.sonar.api.batch.sensor.internal.SensorContextTester;
+import org.sonar.api.internal.SonarRuntimeImpl;
+import org.sonar.api.testfixtures.log.LogTesterJUnit5;
+import org.sonar.api.utils.Version;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.doThrow;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.spy;
+import static org.mockito.Mockito.verify;
+
+class SensorTelemetryStorageTest {
+
+  @RegisterExtension
+  public LogTesterJUnit5 logTester = new LogTesterJUnit5().setLevel(Level.DEBUG);
+
+  @Test
+  void no_send_on_incompatible_version() {
+    var sensorContext = sensorContext(Version.create(10, 8));
+    var storage = new SensorTelemetryStorage();
+    storage.updateMetric(TelemetryMetricKey.NOTEBOOK_PRESENT_KEY, "1");
+    storage.send(sensorContext);
+
+    verify(sensorContext, never()).addTelemetryProperty(any(), any());
+    assertThat(logTester.logs()).contains("Skipping sending metrics because the plugin API version is 10.8");
+  }
+
+  @Test
+  void send_after_10_9() {
+    var sensorContext = sensorContext(Version.create(10, 9));
+    var storage = new SensorTelemetryStorage();
+    storage.updateMetric(TelemetryMetricKey.NOTEBOOK_PRESENT_KEY, "1");
+    storage.send(sensorContext);
+
+    verify(sensorContext).addTelemetryProperty(TelemetryMetricKey.NOTEBOOK_PRESENT_KEY.key(), "1");
+  }
+
+  @Test
+  void no_crash_on_exception() {
+    var sensorContext = sensorContext(Version.create(10, 9));
+    doThrow(new RuntimeException("Some exception")).when(sensorContext).addTelemetryProperty(any(), any());
+    var storage = new SensorTelemetryStorage();
+    storage.updateMetric(TelemetryMetricKey.NOTEBOOK_PRESENT_KEY, "1");
+    Assertions.assertDoesNotThrow(() -> storage.send(sensorContext));
+    assertThat(logTester.logs()).contains("Failed to send metrics");
+  }
+
+  private SensorContext sensorContext(Version version) {
+    var sensorContext = spy(SensorContextTester.create(new File("")));
+    sensorContext.setRuntime(SonarRuntimeImpl.forSonarQube(version, SonarQubeSide.SERVER, SonarEdition.DEVELOPER));
+    return sensorContext;
+  }
+}