chore: refactor prometheus to handle missing data (#2696)

Co-authored-by: Alex Justesen <alexjustesen@users.noreply.github.com>

app/Services/PrometheusMetricsService.php

@@ -105,142 +105,35 @@ class PrometheusMetricsService
         );
         $pingGauge->set($result->ping, $labelValues);
 
-        // Ping jitter
-        $pingJitterGauge = $registry->getOrRegisterGauge(
-            'speedtest_tracker',
-            'ping_jitter_ms',
-            'Ping jitter in milliseconds',
-            $labelNames
-        );
-        $pingJitterGauge->set($result->ping_jitter, $labelValues);
+        // Jitter metrics - optional, may not be present in all test results
+        $this->registerGaugeIfNotNull($registry, 'ping_jitter_ms', 'Ping jitter in milliseconds', $labelNames, $labelValues, $result->ping_jitter);
+        $this->registerGaugeIfNotNull($registry, 'download_jitter_ms', 'Download jitter in milliseconds', $labelNames, $labelValues, $result->download_jitter);
+        $this->registerGaugeIfNotNull($registry, 'upload_jitter_ms', 'Upload jitter in milliseconds', $labelNames, $labelValues, $result->upload_jitter);
 
-        // Download jitter
-        $downloadJitterGauge = $registry->getOrRegisterGauge(
-            'speedtest_tracker',
-            'download_jitter_ms',
-            'Download jitter in milliseconds',
-            $labelNames
-        );
-        $downloadJitterGauge->set($result->download_jitter, $labelValues);
+        // Packet loss - optional
+        $this->registerGaugeIfNotNull($registry, 'packet_loss_percent', 'Packet loss percentage', $labelNames, $labelValues, $result->packet_loss);
 
-        // Upload jitter
-        $uploadJitterGauge = $registry->getOrRegisterGauge(
-            'speedtest_tracker',
-            'upload_jitter_ms',
-            'Upload jitter in milliseconds',
-            $labelNames
-        );
-        $uploadJitterGauge->set($result->upload_jitter, $labelValues);
+        // Ping latency metrics - optional
+        $this->registerGaugeIfNotNull($registry, 'ping_low_ms', 'Ping low latency in milliseconds', $labelNames, $labelValues, $result->ping_low);
+        $this->registerGaugeIfNotNull($registry, 'ping_high_ms', 'Ping high latency in milliseconds', $labelNames, $labelValues, $result->ping_high);
 
-        // Packet loss
-        $packetLossGauge = $registry->getOrRegisterGauge(
-            'speedtest_tracker',
-            'packet_loss_percent',
-            'Packet loss percentage',
-            $labelNames
-        );
-        $packetLossGauge->set($result->packet_loss, $labelValues);
+        // Download latency metrics - optional (IQM = Interquartile Mean - more reliable than average)
+        $this->registerGaugeIfNotNull($registry, 'download_latency_iqm_ms', 'Download latency interquartile mean in milliseconds', $labelNames, $labelValues, $result->downloadlatencyiqm);
+        $this->registerGaugeIfNotNull($registry, 'download_latency_low_ms', 'Download latency low in milliseconds', $labelNames, $labelValues, $result->downloadlatency_low);
+        $this->registerGaugeIfNotNull($registry, 'download_latency_high_ms', 'Download latency high in milliseconds', $labelNames, $labelValues, $result->downloadlatency_high);
 
-        // Ping latency low/high
-        $pingLowGauge = $registry->getOrRegisterGauge(
-            'speedtest_tracker',
-            'ping_low_ms',
-            'Ping low latency in milliseconds',
-            $labelNames
-        );
-        $pingLowGauge->set($result->ping_low, $labelValues);
+        // Upload latency metrics - optional
+        $this->registerGaugeIfNotNull($registry, 'upload_latency_iqm_ms', 'Upload latency interquartile mean in milliseconds', $labelNames, $labelValues, $result->uploadlatencyiqm);
+        $this->registerGaugeIfNotNull($registry, 'upload_latency_low_ms', 'Upload latency low in milliseconds', $labelNames, $labelValues, $result->uploadlatency_low);
+        $this->registerGaugeIfNotNull($registry, 'upload_latency_high_ms', 'Upload latency high in milliseconds', $labelNames, $labelValues, $result->uploadlatency_high);
 
