fix: add check in Telemetry agent for timestamp during publish (#1270)

aws-greengrass · Jul 25, 2022 · 06026ee · 06026ee
1 parent c73fca8
commit 06026ee
Show file tree

Hide file tree

Showing 2 changed files with 34 additions and 10 deletions.
diff --git a/src/main/java/com/aws/greengrass/telemetry/TelemetryAgent.java b/src/main/java/com/aws/greengrass/telemetry/TelemetryAgent.java
@@ -246,19 +246,35 @@ void aggregatePeriodicMetrics() {
     /**
      * Helper for metrics uploader. Also used in tests.
      */
+    @SuppressWarnings("PMD.AvoidCatchingThrowable")
     void publishPeriodicMetrics() {
-        if (!isConnected.get()) {
-            logger.atDebug().log("Cannot publish the metrics. MQTT connection interrupted.");
-            return;
-        }
+        Map<Long, List<AggregatedNamespaceData>> metricsToPublishMap = null;
         long timestamp = Instant.now().toEpochMilli();
         long lastPublish = Coerce.toLong(getPeriodicPublishTimeTopic());
-        Map<Long, List<AggregatedNamespaceData>> metricsToPublishMap =
-                metricsAggregator.getMetricsToPublish(lastPublish, timestamp);
-        getPeriodicPublishTimeTopic().withValue(timestamp);
-        // TODO: [P41214679] Do not publish if the metrics are empty.
-        publisher.publish(MetricsPayload.builder().build(), metricsToPublishMap.get(timestamp));
-        logger.atInfo().event("telemetry-metrics-published").log("Telemetry metrics update published.");
+        try {
+            if (!isConnected.get()) {
+                logger.atDebug().log("Cannot publish the metrics. MQTT connection interrupted");
+                return;
+            }
+            metricsToPublishMap = metricsAggregator.getMetricsToPublish(lastPublish, timestamp);
+            getPeriodicPublishTimeTopic().withValue(timestamp);
+            if (metricsToPublishMap != null && metricsToPublishMap.containsKey(timestamp)) {
+                publisher.publish(MetricsPayload.builder().build(), metricsToPublishMap.get(timestamp));
+                logger.atInfo().event("telemetry-metrics-published").log("Telemetry metrics update published");
+            }
+        } catch (Throwable t) {
+            logger.atWarn().log("Error collecting telemetry. Will retry", t);
+            return;
+        }
+        try {
+            getPeriodicPublishTimeTopic().withValue(timestamp);
+            if (metricsToPublishMap != null && metricsToPublishMap.containsKey(timestamp)) {
+                publisher.publish(MetricsPayload.builder().build(), metricsToPublishMap.get(timestamp));
+                logger.atInfo().event("telemetry-metrics-published").log("Telemetry metrics update published");
+            }
+        } catch (Throwable t) {
+            logger.atWarn().log("Error publishing telemetry. Will retry", t);
+        }
     }
 
     private Topic getPeriodicPublishTimeTopic() {

diff --git a/src/test/java/com/aws/greengrass/telemetry/TelemetryAgentTest.java b/src/test/java/com/aws/greengrass/telemetry/TelemetryAgentTest.java
@@ -33,6 +33,7 @@
 import java.nio.file.Path;
 import java.time.Instant;
 import java.util.ArrayList;
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
@@ -54,6 +55,7 @@
 import static com.aws.greengrass.telemetry.TelemetryAgent.TELEMETRY_TEST_PERIODIC_AGGREGATE_INTERVAL_SEC;
 import static com.aws.greengrass.telemetry.TelemetryAgent.TELEMETRY_TEST_PERIODIC_PUBLISH_INTERVAL_SEC;
 import static com.aws.greengrass.testcommons.testutilities.ExceptionLogProtector.ignoreExceptionUltimateCauseOfType;
+import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
@@ -248,4 +250,10 @@ void GIVEN_Telemetry_Agent_WHEN_mqtt_is_interrupted_THEN_aggregation_continues_b
         // aggregation is continued irrespective of the mqtt connection
         verify(ma, timeout(timeoutMs).atLeastOnce()).aggregateMetrics(anyLong(), anyLong());
     }
+
+    @Test
+    void GIVEN_no_metrics_to_publish_WHEN_publish_THEN_finishes_without_exception() {
+        when(ma.getMetricsToPublish(anyLong(), anyLong())).thenReturn(Collections.emptyMap());
+        assertDoesNotThrow(() -> telemetryAgent.publishPeriodicMetrics());
+    }
 }