From 3ff3ad36bf25045538cfb813536ef6b513327f15 Mon Sep 17 00:00:00 2001 From: Thibault Le Ouay Date: Wed, 8 Jul 2026 21:39:30 +0200 Subject: [PATCH] probe: fix dns otel and private regions (#2358) --- apps/checker/handlers/dns.go | 9 + apps/checker/handlers/otel_wiring_test.go | 171 ++++++++++++++++++ apps/checker/handlers/tcp.go | 18 +- apps/checker/pkg/otel/otel.go | 39 +++- apps/checker/pkg/otel/otel_test.go | 61 +++++++ apps/checker/pkg/scheduler/scheduler.go | 10 +- apps/checker/pkg/scheduler/scheduler_test.go | 10 + .../pages/docs/concept/private-locations.mdx | 2 - 8 files changed, 303 insertions(+), 17 deletions(-) create mode 100644 apps/checker/handlers/otel_wiring_test.go diff --git a/apps/checker/handlers/dns.go b/apps/checker/handlers/dns.go index 9f4df05d..11a6dbd7 100644 --- a/apps/checker/handlers/dns.go +++ b/apps/checker/handlers/dns.go @@ -11,6 +11,7 @@ import ( "github.com/google/uuid" "github.com/openstatushq/openstatus/apps/checker/checker" "github.com/openstatushq/openstatus/apps/checker/pkg/assertions" + otelOS "github.com/openstatushq/openstatus/apps/checker/pkg/otel" "github.com/openstatushq/openstatus/apps/checker/request" "github.com/rs/zerolog/log" @@ -222,6 +223,10 @@ func (h Handler) DNSHandler(c *gin.Context) { log.Ctx(ctx).Error().Err(err).Msg("failed to send event to tinybird") } + if req.OtelConfig.Endpoint != "" { + otelOS.RecordDNSMetrics(ctx, req, latency, err != nil || !isSuccessful, h.Region) + } + event, f := c.Get("event") if f { t := event.(map[string]any) @@ -342,6 +347,10 @@ func (h Handler) DNSHandlerRegion(c *gin.Context) { } } + if req.OtelConfig.Endpoint != "" { + otelOS.RecordDNSMetrics(ctx, req, latency, err != nil || !isSuccessful, h.Region) + } + if err != nil { c.JSON(http.StatusOK, gin.H{"message": "uri not reachable"}) return diff --git a/apps/checker/handlers/otel_wiring_test.go b/apps/checker/handlers/otel_wiring_test.go new file mode 100644 index 00000000..1b913e42 --- /dev/null +++ b/apps/checker/handlers/otel_wiring_test.go @@ -0,0 +1,171 @@ +package handlers_test + +import ( + "encoding/json" + "io" + "net" + "net/http" + "net/http/httptest" + "strings" + "sync/atomic" + "testing" + "time" + + "github.com/gin-gonic/gin" + "github.com/openstatushq/openstatus/apps/checker/handlers" + "github.com/openstatushq/openstatus/apps/checker/pkg/tinybird" + "github.com/openstatushq/openstatus/apps/checker/request" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// countingOTLPServer accepts any OTLP export and counts the requests it receives. +func countingOTLPServer(t *testing.T) (*httptest.Server, *int64) { + t.Helper() + var count int64 + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + _, _ = io.Copy(io.Discard, r.Body) + atomic.AddInt64(&count, 1) + w.WriteHeader(http.StatusOK) + })) + t.Cleanup(server.Close) + return server, &count +} + +func testTinybird(t *testing.T) tinybird.Client { + t.Helper() + hclient := &http.Client{Transport: RoundTripFunc(func(req *http.Request) *http.Response { + return &http.Response{ + StatusCode: http.StatusAccepted, + Body: io.NopCloser(strings.NewReader(`{}`)), + } + })} + return tinybird.NewClient(hclient, "apiKey") +} + +func TestTCPHandler_ExportsOTLPOnSuccess(t *testing.T) { + ln, err := net.Listen("tcp", "127.0.0.1:0") + require.NoError(t, err) + t.Cleanup(func() { ln.Close() }) + + otlp, count := countingOTLPServer(t) + + h := handlers.Handler{ + TbClient: testTinybird(t), + Secret: "test", + Region: "local", + } + router := gin.New() + router.POST("/checker/tcp", h.TCPHandler) + + req := request.TCPCheckerRequest{ + URI: ln.Addr().String(), + WorkspaceID: "1", + MonitorID: "1", + Status: "active", // avoids the network UpdateStatus call + Timeout: 5, + Retry: 1, + } + req.OtelConfig.Endpoint = otlp.URL + body, _ := json.Marshal(req) + + w := httptest.NewRecorder() + r, _ := http.NewRequest(http.MethodPost, "/checker/tcp", strings.NewReader(string(body))) + r.Header.Set("Authorization", "Basic test") + router.ServeHTTP(w, r) + + assert.Equal(t, http.StatusOK, w.Code) + assert.Eventually(t, func() bool { return atomic.LoadInt64(count) > 0 }, 5*time.Second, 50*time.Millisecond, + "expected an OTLP export on TCP success") +} + +func TestTCPHandlerRegion_ExportsOTLPOnFailure(t *testing.T) { + otlp, count := countingOTLPServer(t) + + h := handlers.Handler{ + TbClient: testTinybird(t), + Secret: "test", + Region: "local", + } + router := gin.New() + router.POST("/tcp/:region", h.TCPHandlerRegion) + + req := request.TCPCheckerRequest{ + URI: "127.0.0.1:1", // connection refused + Status: "active", + Timeout: 5, + } + req.OtelConfig.Endpoint = otlp.URL + body, _ := json.Marshal(req) + + w := httptest.NewRecorder() + r, _ := http.NewRequest(http.MethodPost, "/tcp/local", strings.NewReader(string(body))) + r.Header.Set("Authorization", "Basic test") + router.ServeHTTP(w, r) + + assert.Equal(t, http.StatusOK, w.Code) + assert.Eventually(t, func() bool { return atomic.LoadInt64(count) > 0 }, 10*time.Second, 50*time.Millisecond, + "expected an OTLP export on TCP failure") +} + +func TestDNSHandler_ExportsOTLPOnFailure(t *testing.T) { + otlp, count := countingOTLPServer(t) + + h := handlers.Handler{ + TbClient: testTinybird(t), + Secret: "test", + Region: "local", + } + router := gin.New() + router.POST("/checker/dns", h.DNSHandler) + + req := request.DNSCheckerRequest{ + URI: "nonexistent-host-openstatus-test.invalid", + WorkspaceID: "1", + MonitorID: "1", + Status: "error", // resolution failure keeps isSuccessful=true, so no UpdateStatus + Retry: 1, + } + req.OtelConfig.Endpoint = otlp.URL + body, _ := json.Marshal(req) + + w := httptest.NewRecorder() + r, _ := http.NewRequest(http.MethodPost, "/checker/dns", strings.NewReader(string(body))) + r.Header.Set("Authorization", "Basic test") + router.ServeHTTP(w, r) + + assert.Equal(t, http.StatusOK, w.Code) + assert.Eventually(t, func() bool { return atomic.LoadInt64(count) > 0 }, 10*time.Second, 50*time.Millisecond, + "expected an OTLP export on DNS failure") +} + +func TestDNSHandlerRegion_ExportsOTLPOnFailure(t *testing.T) { + otlp, count := countingOTLPServer(t) + + h := handlers.Handler{ + TbClient: testTinybird(t), + Secret: "test", + Region: "local", + } + router := gin.New() + router.POST("/dns/:region", h.DNSHandlerRegion) + + req := request.DNSCheckerRequest{ + URI: "nonexistent-host-openstatus-test.invalid", + WorkspaceID: "1", + MonitorID: "1", + Status: "error", + Retry: 1, + } + req.OtelConfig.Endpoint = otlp.URL + body, _ := json.Marshal(req) + + w := httptest.NewRecorder() + r, _ := http.NewRequest(http.MethodPost, "/dns/local", strings.NewReader(string(body))) + r.Header.Set("Authorization", "Basic test") + router.ServeHTTP(w, r) + + assert.Equal(t, http.StatusOK, w.Code) + assert.Eventually(t, func() bool { return atomic.LoadInt64(count) > 0 }, 10*time.Second, 50*time.Millisecond, + "expected an OTLP export on DNS failure") +} diff --git a/apps/checker/handlers/tcp.go b/apps/checker/handlers/tcp.go index 0e2e2565..1a46f860 100644 --- a/apps/checker/handlers/tcp.go +++ b/apps/checker/handlers/tcp.go @@ -237,6 +237,11 @@ func (h Handler) TCPHandler(c *gin.Context) { CronTimestamp: req.CronTimestamp, }) + response.Error = 1 + } + + if req.OtelConfig.Endpoint != "" { + otelOS.RecordTCPMetrics(ctx, req, response, h.Region) } returnData := c.Query("data") @@ -339,16 +344,19 @@ func (h Handler) TCPHandlerRegion(c *gin.Context) { return nil } - if err := backoff.Retry(op, backoff.WithMaxRetries(backoff.NewExponentialBackOff(), 3)); err != nil { - c.JSON(http.StatusOK, gin.H{"message": "uri not reachable"}) - - return + err := backoff.Retry(op, backoff.WithMaxRetries(backoff.NewExponentialBackOff(), 3)) + if err != nil { + response.Error = 1 } if req.OtelConfig.Endpoint != "" { - otelOS.RecordTCPMetrics(ctx, req, response, region) + } + + if err != nil { + c.JSON(http.StatusOK, gin.H{"message": "uri not reachable"}) + return } c.JSON(http.StatusOK, response) diff --git a/apps/checker/pkg/otel/otel.go b/apps/checker/pkg/otel/otel.go index 5e0a9b96..51f83a0b 100644 --- a/apps/checker/pkg/otel/otel.go +++ b/apps/checker/pkg/otel/otel.go @@ -103,6 +103,16 @@ func recordErrorCounter(ctx context.Context, meter metric.Meter, att metric.Meas counter.Add(ctx, 1, att) } +func recordStatusCounter(ctx context.Context, meter metric.Meter, att metric.MeasurementOption) { + counter, err := meter.Int64Counter("openstatus.status", metric.WithDescription("Status of the check")) + if err != nil { + log.Ctx(ctx).Error().Err(err).Msg("Error setting up counter") + return + } + + counter.Add(ctx, 1, att) +} + func RecordHTTPMetrics(ctx context.Context, req request.HttpCheckerRequest, result checker.Response, region string) { withMeter(ctx, req.OtelConfig.Endpoint, req.OtelConfig.Headers, func(meter metric.Meter) { att := metric.WithAttributes( @@ -116,12 +126,7 @@ func RecordHTTPMetrics(ctx context.Context, req request.HttpCheckerRequest, resu return } - status, err := meter.Int64Counter("openstatus.status", metric.WithDescription("Status of the check")) - if err != nil { - log.Ctx(ctx).Error().Err(err).Msg("Error setting up counter") - } - - status.Add(ctx, 1, att) + recordStatusCounter(ctx, meter, att) timings := []struct { name string @@ -144,6 +149,26 @@ func RecordHTTPMetrics(ctx context.Context, req request.HttpCheckerRequest, resu }) } +func RecordDNSMetrics(ctx context.Context, req request.DNSCheckerRequest, latency int64, isError bool, region string) { + withMeter(ctx, req.OtelConfig.Endpoint, req.OtelConfig.Headers, func(meter metric.Meter) { + att := metric.WithAttributes( + attribute.String("openstatus.probes", region), + attribute.String("openstatus.target", req.URI), + ) + + if isError { + recordErrorCounter(ctx, meter, att) + return + } + + recordStatusCounter(ctx, meter, att) + + if err := recordGauge(ctx, meter, "openstatus.dns.request.duration", "Duration of the check", float64(latency), att); err != nil { + log.Ctx(ctx).Error().Err(err).Str("metric", "openstatus.dns.request.duration").Msg("Error creating gauge") + } + }) +} + func RecordTCPMetrics(ctx context.Context, req request.TCPCheckerRequest, result checker.TCPResponse, region string) { withMeter(ctx, req.OtelConfig.Endpoint, req.OtelConfig.Headers, func(meter metric.Meter) { att := metric.WithAttributes( @@ -156,6 +181,8 @@ func RecordTCPMetrics(ctx context.Context, req request.TCPCheckerRequest, result return } + recordStatusCounter(ctx, meter, att) + timings := []struct { name string description string diff --git a/apps/checker/pkg/otel/otel_test.go b/apps/checker/pkg/otel/otel_test.go index 7d1d1e19..628e46f5 100644 --- a/apps/checker/pkg/otel/otel_test.go +++ b/apps/checker/pkg/otel/otel_test.go @@ -120,6 +120,28 @@ func TestRecordErrorCounter(t *testing.T) { assert.Equal(t, int64(1), sum.DataPoints[0].Value) } +// --- recordStatusCounter tests --- + +func TestRecordStatusCounter(t *testing.T) { + meter, reader := newTestMeter(t) + ctx := context.Background() + att := metric.WithAttributes(attribute.String("region", "us-east-1")) + + recordStatusCounter(ctx, meter, att) + + rm := collectMetrics(t, reader) + require.Len(t, rm.ScopeMetrics, 1) + require.Len(t, rm.ScopeMetrics[0].Metrics, 1) + + m := rm.ScopeMetrics[0].Metrics[0] + assert.Equal(t, "openstatus.status", m.Name) + + sum, ok := m.Data.(metricdata.Sum[int64]) + require.True(t, ok, "expected Sum[int64] data type") + require.Len(t, sum.DataPoints, 1) + assert.Equal(t, int64(1), sum.DataPoints[0].Value) +} + // --- setupOTelSDK tests --- func TestSetupOTelSDK(t *testing.T) { @@ -291,3 +313,42 @@ func TestRecordTCPMetrics_SetupFailure(t *testing.T) { // Must not panic — same nil pointer guard as HTTP. RecordTCPMetrics(context.Background(), req, result, "us-east-1") } + +// --- RecordDNSMetrics tests --- + +func TestRecordDNSMetrics_Success(t *testing.T) { + server := newOTLPTestServer(t) + + req := request.DNSCheckerRequest{ + URI: "example.com", + MonitorID: "mon-3", + } + req.OtelConfig.Endpoint = server.URL + + // Should not panic. + RecordDNSMetrics(context.Background(), req, 30, false, "us-east-1") +} + +func TestRecordDNSMetrics_Error(t *testing.T) { + server := newOTLPTestServer(t) + + req := request.DNSCheckerRequest{ + URI: "example.com", + MonitorID: "mon-3", + } + req.OtelConfig.Endpoint = server.URL + + // Should record error counter and not panic. + RecordDNSMetrics(context.Background(), req, 0, true, "us-east-1") +} + +func TestRecordDNSMetrics_SetupFailure(t *testing.T) { + req := request.DNSCheckerRequest{ + URI: "example.com", + MonitorID: "mon-3", + } + req.OtelConfig.Endpoint = "://invalid" + + // Must not panic — same nil pointer guard as HTTP. + RecordDNSMetrics(context.Background(), req, 30, false, "us-east-1") +} diff --git a/apps/checker/pkg/scheduler/scheduler.go b/apps/checker/pkg/scheduler/scheduler.go index 0ab4dc00..396e1593 100644 --- a/apps/checker/pkg/scheduler/scheduler.go +++ b/apps/checker/pkg/scheduler/scheduler.go @@ -116,12 +116,13 @@ func (mm *MonitorManager) UpdateMonitors(ctx context.Context) { FuncWithTaskContext: func(ctx tasks.TaskContext) error { monitor := m + c := context.Background() log.Printf("Starting TCP job for monitor %s (%s)", monitor.Id, monitor.Uri) - data, err := mm.JobRunner.TCPJob(ctx.Context, monitor) + data, err := mm.JobRunner.TCPJob(c, monitor) if err != nil { log.Printf("TCP monitor check failed for %s (%s): %v", monitor.Id, monitor.Uri, err) } - resp, ingestErr := mm.Client.IngestTCP(ctx.Context, &connect.Request[v1.IngestTCPRequest]{ + resp, ingestErr := mm.Client.IngestTCP(c, &connect.Request[v1.IngestTCPRequest]{ Msg: &v1.IngestTCPRequest{ MonitorId: monitor.Id, Id: data.ID, @@ -167,12 +168,13 @@ func (mm *MonitorManager) UpdateMonitors(ctx context.Context) { FuncWithTaskContext: func(ctx tasks.TaskContext) error { monitor := m + c := context.Background() log.Printf("Starting TCP job for monitor %s (%s)", monitor.Id, monitor.Uri) - _, err := mm.JobRunner.DNSJob(ctx.Context, monitor) + _, err := mm.JobRunner.DNSJob(c, monitor) if err != nil { log.Printf("TCP monitor check failed for %s (%s): %v", monitor.Id, monitor.Uri, err) } - resp, ingestErr := mm.Client.IngestDNS(ctx.Context, &connect.Request[v1.IngestDNSRequest]{ + resp, ingestErr := mm.Client.IngestDNS(c, &connect.Request[v1.IngestDNSRequest]{ Msg: &v1.IngestDNSRequest{ MonitorId: monitor.Id, diff --git a/apps/checker/pkg/scheduler/scheduler_test.go b/apps/checker/pkg/scheduler/scheduler_test.go index 9642707e..7fdf8001 100644 --- a/apps/checker/pkg/scheduler/scheduler_test.go +++ b/apps/checker/pkg/scheduler/scheduler_test.go @@ -65,6 +65,10 @@ func TestMonitorManager_StartAndStopJobs_WithJobRunner(t *testing.T) { httpMonitor := &v1.HTTPMonitor{Id: "http1", Url: "http://openstat.us", Periodicity: "10s"} tcpMonitor := &v1.TCPMonitor{Id: "tcp1", Uri: "openstatus:80", Periodicity: "10s"} + // Regression guard: the tasks scheduler never populates TaskContext.Context, + // so jobs must pass a non-nil context to the connect client or IngestTCP panics. + var tcpIngestCtxNonNil atomic.Bool + client := &mockClient{ MonitorsFunc: func(ctx context.Context, req *connect.Request[v1.MonitorsRequest]) (*connect.Response[v1.MonitorsResponse], error) { return connect.NewResponse(&v1.MonitorsResponse{ @@ -76,6 +80,9 @@ func TestMonitorManager_StartAndStopJobs_WithJobRunner(t *testing.T) { return connect.NewResponse(&v1.IngestHTTPResponse{}), nil }, IngestTCPFunc: func(ctx context.Context, req *connect.Request[v1.IngestTCPRequest]) (*connect.Response[v1.IngestTCPResponse], error) { + if ctx != nil { + tcpIngestCtxNonNil.Store(true) + } return connect.NewResponse(&v1.IngestTCPResponse{}), nil }, @@ -101,6 +108,9 @@ func TestMonitorManager_StartAndStopJobs_WithJobRunner(t *testing.T) { if !jobRunner.TCPJobCalled.Load() == true { t.Errorf("expected TCPJob to be called") } + if !tcpIngestCtxNonNil.Load() { + t.Errorf("expected IngestTCP to receive a non-nil context") + } // Remove monitors and ensure jobs are stopped client.MonitorsFunc = func(ctx context.Context, req *connect.Request[v1.MonitorsRequest]) (*connect.Response[v1.MonitorsResponse], error) { diff --git a/apps/web/src/content/pages/docs/concept/private-locations.mdx b/apps/web/src/content/pages/docs/concept/private-locations.mdx index cab1d0ab..6b02a600 100644 --- a/apps/web/src/content/pages/docs/concept/private-locations.mdx +++ b/apps/web/src/content/pages/docs/concept/private-locations.mdx @@ -54,8 +54,6 @@ Running a private location moves operational responsibility to you. Plan for: ## Current limitations -Private locations are in **beta**. Two capabilities are not yet wired up: - - **No automatic incidents.** A failing check from a private location does not currently open an incident or fire notifications. If you need alerting today, keep at least one public location on the monitor. - **Not displayed on public status pages.** Monitors assigned only to private locations won't appear on a public status page as a monitor-linked component. -- 2.51.2