-        $pingHighGauge = $registry->getOrRegisterGauge(
-            'speedtest_tracker',
-            'ping_high_ms',
-            'Ping high latency in milliseconds',
-            $labelNames
-        );
-        $pingHighGauge->set($result->ping_high, $labelValues);
+        // Bytes transferred during test - optional
+        $this->registerGaugeIfNotNull($registry, 'downloaded_bytes', 'Total bytes downloaded during test', $labelNames, $labelValues, $result->downloaded_bytes);
+        $this->registerGaugeIfNotNull($registry, 'uploaded_bytes', 'Total bytes uploaded during test', $labelNames, $labelValues, $result->uploaded_bytes);
 
-        // Download latency metrics (IQM = Interquartile Mean - more reliable than average)
-        $downloadLatencyIqmGauge = $registry->getOrRegisterGauge(
-            'speedtest_tracker',
-            'download_latency_iqm_ms',
-            'Download latency interquartile mean in milliseconds',
-            $labelNames
-        );
-        $downloadLatencyIqmGauge->set($result->downloadlatencyiqm, $labelValues);
-
-        $downloadLatencyLowGauge = $registry->getOrRegisterGauge(
-            'speedtest_tracker',
-            'download_latency_low_ms',
-            'Download latency low in milliseconds',
-            $labelNames
-        );
-        $downloadLatencyLowGauge->set($result->downloadlatency_low, $labelValues);
-
-        $downloadLatencyHighGauge = $registry->getOrRegisterGauge(
-            'speedtest_tracker',
-            'download_latency_high_ms',
-            'Download latency high in milliseconds',
-            $labelNames
-        );
-        $downloadLatencyHighGauge->set($result->downloadlatency_high, $labelValues);
-
-        // Upload latency metrics
-        $uploadLatencyIqmGauge = $registry->getOrRegisterGauge(
-            'speedtest_tracker',
-            'upload_latency_iqm_ms',
-            'Upload latency interquartile mean in milliseconds',
-            $labelNames
-        );
-        $uploadLatencyIqmGauge->set($result->uploadlatencyiqm, $labelValues);
-
-        $uploadLatencyLowGauge = $registry->getOrRegisterGauge(
-            'speedtest_tracker',
-            'upload_latency_low_ms',
-            'Upload latency low in milliseconds',
-            $labelNames
-        );
-        $uploadLatencyLowGauge->set($result->uploadlatency_low, $labelValues);
-
-        $uploadLatencyHighGauge = $registry->getOrRegisterGauge(
-            'speedtest_tracker',
-            'upload_latency_high_ms',
-            'Upload latency high in milliseconds',
-            $labelNames
-        );
-        $uploadLatencyHighGauge->set($result->uploadlatency_high, $labelValues);
-
-        // Bytes transferred during test
-        $downloadedBytesGauge = $registry->getOrRegisterGauge(
-            'speedtest_tracker',
-            'downloaded_bytes',
-            'Total bytes downloaded during test',
-            $labelNames
-        );
-        $downloadedBytesGauge->set($result->downloaded_bytes, $labelValues);
-
-        $uploadedBytesGauge = $registry->getOrRegisterGauge(
-            'speedtest_tracker',
-            'uploaded_bytes',
-            'Total bytes uploaded during test',
-            $labelNames
-        );
-        $uploadedBytesGauge->set($result->uploaded_bytes, $labelValues);
-
-        // Test duration
-        $downloadElapsedGauge = $registry->getOrRegisterGauge(
-            'speedtest_tracker',
-            'download_elapsed_ms',
-            'Download test duration in milliseconds',
-            $labelNames
-        );
-        $downloadElapsedGauge->set($result->download_elapsed, $labelValues);
-
-        $uploadElapsedGauge = $registry->getOrRegisterGauge(
-            'speedtest_tracker',
-            'upload_elapsed_ms',
-            'Upload test duration in milliseconds',
-            $labelNames
-        );
-        $uploadElapsedGauge->set($result->upload_elapsed, $labelValues);
+        // Test duration - optional
+        $this->registerGaugeIfNotNull($registry, 'download_elapsed_ms', 'Download test duration in milliseconds', $labelNames, $labelValues, $result->download_elapsed);
+        $this->registerGaugeIfNotNull($registry, 'upload_elapsed_ms', 'Upload test duration in milliseconds', $labelNames, $labelValues, $result->upload_elapsed);
     }
 
     protected function buildLabels(Result $result): array
@@ -262,4 +155,27 @@ class PrometheusMetricsService
     {
         return "# no data available\n";
     }
