SQRP-214 ComputationStep.Context telemetry methods add a prefix argumentHEADmaster

This allows us to nudge using a telemetry prefix, while also leaving it off
the statistics keys since it's just noisy when used on those.

2 files changed, 63 insertions, 10 deletions

diff --git a/server/sonar-ce-task/src/main/java/org/sonar/ce/task/step/ComputationStep.java b/server/sonar-ce-task/src/main/java/org/sonar/ce/task/step/ComputationStep.java
index 7ab1e8848f3..dacce723866 100644
--- a/server/sonar-ce-task/src/main/java/org/sonar/ce/task/step/ComputationStep.java
+++ b/server/sonar-ce-task/src/main/java/org/sonar/ce/task/step/ComputationStep.java
@@ -19,12 +19,18 @@
  */
 package org.sonar.ce.task.step;
 
+import javax.annotation.Nullable;
+
 /**
  * A way of splitting the processing of a task into smaller items which can be executed sequentially
  * by {@link ComputationStepExecutor}.
  */
 public interface ComputationStep {
 
+  void execute(Context context);
+
+  String getDescription();
+
   /**
    * Statistics are displayed in the step ending log. They help
    * understanding the runtime context and potential performance hotspots.
@@ -43,6 +49,10 @@ public interface ComputationStep {
    */
   interface Statistics {
     /**
+     * Adds a key-value pair to be logged at the end of the step.
+     * The keys are in a per-step context so do not need to be namespaced
+     * to uniquely identify them across steps, just short names are fine.
+     *
      * @return this
      * @throws NullPointerException if key or value is null
      * @throws IllegalArgumentException if key has already been set
@@ -52,14 +62,47 @@ public interface ComputationStep {
   }
 
   interface Context {
+    /**
+     * Get a statistics object from the current task context. The object
+     * is cleared between each step, and used to log statistics at the
+     * end of the step.
+     *
+     * @return the Statistics instance
+     */
     Statistics getStatistics();
 
-    void addTelemetryMetricOnly(String key, Object value);
+    /**
+     * Add a metric to the telemetry data. The given prefix will be prepended to the key
+     * with a period separating it from the key. Using a prefix is advisable because telemetry
+     * is in a global namespace, but if needed the prefix may be null to leave it off.
+     * The naming convention for keys is to use snake_case with underscores.
+     *<p>
+     * Typically it will make sense to prefer {@link #addTelemetryWithStatistic(String, String, Object)}
+     * so the information is visible in the logs as well as the telemetry. But if you know you're
+     * already logging the information yourself, you might not need it to be a statistic.
+     *</p>
+     * @param telemetryPrefix to prepend to the key with a period separating them, or null for none
+     * @param key key following snake_case convention
+     * @param value telemetry value
+     */
+    void addTelemetryMetricOnly(@Nullable String telemetryPrefix, String key, Object value);
 
-    void addTelemetryWithStatistic(String key, Object value);
+    /**
+     * Adds a metric to the telemetry data and also adds it to the statistics log line at the same
+     * time. The telemetry metric gets the prefix and the statistic does not. For full control of
+     * statistic and telemetry separately, use {@link Context#getStatistics()} and
+     * {@link Context#addTelemetryMetricOnly(String, String, Object)}. The telemetryPrefix is
+     * not nullable on this method because the key really should be different for the telemetry
+     * and the statistic (telemetry is globally namespaced, statistics are per-step).
+     * <p>
+     *   This method is preferred over telemetry only, it can't hurt to show the information in both
+     *   places most of the time.
+     * </p>
+     * 
+     * @param telemetryPrefix to prepend to the key for the telemetry, but not for the statistic
+     * @param key key following snake_case convention
+     * @param value telemetry/statistic value
+     */
+    void addTelemetryWithStatistic(String telemetryPrefix, String key, Object value);
   }
-
-  void execute(Context context);
-
-  String getDescription();
 }
diff --git a/server/sonar-ce-task/src/main/java/org/sonar/ce/task/step/ComputationStepExecutor.java b/server/sonar-ce-task/src/main/java/org/sonar/ce/task/step/ComputationStepExecutor.java
index cd1e3d343f4..ffde161a8c0 100644
--- a/server/sonar-ce-task/src/main/java/org/sonar/ce/task/step/ComputationStepExecutor.java
+++ b/server/sonar-ce-task/src/main/java/org/sonar/ce/task/step/ComputationStepExecutor.java
@@ -19,6 +19,7 @@
  */
 package org.sonar.ce.task.step;
 
+import java.util.Objects;
 import javax.annotation.CheckForNull;
 import javax.annotation.Nullable;
 import org.slf4j.Logger;
@@ -136,19 +137,28 @@ public final class ComputationStepExecutor {
       this.stepsTelemetryHolder = stepsTelemetryHolder;
     }
 
+    private static String prependPrefix(@Nullable String prefix, String key) {
+      if (prefix == null) {
+        return key;
+      } else {
+        return prefix + "." + key;
+      }
+    }
+
     @Override
     public ComputationStep.Statistics getStatistics() {
       return statistics;
     }
 
     @Override
-    public void addTelemetryMetricOnly(String key, Object value) {
-      stepsTelemetryHolder.add(key, value);
+    public void addTelemetryMetricOnly(@Nullable String telemetryPrefix, String key, Object value) {
+      stepsTelemetryHolder.add(prependPrefix(telemetryPrefix, key), value);
     }
 
     @Override
-    public void addTelemetryWithStatistic(String key, Object value) {
-      stepsTelemetryHolder.add(key, value);
+    public void addTelemetryWithStatistic(String telemetryPrefix, String key, Object value) {
+      Objects.requireNonNull(telemetryPrefix);
+      stepsTelemetryHolder.add(prependPrefix(telemetryPrefix, key), value);
       statistics.add(key, value);
     }
   }