From ddb04b4b70ce3aec8c5bdedc4febfaf6aa0288b7 Mon Sep 17 00:00:00 2001 From: Thibault Le Ouay Ducasse Date: Wed, 21 Oct 2026 10:57:51 +0200 Subject: [PATCH] checker: split cron check logic out of the gin handlers Run{HTTP,TCP,DNS,ICMP,GRPC}Check probe and record one cron check and return an error only when the request is invalid (InvalidRequestError) or the result could not be recorded. The HTTP handlers keep their responses; this is the seam the Pub/Sub consumer will call into. Co-Authored-By: Claude Opus 5.5 --- apps/checker/handlers/checker.go | 82 ++++++------- apps/checker/handlers/dns.go | 68 +++++------ apps/checker/handlers/grpc.go | 80 ++++++------- apps/checker/handlers/handler.go | 73 ++++++++++++ apps/checker/handlers/icmp.go | 80 ++++++------- apps/checker/handlers/run_check_test.go | 147 ++++++++++++++++++++++++ apps/checker/handlers/tcp.go | 80 ++++++------- 7 files changed, 381 insertions(+), 229 deletions(-) create mode 100644 apps/checker/handlers/run_check_test.go diff --git a/apps/checker/handlers/checker.go b/apps/checker/handlers/checker.go index 936c05595..f7a0f5021 100644 --- a/apps/checker/handlers/checker.go +++ b/apps/checker/handlers/checker.go @@ -1,6 +1,7 @@ package handlers import ( + "context" "encoding/json" "fmt" "net/http" @@ -46,26 +47,11 @@ type PingData struct { func (h Handler) HTTPCheckerHandler(c *gin.Context) { ctx := c.Request.Context() - const defaultRetry = 3 - dataSourceName := "ping_response__v8" - - if c.GetHeader("Authorization") != fmt.Sprintf("Basic %s", h.Secret) { - c.JSON(http.StatusUnauthorized, gin.H{"error": "unauthorized"}) + if !h.acceptCron(c) { return } - if h.CloudProvider == "fly" { - // if the request has been routed to a wrong region, we forward it to the correct one. - region := c.GetHeader("fly-prefer-region") - if region != "" && region != h.Region { - c.Header("fly-replay", fmt.Sprintf("region=%s", region)) - c.String(http.StatusAccepted, "Forwarding request to %s", region) - - return - } - } - var req request.HttpCheckerRequest if err := c.ShouldBindJSON(&req); err != nil { log.Ctx(ctx).Error().Err(err).Msg("failed to decode checker request") @@ -74,6 +60,31 @@ func (h Handler) HTTPCheckerHandler(c *gin.Context) { return } + setCheckerEvent(c, req.URL, req.WorkspaceID, req.MonitorID, cronTrigger(req.Trigger), "http") + + result, _ := h.RunHTTPCheck(ctx, req) + + returnData := c.Query("data") + if returnData == "true" { + + if len(result.Body) > 1024 { + result.Body = result.Body[:1000] + } + + c.JSON(http.StatusOK, result) + + return + } + + c.JSON(http.StatusOK, nil) +} + +// RunHTTPCheck probes and records one cron HTTP check. A nil error means the +// result was recorded, whether the monitor passed or not. +func (h Handler) RunHTTPCheck(ctx context.Context, req request.HttpCheckerRequest) (checker.Response, error) { + const defaultRetry = 3 + dataSourceName := "ping_response__v8" + ctx = withMonitorLogger(ctx, req.MonitorID, req.WorkspaceID) requestClient := checker.NewHTTPClient(time.Duration(req.Timeout)*time.Millisecond, req.FollowRedirects) @@ -88,15 +99,14 @@ func (h Handler) HTTPCheckerHandler(c *gin.Context) { assertionAsString = "" } - trigger := "cron" - if req.Trigger != "" { - trigger = req.Trigger - } + trigger := cronTrigger(req.Trigger) var called int var result checker.Response + var recordErr error + retry := defaultRetry if req.Retry != 0 { retry = int(req.Retry) @@ -241,20 +251,7 @@ func (h Handler) HTTPCheckerHandler(c *gin.Context) { if err := h.TbClient.SendEvent(ctx, data, dataSourceName); err != nil { log.Ctx(ctx).Error().Err(err).Msg("failed to send event to tinybird") - } - - - e, f := c.Get("event") - if f { - t := e.(map[string]any) - t["checker"] = map[string]string{ - "uri": req.URL, - "workspace_id": req.WorkspaceID, - "monitor_id":req.MonitorID, - "trigger": trigger, - "type": "http", - } - c.Set("event", t) + recordErr = err } return struct{}{}, nil @@ -264,7 +261,7 @@ func (h Handler) HTTPCheckerHandler(c *gin.Context) { id, e := uuid.NewV7() if e != nil { log.Ctx(ctx).Error().Err(e).Msg("failed to send event to tinybird") - return + return result, e } data := PingData{ @@ -286,6 +283,7 @@ func (h Handler) HTTPCheckerHandler(c *gin.Context) { if err := h.TbClient.SendEvent(ctx, data, dataSourceName); err != nil { log.Ctx(ctx).Error().Err(err).Msg("failed to send event to tinybird") + recordErr = err } if req.Status != "error" { @@ -303,19 +301,7 @@ func (h Handler) HTTPCheckerHandler(c *gin.Context) { otelOS.RecordHTTPMetrics(ctx, req, result, h.Region) } - returnData := c.Query("data") - if returnData == "true" { - - if len(result.Body) > 1024 { - result.Body = result.Body[:1000] - } - - c.JSON(http.StatusOK, result) - - return - } - - c.JSON(http.StatusOK, nil) + return result, recordErr } func EvaluateHTTPAssertions(raw []json.RawMessage, data PingData, res checker.Response) (bool, error) { diff --git a/apps/checker/handlers/dns.go b/apps/checker/handlers/dns.go index 22efe01c8..d8aa3425e 100644 --- a/apps/checker/handlers/dns.go +++ b/apps/checker/handlers/dns.go @@ -60,25 +60,11 @@ func (d DNSResponse) tinybirdEvent() (dnsTinybirdEvent, error) { func (h Handler) DNSHandler(c *gin.Context) { ctx := c.Request.Context() - const defaultRetry = 3 - dataSourceName := "dns_response__v0" - // Authorization check - if c.GetHeader("Authorization") != fmt.Sprintf("Basic %s", h.Secret) { - c.JSON(http.StatusUnauthorized, gin.H{"error": "unauthorized"}) + if !h.acceptCron(c) { return } - // Fly region forwarding - if h.CloudProvider == "fly" { - region := c.GetHeader("fly-prefer-region") - if region != "" && region != h.Region { - c.Header("fly-replay", fmt.Sprintf("region=%s", region)) - c.String(http.StatusAccepted, "Forwarding request to %s", region) - return - } - } - // Parse request var req request.DNSCheckerRequest if err := c.ShouldBindJSON(&req); err != nil { @@ -87,24 +73,35 @@ func (h Handler) DNSHandler(c *gin.Context) { return } + setCheckerEvent(c, req.URI, req.WorkspaceID, req.MonitorID, cronTrigger(req.Trigger), "dns") + + data, err := h.RunDNSCheck(ctx, req) + if respondInvalid(c, err) { + return + } + + c.JSON(http.StatusOK, data) +} + +// RunDNSCheck probes and records one cron DNS check. A nil error means the +// result was recorded, whether the monitor passed or not. +func (h Handler) RunDNSCheck(ctx context.Context, req request.DNSCheckerRequest) (DNSResponse, error) { + const defaultRetry = 3 + dataSourceName := "dns_response__v0" + ctx = withMonitorLogger(ctx, req.MonitorID, req.WorkspaceID) workspaceId, err := strconv.ParseInt(req.WorkspaceID, 10, 64) if err != nil { - c.JSON(http.StatusBadRequest, gin.H{"error": "invalid workspace id"}) - return + return DNSResponse{}, InvalidRequestError{Reason: "invalid workspace id"} } monitorId, err := strconv.ParseInt(req.MonitorID, 10, 64) if err != nil { - c.JSON(http.StatusBadRequest, gin.H{"error": "invalid monitor id"}) - return + return DNSResponse{}, InvalidRequestError{Reason: "invalid monitor id"} } - trigger := req.Trigger - if trigger == "" { - trigger = "cron" - } + trigger := cronTrigger(req.Trigger) retry := defaultRetry if req.Retry != 0 { @@ -114,7 +111,7 @@ func (h Handler) DNSHandler(c *gin.Context) { id, e := uuid.NewV7() if e != nil { log.Ctx(ctx).Error().Err(e).Msg("failed to generate UUID") - return + return DNSResponse{}, e } statusMap := map[string]string{ @@ -144,6 +141,7 @@ func (h Handler) DNSHandler(c *gin.Context) { op := func() (*checker.DnsResponse, error) { called++ + isSuccessful = true log.Ctx(ctx).Debug().Msgf("performing dns check for %s (attempt %d/%d)", req.URI, called, retry) attemptCtx, cancel := dnsAttemptContext(ctx, req.Timeout) defer cancel() @@ -152,6 +150,8 @@ func (h Handler) DNSHandler(c *gin.Context) { latency = time.Now().UTC().UnixMilli() - start if err != nil { + // A resolver error is a failed check, not a pass with no records. + isSuccessful = false return nil, err } if len(req.RawAssertions) > 0 { @@ -188,7 +188,7 @@ func (h Handler) DNSHandler(c *gin.Context) { // Status update logic switch { case !isSuccessful: - log.Ctx(ctx).Debug().Msg("DNS check failed assertions") + log.Ctx(ctx).Debug().Msg("DNS check failed") data.RequestStatus = "error" data.Error = 1 data.ErrorMessage = err.Error() @@ -222,30 +222,20 @@ func (h Handler) DNSHandler(c *gin.Context) { data.RequestStatus = "success" } + var recordErr error if tbEvent, err := data.tinybirdEvent(); err != nil { log.Ctx(ctx).Error().Err(err).Msg("failed to marshal dns records") + recordErr = err } else if err := h.TbClient.SendEvent(ctx, tbEvent, dataSourceName); err != nil { log.Ctx(ctx).Error().Err(err).Msg("failed to send event to tinybird") + recordErr = err } 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) - t["checker"] = map[string]string{ - "uri": req.URI, - "workspace_id": req.WorkspaceID, - "monitor_id": req.MonitorID, - "trigger": trigger, - "type": "dns", - } - c.Set("event", t) - } - - c.JSON(http.StatusOK, data) + return data, recordErr } func (h Handler) DNSHandlerRegion(c *gin.Context) { diff --git a/apps/checker/handlers/grpc.go b/apps/checker/handlers/grpc.go index 3faf3113a..608a29261 100644 --- a/apps/checker/handlers/grpc.go +++ b/apps/checker/handlers/grpc.go @@ -1,6 +1,7 @@ package handlers import ( + "context" "encoding/json" "fmt" "net/http" @@ -51,25 +52,11 @@ func grpcCheck(req request.GRPCCheckerRequest) (checker.GRPCResult, error) { func (h Handler) GRPCHandler(c *gin.Context) { ctx := c.Request.Context() - dataSourceName := "grpc_response__v0" - - if c.GetHeader("Authorization") != fmt.Sprintf("Basic %s", h.Secret) { - c.JSON(http.StatusUnauthorized, gin.H{"error": "unauthorized"}) + if !h.acceptCron(c) { return } - if h.CloudProvider == "fly" { - // if the request has been routed to a wrong region, we forward it to the correct one. - region := c.GetHeader("fly-prefer-region") - if region != "" && region != h.Region { - c.Header("fly-replay", fmt.Sprintf("region=%s", region)) - c.String(http.StatusAccepted, "Forwarding request to %s", region) - - return - } - } - var req request.GRPCCheckerRequest if err := c.ShouldBindJSON(&req); err != nil { log.Ctx(ctx).Error().Err(err).Msg("failed to decode checker request") @@ -78,41 +65,45 @@ func (h Handler) GRPCHandler(c *gin.Context) { return } - ctx = withMonitorLogger(ctx, req.MonitorID, req.WorkspaceID) - - workspaceId, err := strconv.ParseInt(req.WorkspaceID, 10, 64) - if err != nil { - c.JSON(http.StatusBadRequest, gin.H{"error": "invalid request"}) + setCheckerEvent(c, req.URI, req.WorkspaceID, req.MonitorID, cronTrigger(req.Trigger), "grpc") + response, err := h.RunGRPCCheck(ctx, req) + if respondInvalid(c, err) { return } - monitorId, err := strconv.ParseInt(req.MonitorID, 10, 64) - if err != nil { - c.JSON(http.StatusBadRequest, gin.H{"error": "invalid request"}) + returnData := c.Query("data") + if returnData == "true" { + c.JSON(http.StatusOK, response) return } - var trigger = "cron" - if req.Trigger != "" { - trigger = req.Trigger + c.JSON(http.StatusOK, nil) +} + +// RunGRPCCheck probes and records one cron gRPC check. A nil error means the +// result was recorded, whether the monitor passed or not. +func (h Handler) RunGRPCCheck(ctx context.Context, req request.GRPCCheckerRequest) (checker.GRPCResponse, error) { + dataSourceName := "grpc_response__v0" + + ctx = withMonitorLogger(ctx, req.MonitorID, req.WorkspaceID) + + var response checker.GRPCResponse + + workspaceId, err := strconv.ParseInt(req.WorkspaceID, 10, 64) + if err != nil { + return response, InvalidRequestError{Reason: "invalid request"} } - e, f := c.Get("event") - if f { - t := e.(map[string]any) - t["checker"] = map[string]string{ - "uri": req.URI, - "workspace_id": req.WorkspaceID, - "monitor_id": req.MonitorID, - "trigger": trigger, - "type": "grpc", - } - c.Set("event", t) + monitorId, err := strconv.ParseInt(req.MonitorID, 10, 64) + if err != nil { + return response, InvalidRequestError{Reason: "invalid request"} } - var response checker.GRPCResponse + trigger := cronTrigger(req.Trigger) + + var recordErr error var retry int if req.Retry != 0 { @@ -228,6 +219,7 @@ func (h Handler) GRPCHandler(c *gin.Context) { if err := h.TbClient.SendEvent(ctx, data, dataSourceName); err != nil { log.Ctx(ctx).Error().Err(err).Msg("failed to send event to tinybird") + recordErr = err } return struct{}{}, nil @@ -237,7 +229,7 @@ func (h Handler) GRPCHandler(c *gin.Context) { id, e := uuid.NewV7() if e != nil { log.Ctx(ctx).Error().Err(e).Msg("failed to send event to tinybird") - return + return response, e } // A marshal failure leaves the column empty rather than dropping the row: // the rest of the failure record is still worth ingesting. @@ -264,6 +256,7 @@ func (h Handler) GRPCHandler(c *gin.Context) { } if err := h.TbClient.SendEvent(ctx, data, dataSourceName); err != nil { log.Ctx(ctx).Error().Err(err).Msg("failed to send event to tinybird") + recordErr = err } checker.UpdateStatus(ctx, checker.UpdateData{ MonitorId: req.MonitorID, @@ -288,14 +281,7 @@ func (h Handler) GRPCHandler(c *gin.Context) { otelOS.RecordGRPCMetrics(ctx, req, response, h.Region) } - returnData := c.Query("data") - if returnData == "true" { - c.JSON(http.StatusOK, response) - - return - } - - c.JSON(http.StatusOK, nil) + return response, recordErr } func (h Handler) GRPCHandlerRegion(c *gin.Context) { diff --git a/apps/checker/handlers/handler.go b/apps/checker/handlers/handler.go index e15a75f23..954633f9a 100644 --- a/apps/checker/handlers/handler.go +++ b/apps/checker/handlers/handler.go @@ -2,8 +2,12 @@ package handlers import ( "context" + "errors" + "fmt" + "net/http" "time" + "github.com/gin-gonic/gin" "github.com/rs/zerolog/log" "github.com/openstatushq/openstatus/apps/checker/pkg/tinybird" @@ -34,3 +38,72 @@ func withMonitorLogger(ctx context.Context, monitorID, workspaceID string) conte Logger(). WithContext(ctx) } + +// InvalidRequestError marks a check request that can never succeed, so a +// transport must drop it instead of retrying. +type InvalidRequestError struct { + Reason string +} + +func (e InvalidRequestError) Error() string { + return e.Reason +} + +// acceptCron answers the request itself when it is unauthorized or belongs to +// another fly region, and reports whether the handler should go on. +func (h Handler) acceptCron(c *gin.Context) bool { + if c.GetHeader("Authorization") != fmt.Sprintf("Basic %s", h.Secret) { + c.JSON(http.StatusUnauthorized, gin.H{"error": "unauthorized"}) + + return false + } + + if h.CloudProvider == "fly" { + // if the request has been routed to a wrong region, we forward it to the correct one. + region := c.GetHeader("fly-prefer-region") + if region != "" && region != h.Region { + c.Header("fly-replay", fmt.Sprintf("region=%s", region)) + c.String(http.StatusAccepted, "Forwarding request to %s", region) + + return false + } + } + + return true +} + +func cronTrigger(trigger string) string { + if trigger == "" { + return "cron" + } + + return trigger +} + +func setCheckerEvent(c *gin.Context, uri, workspaceID, monitorID, trigger, jobType string) { + e, f := c.Get("event") + if !f { + return + } + t := e.(map[string]any) + t["checker"] = map[string]any{ + "uri": uri, + "workspace_id": workspaceID, + "monitor_id": monitorID, + "trigger": trigger, + "type": jobType, + } + c.Set("event", t) +} + +// respondInvalid answers 400 when err is an InvalidRequestError and reports +// whether it did. +func respondInvalid(c *gin.Context, err error) bool { + var invalid InvalidRequestError + if !errors.As(err, &invalid) { + return false + } + c.JSON(http.StatusBadRequest, gin.H{"error": invalid.Reason}) + + return true +} diff --git a/apps/checker/handlers/icmp.go b/apps/checker/handlers/icmp.go index da751988f..df4abf7a7 100644 --- a/apps/checker/handlers/icmp.go +++ b/apps/checker/handlers/icmp.go @@ -1,6 +1,7 @@ package handlers import ( + "context" "encoding/json" "fmt" "net/http" @@ -43,25 +44,11 @@ type ICMPData struct { func (h Handler) ICMPHandler(c *gin.Context) { ctx := c.Request.Context() - dataSourceName := "icmp_response__v0" - - if c.GetHeader("Authorization") != fmt.Sprintf("Basic %s", h.Secret) { - c.JSON(http.StatusUnauthorized, gin.H{"error": "unauthorized"}) + if !h.acceptCron(c) { return } - if h.CloudProvider == "fly" { - // if the request has been routed to a wrong region, we forward it to the correct one. - region := c.GetHeader("fly-prefer-region") - if region != "" && region != h.Region { - c.Header("fly-replay", fmt.Sprintf("region=%s", region)) - c.String(http.StatusAccepted, "Forwarding request to %s", region) - - return - } - } - var req request.ICMPCheckerRequest if err := c.ShouldBindJSON(&req); err != nil { log.Ctx(ctx).Error().Err(err).Msg("failed to decode checker request") @@ -70,41 +57,45 @@ func (h Handler) ICMPHandler(c *gin.Context) { return } - ctx = withMonitorLogger(ctx, req.MonitorID, req.WorkspaceID) - - workspaceId, err := strconv.ParseInt(req.WorkspaceID, 10, 64) - if err != nil { - c.JSON(http.StatusBadRequest, gin.H{"error": "invalid request"}) + setCheckerEvent(c, req.URI, req.WorkspaceID, req.MonitorID, cronTrigger(req.Trigger), "icmp") + response, err := h.RunICMPCheck(ctx, req) + if respondInvalid(c, err) { return } - monitorId, err := strconv.ParseInt(req.MonitorID, 10, 64) - if err != nil { - c.JSON(http.StatusBadRequest, gin.H{"error": "invalid request"}) + returnData := c.Query("data") + if returnData == "true" { + c.JSON(http.StatusOK, response) return } - var trigger = "cron" - if req.Trigger != "" { - trigger = req.Trigger + c.JSON(http.StatusOK, nil) +} + +// RunICMPCheck probes and records one cron ICMP check. A nil error means the +// result was recorded, whether the monitor passed or not. +func (h Handler) RunICMPCheck(ctx context.Context, req request.ICMPCheckerRequest) (checker.ICMPResponse, error) { + dataSourceName := "icmp_response__v0" + + ctx = withMonitorLogger(ctx, req.MonitorID, req.WorkspaceID) + + var response checker.ICMPResponse + + workspaceId, err := strconv.ParseInt(req.WorkspaceID, 10, 64) + if err != nil { + return response, InvalidRequestError{Reason: "invalid request"} } - e, f := c.Get("event") - if f { - t := e.(map[string]any) - t["checker"] = map[string]string{ - "uri": req.URI, - "workspace_id": req.WorkspaceID, - "monitor_id": req.MonitorID, - "trigger": trigger, - "type": "icmp", - } - c.Set("event", t) + monitorId, err := strconv.ParseInt(req.MonitorID, 10, 64) + if err != nil { + return response, InvalidRequestError{Reason: "invalid request"} } - var response checker.ICMPResponse + trigger := cronTrigger(req.Trigger) + + var recordErr error var retry int if req.Retry != 0 { @@ -210,6 +201,7 @@ func (h Handler) ICMPHandler(c *gin.Context) { if err := h.TbClient.SendEvent(ctx, data, dataSourceName); err != nil { log.Ctx(ctx).Error().Err(err).Msg("failed to send event to tinybird") + recordErr = err } return struct{}{}, nil @@ -219,7 +211,7 @@ func (h Handler) ICMPHandler(c *gin.Context) { id, e := uuid.NewV7() if e != nil { log.Ctx(ctx).Error().Err(e).Msg("failed to send event to tinybird") - return + return response, e } data := ICMPData{ ID: id.String(), @@ -235,6 +227,7 @@ func (h Handler) ICMPHandler(c *gin.Context) { } if err := h.TbClient.SendEvent(ctx, data, dataSourceName); err != nil { log.Ctx(ctx).Error().Err(err).Msg("failed to send event to tinybird") + recordErr = err } checker.UpdateStatus(ctx, checker.UpdateData{ MonitorId: req.MonitorID, @@ -256,14 +249,7 @@ func (h Handler) ICMPHandler(c *gin.Context) { otelOS.RecordICMPMetrics(ctx, req, response, h.Region) } - returnData := c.Query("data") - if returnData == "true" { - c.JSON(http.StatusOK, response) - - return - } - - c.JSON(http.StatusOK, nil) + return response, recordErr } func (h Handler) ICMPHandlerRegion(c *gin.Context) { diff --git a/apps/checker/handlers/run_check_test.go b/apps/checker/handlers/run_check_test.go new file mode 100644 index 000000000..b5e498840 --- /dev/null +++ b/apps/checker/handlers/run_check_test.go @@ -0,0 +1,147 @@ +package handlers_test + +import ( + "context" + "errors" + "io" + "net" + "net/http" + "strings" + "testing" + + "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" +) + +func failingTinybird(t *testing.T) tinybird.Client { + t.Helper() + hclient := &http.Client{Transport: RoundTripFunc(func(req *http.Request) *http.Response { + return &http.Response{ + StatusCode: http.StatusInternalServerError, + Body: io.NopCloser(strings.NewReader(`{}`)), + } + })} + return tinybird.NewClient(hclient, "apiKey") +} + +func TestRunTCPCheck_InvalidWorkspaceIsPermanent(t *testing.T) { + h := handlers.Handler{TbClient: testTinybird(t), Region: "local"} + + _, err := h.RunTCPCheck(context.Background(), request.TCPCheckerRequest{WorkspaceID: "abc", MonitorID: "1"}) + + var invalid handlers.InvalidRequestError + assert.True(t, errors.As(err, &invalid)) +} + +func TestRunTCPCheck_ReportsRecordFailure(t *testing.T) { + ln, err := net.Listen("tcp", "127.0.0.1:0") + require.NoError(t, err) + t.Cleanup(func() { ln.Close() }) + + h := handlers.Handler{TbClient: failingTinybird(t), Region: "local"} + + _, err = h.RunTCPCheck(context.Background(), request.TCPCheckerRequest{ + URI: ln.Addr().String(), + WorkspaceID: "1", + MonitorID: "1", + Status: "active", + Timeout: 5, + Retry: 1, + }) + + require.Error(t, err) + var invalid handlers.InvalidRequestError + assert.False(t, errors.As(err, &invalid)) +} + +func TestRunTCPCheck_RecordedFailureIsNotAnError(t *testing.T) { + ln, err := net.Listen("tcp", "127.0.0.1:0") + require.NoError(t, err) + addr := ln.Addr().String() + ln.Close() + + h := handlers.Handler{TbClient: testTinybird(t), Region: "local"} + + res, err := h.RunTCPCheck(context.Background(), request.TCPCheckerRequest{ + URI: addr, + WorkspaceID: "1", + MonitorID: "1", + Status: "error", + Timeout: 5, + Retry: 1, + }) + + assert.NoError(t, err) + assert.Equal(t, uint8(1), res.Error) +} + +func TestRunCheck_InvalidIDsArePermanent(t *testing.T) { + h := handlers.Handler{TbClient: testTinybird(t), Region: "local"} + ctx := context.Background() + + tests := []struct { + name string + run func(workspaceID, monitorID string) error + reason map[string]string + }{ + { + name: "dns", + run: func(w, m string) error { + _, err := h.RunDNSCheck(ctx, request.DNSCheckerRequest{WorkspaceID: w, MonitorID: m}) + return err + }, + reason: map[string]string{"workspace": "invalid workspace id", "monitor": "invalid monitor id"}, + }, + { + name: "icmp", + run: func(w, m string) error { + _, err := h.RunICMPCheck(ctx, request.ICMPCheckerRequest{WorkspaceID: w, MonitorID: m}) + return err + }, + reason: map[string]string{"workspace": "invalid request", "monitor": "invalid request"}, + }, + { + name: "grpc", + run: func(w, m string) error { + _, err := h.RunGRPCCheck(ctx, request.GRPCCheckerRequest{WorkspaceID: w, MonitorID: m}) + return err + }, + reason: map[string]string{"workspace": "invalid request", "monitor": "invalid request"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name+"/workspace", func(t *testing.T) { + var invalid handlers.InvalidRequestError + require.True(t, errors.As(tt.run("abc", "1"), &invalid)) + assert.Equal(t, tt.reason["workspace"], invalid.Reason) + }) + t.Run(tt.name+"/monitor", func(t *testing.T) { + var invalid handlers.InvalidRequestError + require.True(t, errors.As(tt.run("1", "abc"), &invalid)) + assert.Equal(t, tt.reason["monitor"], invalid.Reason) + }) + } +} + +func TestRunDNSCheck_ResolverErrorIsRecordedAsFailure(t *testing.T) { + h := handlers.Handler{TbClient: testTinybird(t), Region: "local"} + + // An empty host fails the lookup without touching the network. + res, err := h.RunDNSCheck(context.Background(), request.DNSCheckerRequest{ + URI: "", + WorkspaceID: "1", + MonitorID: "1", + Status: "error", + Timeout: 5, + Retry: 1, + }) + + assert.NoError(t, err) + assert.Equal(t, uint8(1), res.Error) + assert.Equal(t, "error", res.RequestStatus) + assert.NotEmpty(t, res.ErrorMessage) +} diff --git a/apps/checker/handlers/tcp.go b/apps/checker/handlers/tcp.go index ed4ad54d2..8fe668086 100644 --- a/apps/checker/handlers/tcp.go +++ b/apps/checker/handlers/tcp.go @@ -1,6 +1,7 @@ package handlers import ( + "context" "encoding/json" "fmt" "net/http" @@ -38,25 +39,11 @@ type TCPData struct { func (h Handler) TCPHandler(c *gin.Context) { ctx := c.Request.Context() - dataSourceName := "tcp_response__v0" - - if c.GetHeader("Authorization") != fmt.Sprintf("Basic %s", h.Secret) { - c.JSON(http.StatusUnauthorized, gin.H{"error": "unauthorized"}) + if !h.acceptCron(c) { return } - if h.CloudProvider == "fly" { - // if the request has been routed to a wrong region, we forward it to the correct one. - region := c.GetHeader("fly-prefer-region") - if region != "" && region != h.Region { - c.Header("fly-replay", fmt.Sprintf("region=%s", region)) - c.String(http.StatusAccepted, "Forwarding request to %s", region) - - return - } - } - var req request.TCPCheckerRequest if err := c.ShouldBindJSON(&req); err != nil { log.Ctx(ctx).Error().Err(err).Msg("failed to decode checker request") @@ -65,43 +52,43 @@ func (h Handler) TCPHandler(c *gin.Context) { return } - ctx = withMonitorLogger(ctx, req.MonitorID, req.WorkspaceID) + setCheckerEvent(c, req.URI, req.WorkspaceID, req.MonitorID, cronTrigger(req.Trigger), "tcp") - workspaceId, err := strconv.ParseInt(req.WorkspaceID, 10, 64) + response, err := h.RunTCPCheck(ctx, req) + if respondInvalid(c, err) { + return + } - if err != nil { - c.JSON(http.StatusBadRequest, gin.H{"error": "invalid request"}) + returnData := c.Query("data") + if returnData == "true" { + c.JSON(http.StatusOK, response) return } - monitorId, err := strconv.ParseInt(req.MonitorID, 10, 64) + c.JSON(http.StatusOK, nil) +} - if err != nil { - c.JSON(http.StatusBadRequest, gin.H{"error": "invalid request"}) +// RunTCPCheck probes and records one cron TCP check. A nil error means the +// result was recorded, whether the monitor passed or not. +func (h Handler) RunTCPCheck(ctx context.Context, req request.TCPCheckerRequest) (checker.TCPResponse, error) { + dataSourceName := "tcp_response__v0" - return - } + ctx = withMonitorLogger(ctx, req.MonitorID, req.WorkspaceID) - var trigger = "cron" - if req.Trigger != "" { - trigger = req.Trigger + var response checker.TCPResponse + + workspaceId, err := strconv.ParseInt(req.WorkspaceID, 10, 64) + if err != nil { + return response, InvalidRequestError{Reason: "invalid request"} } - e, f := c.Get("event") - if f { - t := e.(map[string]any) - t["checker"] = map[string]string{ - "uri": req.URI, - "workspace_id": req.WorkspaceID, - "monitor_id":req.MonitorID, - "trigger": trigger, - "type": "tcp", - } - c.Set("event", t) + monitorId, err := strconv.ParseInt(req.MonitorID, 10, 64) + if err != nil { + return response, InvalidRequestError{Reason: "invalid request"} } - var response checker.TCPResponse + trigger := cronTrigger(req.Trigger) var retry int if req.Retry != 0 { @@ -110,6 +97,8 @@ func (h Handler) TCPHandler(c *gin.Context) { retry = 3 } + var recordErr error + op := func() (struct{}, error) { res, err := checker.PingTCP(ctx, req.Timeout, req.URI) @@ -204,6 +193,7 @@ func (h Handler) TCPHandler(c *gin.Context) { if err := h.TbClient.SendEvent(ctx, data, dataSourceName); err != nil { log.Ctx(ctx).Error().Err(err).Msg("failed to send event to tinybird") + recordErr = err } return struct{}{}, nil @@ -214,7 +204,7 @@ func (h Handler) TCPHandler(c *gin.Context) { id, e := uuid.NewV7() if e != nil { log.Ctx(ctx).Error().Err(e).Msg("failed to send event to tinybird") - return + return response, e } data := TCPData{ ID: id.String(), @@ -230,6 +220,7 @@ func (h Handler) TCPHandler(c *gin.Context) { } if err := h.TbClient.SendEvent(ctx, data, dataSourceName); err != nil { log.Ctx(ctx).Error().Err(err).Msg("failed to send event to tinybird") + recordErr = err } checker.UpdateStatus(ctx, checker.UpdateData{ MonitorId: req.MonitorID, @@ -246,14 +237,7 @@ func (h Handler) TCPHandler(c *gin.Context) { otelOS.RecordTCPMetrics(ctx, req, response, h.Region) } - returnData := c.Query("data") - if returnData == "true" { - c.JSON(http.StatusOK, response) - - return - } - - c.JSON(http.StatusOK, nil) + return response, recordErr } func (h Handler) TCPHandlerRegion(c *gin.Context) { -- 2.54.0