+
+    /**
+     * Register a gauge metric only if the value is not null.
+     * Follows Prometheus best practice of not exporting missing metrics.
+     */
+    protected function registerGaugeIfNotNull(
+        CollectorRegistry $registry,
+        string $name,
+        string $help,
+        array $labelNames,
+        array $labelValues,
+        mixed $value
+    ): void {
+        if ($value !== null) {
+            $gauge = $registry->getOrRegisterGauge(
+                'speedtest_tracker',
+                $name,
+                $help,
+                $labelNames
+            );
+            $gauge->set($value, $labelValues);
+        }
+    }
 }

tests/Feature/MetricsEndpointTest.php

@@ -123,4 +123,62 @@ describe('metrics endpoint', function () {
 
         $response->assertSuccessful();
     });
+
+    test('handles results with missing packet loss data', function () {
+        app(DataIntegrationSettings::class)->fill([
+            'prometheus_enabled' => true,
+            'prometheus_allowed_ips' => [],
+        ])->save();
+
+        // Create a result without packet loss data
+        $dataWithoutPacketLoss = json_decode('{"isp": "Speedtest Communications", "ping": {"low": 17.841, "high": 24.077, "jitter": 1.878, "latency": 19.133}, "type": "result", "result": {"id": "d6fe2fb3-f4f8-4cc5-b898-7b42109e67c2", "url": "https://docs.speedtest-tracker.dev", "persisted": true}, "server": {"id": 0, "ip": "127.0.0.1", "host": "docs.speedtest-tracker.dev", "name": "Speedtest", "port": 8080, "country": "United States", "location": "New York City, NY"}, "upload": {"bytes": 124297377, "elapsed": 9628, "latency": {"iqm": 341.111, "low": 16.663, "high": 529.86, "jitter": 37.587}, "bandwidth": 113750000}, "download": {"bytes": 230789788, "elapsed": 14301, "latency": {"iqm": 104.125, "low": 23.72, "high": 269.563, "jitter": 13.447}, "bandwidth": 115625000}, "interface": {"name": "eth0", "isVpn": false, "macAddr": "00:00:00:00:00:00", "externalIp": "127.0.0.1", "internalIp": "127.0.0.1"}, "timestamp": "2024-03-01T01:00:00Z"}', true);
+
+        Result::factory()->create([
+            'ping' => $dataWithoutPacketLoss['ping']['latency'],
+            'download' => $dataWithoutPacketLoss['download']['bandwidth'],
+            'upload' => $dataWithoutPacketLoss['upload']['bandwidth'],
+            'data' => $dataWithoutPacketLoss,
+        ]);
+
+        $response = $this->get('/prometheus');
+
+        $response->assertSuccessful();
+        $response->assertHeader('Content-Type', 'text/plain; version=0.0.4; charset=utf-8');
+        // Verify packet_loss metric is not in the output when data is missing
+        expect($response->getContent())->not->toContain('speedtest_tracker_packet_loss_percent');
+    });
+
+    test('handles failed speedtests by only exporting info metric', function () {
+        app(DataIntegrationSettings::class)->fill([
+            'prometheus_enabled' => true,
+            'prometheus_allowed_ips' => [],
+        ])->save();
+
+        // Create a failed result
+        $failedData = json_decode('{"type": "log", "level": "error", "message": "Connection timeout", "timestamp": "2024-03-01T01:00:00Z"}', true);
+
+        $result = Result::factory()->create([
+            'status' => \App\Enums\ResultStatus::Failed,
+            'data' => $failedData,
+        ]);
+
+        // Cache the result ID so the Prometheus service can find it
+        Cache::forever('prometheus:latest_result', $result->id);
+
+        $response = $this->get('/prometheus');
+
+        $response->assertSuccessful();
+        $response->assertHeader('Content-Type', 'text/plain; version=0.0.4; charset=utf-8');
+
+        $content = $response->getContent();
+
+        // Should have the info metric (result_id)
+        expect($content)->toContain('speedtest_tracker_result_id');
+
+        // Should NOT have numeric metrics for failed tests
+        expect($content)->not->toContain('speedtest_tracker_download_bytes');
+        expect($content)->not->toContain('speedtest_tracker_upload_bytes');
+        expect($content)->not->toContain('speedtest_tracker_ping_ms');
+        expect($content)->not->toContain('speedtest_tracker_packet_loss_percent');
+    });
 });