From 32fe10d332caffb54e3d11c2c51347f76e022468 Mon Sep 17 00:00:00 2001 From: svenvg93 Date: Wed, 4 Feb 2026 21:38:10 +0000 Subject: [PATCH] chore: refactor prometheus to handle missing data --- app/Services/PrometheusMetricsService.php | 188 ++++++---------------- tests/Feature/MetricsEndpointTest.php | 58 +++++++ 2 files changed, 110 insertions(+), 136 deletions(-) diff --git a/app/Services/PrometheusMetricsService.php b/app/Services/PrometheusMetricsService.php index 08473a6df..a405608d8 100644 --- a/app/Services/PrometheusMetricsService.php +++ b/app/Services/PrometheusMetricsService.php @@ -105,142 +105,35 @@ protected function registerMetrics(CollectorRegistry $registry, Result $result): ); $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); - - // Download jitter - $downloadJitterGauge = $registry->getOrRegisterGauge( - 'speedtest_tracker', - 'download_jitter_ms', - 'Download jitter in milliseconds', - $labelNames - ); - $downloadJitterGauge->set($result->download_jitter, $labelValues); - - // Upload jitter - $uploadJitterGauge = $registry->getOrRegisterGauge( - 'speedtest_tracker', - 'upload_jitter_ms', - 'Upload jitter in milliseconds', - $labelNames - ); - $uploadJitterGauge->set($result->upload_jitter, $labelValues); - - // Packet loss - $packetLossGauge = $registry->getOrRegisterGauge( - 'speedtest_tracker', - 'packet_loss_percent', - 'Packet loss percentage', - $labelNames - ); - $packetLossGauge->set($result->packet_loss, $labelValues); - - // Ping latency low/high - $pingLowGauge = $registry->getOrRegisterGauge( - 'speedtest_tracker', - 'ping_low_ms', - 'Ping low latency in milliseconds', - $labelNames - ); - $pingLowGauge->set($result->ping_low, $labelValues); - - $pingHighGauge = $registry->getOrRegisterGauge( - 'speedtest_tracker', - 'ping_high_ms', - 'Ping high latency in milliseconds', - $labelNames - ); - $pingHighGauge->set($result->ping_high, $labelValues); - - // 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); + // 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); + + // Packet loss - optional + $this->registerGaugeIfNotNull($registry, 'packet_loss_percent', 'Packet loss percentage', $labelNames, $labelValues, $result->packet_loss); + + // 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); + + // 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); + + // 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); + + // 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); + + // 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 @@ protected function emptyMetrics(): string { 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); + } + } } diff --git a/tests/Feature/MetricsEndpointTest.php b/tests/Feature/MetricsEndpointTest.php index cadf0ce28..a1609fce1 100644 --- a/tests/Feature/MetricsEndpointTest.php +++ b/tests/Feature/MetricsEndpointTest.php @@ -123,4 +123,62 @@ $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'); + }); });