SONAR-17735 Telemetry improvements

4 files changed, 65 insertions, 44 deletions

diff --git a/server/sonar-webserver-core/src/main/java/org/sonar/server/telemetry/TelemetryDaemon.java b/server/sonar-webserver-core/src/main/java/org/sonar/server/telemetry/TelemetryDaemon.java
index 1a5c0296c75..106e97d0fd7 100644
--- a/server/sonar-webserver-core/src/main/java/org/sonar/server/telemetry/TelemetryDaemon.java
+++ b/server/sonar-webserver-core/src/main/java/org/sonar/server/telemetry/TelemetryDaemon.java
@@ -50,6 +50,7 @@ public class TelemetryDaemon implements Startable {
   private static final String LOCK_NAME = "TelemetryStat";
   private static final Logger LOG = Loggers.get(TelemetryDaemon.class);
   private static final String LOCK_DELAY_SEC = "sonar.telemetry.lock.delay";
+  static final String I_PROP_MESSAGE_SEQUENCE = "telemetry.messageSeq";
 
   private final TelemetryDataLoader dataLoader;
   private final TelemetryDataJsonWriter dataJsonWriter;
@@ -125,7 +126,7 @@ public class TelemetryDaemon implements Startable {
         long now = system2.now();
         if (shouldUploadStatistics(now)) {
           uploadStatistics();
-          internalProperties.write(I_PROP_LAST_PING, String.valueOf(now));
+          updateTelemetryProps(now);
         }
       } catch (Exception e) {
         LOG.debug("Error while checking SonarQube statistics: {}", e);
@@ -134,6 +135,19 @@ public class TelemetryDaemon implements Startable {
     };
   }
 
+  private void updateTelemetryProps(long now) {
+    internalProperties.write(I_PROP_LAST_PING, String.valueOf(now));
+
+    Optional<String> currentSequence = internalProperties.read(I_PROP_MESSAGE_SEQUENCE);
+    if (currentSequence.isEmpty()) {
+      internalProperties.write(I_PROP_MESSAGE_SEQUENCE, String.valueOf(1));
+      return;
+    }
+
+    long current = Long.parseLong(currentSequence.get());
+    internalProperties.write(I_PROP_MESSAGE_SEQUENCE, String.valueOf(current + 1));
+  }
+
   private void optOut() {
     StringWriter json = new StringWriter();
     try (JsonWriter writer = JsonWriter.of(json)) {
@@ -155,7 +169,7 @@ public class TelemetryDaemon implements Startable {
 
   private boolean shouldUploadStatistics(long now) {
     Optional<Long> lastPing = internalProperties.read(I_PROP_LAST_PING).map(Long::valueOf);
-    return !lastPing.isPresent() || now - lastPing.get() >= ONE_DAY;
+    return lastPing.isEmpty() || now - lastPing.get() >= ONE_DAY;
   }
 
   private int frequency() {
diff --git a/server/sonar-webserver-core/src/main/java/org/sonar/server/telemetry/TelemetryDataLoaderImpl.java b/server/sonar-webserver-core/src/main/java/org/sonar/server/telemetry/TelemetryDataLoaderImpl.java
index 21f524078b1..147d0a7c39a 100644
--- a/server/sonar-webserver-core/src/main/java/org/sonar/server/telemetry/TelemetryDataLoaderImpl.java
+++ b/server/sonar-webserver-core/src/main/java/org/sonar/server/telemetry/TelemetryDataLoaderImpl.java
@@ -29,8 +29,6 @@ import java.util.Optional;
 import java.util.Set;
 import java.util.function.Function;
 import java.util.stream.Collectors;
-import javax.annotation.CheckForNull;
-import javax.annotation.Nullable;
 import javax.inject.Inject;
 import org.sonar.api.config.Configuration;
 import org.sonar.api.platform.Server;
@@ -59,6 +57,7 @@ import static org.sonar.core.config.CorePropertyDefinitions.SONAR_ANALYSIS_DETEC
 import static org.sonar.core.platform.EditionProvider.Edition.COMMUNITY;
 import static org.sonar.core.platform.EditionProvider.Edition.DATACENTER;
 import static org.sonar.core.platform.EditionProvider.Edition.ENTERPRISE;
+import static org.sonar.server.telemetry.TelemetryDaemon.I_PROP_MESSAGE_SEQUENCE;
 
 @ServerSide
 public class TelemetryDataLoaderImpl implements TelemetryDataLoader {
@@ -79,14 +78,11 @@ public class TelemetryDataLoaderImpl implements TelemetryDataLoader {
   private final Configuration configuration;
   private final InternalProperties internalProperties;
   private final DockerSupport dockerSupport;
-  @CheckForNull
-  private final LicenseReader licenseReader;
-
 
   @Inject
   public TelemetryDataLoaderImpl(Server server, DbClient dbClient, PluginRepository pluginRepository,
     PlatformEditionProvider editionProvider, InternalProperties internalProperties, Configuration configuration,
-    DockerSupport dockerSupport, @Nullable LicenseReader licenseReader) {
+    DockerSupport dockerSupport) {
     this.server = server;
     this.dbClient = dbClient;
     this.pluginRepository = pluginRepository;
@@ -94,7 +90,6 @@ public class TelemetryDataLoaderImpl implements TelemetryDataLoader {
     this.internalProperties = internalProperties;
     this.configuration = configuration;
     this.dockerSupport = dockerSupport;
-    this.licenseReader = licenseReader;
   }
 
   private static Database loadDatabaseMetadata(DbSession dbSession) {
@@ -110,12 +105,10 @@ public class TelemetryDataLoaderImpl implements TelemetryDataLoader {
   public TelemetryData load() {
     TelemetryData.Builder data = TelemetryData.builder();
 
+    data.setMessageSequenceNumber(retrieveCurrentMessageSequenceNumber() + 1);
     data.setServerId(server.getId());
     data.setVersion(server.getVersion());
     data.setEdition(editionProvider.get().orElse(null));
-    ofNullable(licenseReader)
-      .flatMap(reader -> licenseReader.read())
-      .ifPresent(license -> data.setLicenseType(license.getType()));
     Function<PluginInfo, String> getVersion = plugin -> plugin.getVersion() == null ? "undefined" : plugin.getVersion().getName();
     Map<String, String> plugins = pluginRepository.getPluginInfos().stream().collect(MoreCollectors.uniqueIndex(PluginInfo::getKey, getVersion));
     data.setPlugins(plugins);
@@ -140,6 +133,10 @@ public class TelemetryDataLoaderImpl implements TelemetryDataLoader {
       .build();
   }
 
+  private Long retrieveCurrentMessageSequenceNumber() {
+    return internalProperties.read(I_PROP_MESSAGE_SEQUENCE).map(Long::parseLong).orElse(0L);
+  }
+
   private void resolveProjectStatistics(TelemetryData.Builder data, DbSession dbSession) {
     List<String> projectUuids = dbClient.projectDao().selectAllProjectUuids(dbSession);
     Map<String, String> scmByProject = getAnalysisPropertyByProject(dbSession, SONAR_ANALYSIS_DETECTEDSCM);
diff --git a/server/sonar-webserver-core/src/test/java/org/sonar/server/telemetry/TelemetryDaemonTest.java b/server/sonar-webserver-core/src/test/java/org/sonar/server/telemetry/TelemetryDaemonTest.java
index ba4186af014..330bcaa0614 100644
--- a/server/sonar-webserver-core/src/test/java/org/sonar/server/telemetry/TelemetryDaemonTest.java
+++ b/server/sonar-webserver-core/src/test/java/org/sonar/server/telemetry/TelemetryDaemonTest.java
@@ -62,6 +62,7 @@ public class TelemetryDaemonTest {
   private static final TelemetryData SOME_TELEMETRY_DATA = TelemetryData.builder()
     .setServerId("foo")
     .setVersion("bar")
+    .setMessageSequenceNumber(1L)
     .setPlugins(Collections.emptyMap())
     .setDatabase(new TelemetryData.Database("H2", "11"))
     .build();
@@ -153,6 +154,7 @@ public class TelemetryDaemonTest {
     long oneDayAgo = today - ONE_DAY - ONE_HOUR;
     internalProperties.write("telemetry.lastPing", String.valueOf(oneDayAgo));
     reset(internalProperties);
+    when(dataLoader.load()).thenReturn(SOME_TELEMETRY_DATA);
     mockDataJsonWriterDoingSomething();
 
     underTest.start();
@@ -177,6 +179,36 @@ public class TelemetryDaemonTest {
     assertThat(logger.logs(LoggerLevel.INFO)).contains("Sharing of SonarQube statistics is disabled.");
   }
 
+  @Test
+  public void write_sequence_as_one_if_not_previously_present() {
+    initTelemetrySettingsToDefaultValues();
+    when(lockManager.tryLock(any(), anyInt())).thenReturn(true);
+    settings.setProperty("sonar.telemetry.frequencyInSeconds", "1");
+    mockDataJsonWriterDoingSomething();
+
+    underTest.start();
+
+    verify(internalProperties, timeout(4_000)).write("telemetry.messageSeq", "1");
+  }
+
+  @Test
+  public void write_sequence_correctly_incremented() {
+    initTelemetrySettingsToDefaultValues();
+    when(lockManager.tryLock(any(), anyInt())).thenReturn(true);
+    settings.setProperty("sonar.telemetry.frequencyInSeconds", "1");
+    internalProperties.write("telemetry.messageSeq", "10");
+    mockDataJsonWriterDoingSomething();
+
+    underTest.start();
+
+    verify(internalProperties, timeout(4_000)).write("telemetry.messageSeq", "10");
+
+    // force another ping
+    internalProperties.write("telemetry.lastPing", String.valueOf(system2.now() - ONE_DAY));
+
+    verify(internalProperties, timeout(4_000)).write("telemetry.messageSeq", "11");
+  }
+
   private void initTelemetrySettingsToDefaultValues() {
     settings.setProperty(SONAR_TELEMETRY_ENABLE.getKey(), SONAR_TELEMETRY_ENABLE.getDefaultValue());
     settings.setProperty(SONAR_TELEMETRY_URL.getKey(), SONAR_TELEMETRY_URL.getDefaultValue());
diff --git a/server/sonar-webserver-core/src/test/java/org/sonar/server/telemetry/TelemetryDataLoaderImplTest.java b/server/sonar-webserver-core/src/test/java/org/sonar/server/telemetry/TelemetryDataLoaderImplTest.java
index 1e384cdeba6..34ceb0b210c 100644
--- a/server/sonar-webserver-core/src/test/java/org/sonar/server/telemetry/TelemetryDataLoaderImplTest.java
+++ b/server/sonar-webserver-core/src/test/java/org/sonar/server/telemetry/TelemetryDataLoaderImplTest.java
@@ -55,7 +55,6 @@ import org.sonar.server.property.MapInternalProperties;
 import org.sonar.updatecenter.common.Version;
 
 import static java.util.Arrays.asList;
-import static org.apache.commons.lang.RandomStringUtils.randomAlphabetic;
 import static org.apache.commons.lang.RandomStringUtils.randomAlphanumeric;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.entry;
@@ -91,12 +90,11 @@ public class TelemetryDataLoaderImplTest {
   private final PlatformEditionProvider editionProvider = mock(PlatformEditionProvider.class);
   private final DockerSupport dockerSupport = mock(DockerSupport.class);
   private final InternalProperties internalProperties = spy(new MapInternalProperties());
-  private final LicenseReader licenseReader = mock(LicenseReader.class);
 
   private final TelemetryDataLoader communityUnderTest = new TelemetryDataLoaderImpl(server, db.getDbClient(), pluginRepository, editionProvider,
-      internalProperties, configuration, dockerSupport, null);
+      internalProperties, configuration, dockerSupport);
   private final TelemetryDataLoader commercialUnderTest = new TelemetryDataLoaderImpl(server, db.getDbClient(), pluginRepository, editionProvider,
-      internalProperties, configuration, dockerSupport, licenseReader);
+      internalProperties, configuration, dockerSupport);
 
   @Test
   public void send_telemetry_data() {
@@ -157,6 +155,7 @@ public class TelemetryDataLoaderImplTest {
     assertThat(data.getServerId()).isEqualTo(serverId);
     assertThat(data.getVersion()).isEqualTo(version);
     assertThat(data.getEdition()).contains(DEVELOPER);
+    assertThat(data.getMessageSequenceNumber()).isOne();
     assertDatabaseMetadata(data.getDatabase());
     assertThat(data.getPlugins()).containsOnly(
       entry("java", "4.12.0.11033"), entry("scmgit", "1.2"), entry("other", "undefined"));
@@ -247,22 +246,6 @@ public class TelemetryDataLoaderImplTest {
   }
 
   @Test
-  public void data_contains_no_license_type_on_community_edition() {
-    TelemetryData data = communityUnderTest.load();
-
-    assertThat(data.getLicenseType()).isEmpty();
-  }
-
-  @Test
-  public void data_contains_no_license_type_on_commercial_edition_if_no_license() {
-    when(licenseReader.read()).thenReturn(Optional.empty());
-
-    TelemetryData data = commercialUnderTest.load();
-
-    assertThat(data.getLicenseType()).isEmpty();
-  }
-
-  @Test
   public void data_contains_weekly_count_sonarlint_users() {
     db.users().insertUser(c -> c.setLastSonarlintConnectionDate(NOW - 100_000L));
     db.users().insertUser(c -> c.setLastSonarlintConnectionDate(NOW));
@@ -276,18 +259,6 @@ public class TelemetryDataLoaderImplTest {
   }
 
   @Test
-  public void data_has_license_type_on_commercial_edition_if_no_license() {
-    String licenseType = randomAlphabetic(12);
-    LicenseReader.License license = mock(LicenseReader.License.class);
-    when(license.getType()).thenReturn(licenseType);
-    when(licenseReader.read()).thenReturn(Optional.of(license));
-
-    TelemetryData data = commercialUnderTest.load();
-
-    assertThat(data.getLicenseType()).contains(licenseType);
-  }
-
-  @Test
   public void send_server_id_and_version() {
     String id = randomAlphanumeric(40);
     String version = randomAlphanumeric(10);
@@ -317,6 +288,13 @@ public class TelemetryDataLoaderImplTest {
   }
 
   @Test
+  public void send_correct_sequence_number() {
+    internalProperties.write(TelemetryDaemon.I_PROP_MESSAGE_SEQUENCE, "10");
+    TelemetryData data = communityUnderTest.load();
+    assertThat(data.getMessageSequenceNumber()).isEqualTo(11L);
+  }
+
+  @Test
   public void do_not_send_server_installation_details_if_missing_property() {
     TelemetryData data = communityUnderTest.load();
     assertThat(data.getInstallationDate()).isNull();