From a3e6d4a6955bc8b0ff14e72e32e4da3839bb1e7b Mon Sep 17 00:00:00 2001 From: lvlcn-t <75443136+lvlcn-t@users.noreply.github.com> Date: Sun, 23 Mar 2025 21:13:06 +0100 Subject: [PATCH 1/2] refactor(checks): move global target merging responsibility to checks This commit moves the responsibility of merging global targets with the configured targets to each check. This way the checks can decide how to merge the global targets with the configured targets. Signed-off-by: lvlcn-t <75443136+lvlcn-t@users.noreply.github.com> --- pkg/checks/base.go | 58 +++++++++++++++++++++++++++++++-- pkg/checks/dns/config.go | 22 ++++++++++++- pkg/checks/health/config.go | 23 ++++++++++++- pkg/checks/latency/config.go | 21 ++++++++++++ pkg/checks/runtime/config.go | 12 +++++++ pkg/checks/traceroute/config.go | 26 +++++++++++++++ pkg/sparrow/run.go | 33 +++++-------------- pkg/sparrow/run_test.go | 44 ++++++++++++++++++------- 8 files changed, 198 insertions(+), 41 deletions(-) diff --git a/pkg/checks/base.go b/pkg/checks/base.go index 30a9c542..649ac2c5 100644 --- a/pkg/checks/base.go +++ b/pkg/checks/base.go @@ -6,6 +6,8 @@ package checks import ( "context" + "net/url" + "strconv" "sync" "time" @@ -56,12 +58,15 @@ type CheckBase struct { DoneChan chan struct{} } -// Runtime is the interface that all check configurations must implement +// Runtime is the interface that all check configurations must implement. type Runtime interface { - // For returns the name of the check being configured + // For returns the name of the check being configured. For() string - // Validate checks if the configuration is valid + // Validate checks if the configuration is valid. Validate() error + // Enrich enriches the configuration with the [GlobalTarget]s. + // Each configuration is responsible for adding the targets it needs. + Enrich(ctx context.Context, targets []GlobalTarget) } // Result encapsulates the outcome of a check run. @@ -84,3 +89,50 @@ type GlobalTarget struct { Url string `json:"url"` LastSeen time.Time `json:"lastSeen"` } + +// URL returns the [url.URL] representation of the target. +func (g GlobalTarget) URL() (*url.URL, error) { + return url.Parse(g.Url) +} + +// Hostname returns the host of the target URL, stripping the port if present. +func (g GlobalTarget) Hostname() (string, error) { + u, err := g.URL() + if err != nil { + return "", err + } + return u.Hostname(), nil +} + +// Default ports +const ( + httpPort = 80 + httpsPort = 443 +) + +// Port returns the port of the target URL. +func (g GlobalTarget) Port() (int, error) { + u, err := g.URL() + if err != nil { + return 0, err + } + + // If the port is not specified, the default port for the scheme is returned. + if u.Port() == "" { + switch u.Scheme { + case "http": + return httpPort, nil + case "https": + return httpsPort, nil + default: + return 0, nil + } + } + + return strconv.Atoi(u.Port()) +} + +// String returns the URL of the target. +func (g GlobalTarget) String() string { + return g.Url +} diff --git a/pkg/checks/dns/config.go b/pkg/checks/dns/config.go index 0faeaa53..d540be0c 100644 --- a/pkg/checks/dns/config.go +++ b/pkg/checks/dns/config.go @@ -5,11 +5,14 @@ package dns import ( + "context" "fmt" + "slices" "strings" "time" "github.com/telekom/sparrow/internal/helper" + "github.com/telekom/sparrow/internal/logger" "github.com/telekom/sparrow/pkg/checks" ) @@ -21,9 +24,9 @@ const ( // Config defines the configuration parameters for a DNS check type Config struct { Targets []string `json:"targets" yaml:"targets"` + Retry helper.RetryConfig `json:"retry" yaml:"retry"` Interval time.Duration `json:"interval" yaml:"interval"` Timeout time.Duration `json:"timeout" yaml:"timeout"` - Retry helper.RetryConfig `json:"retry" yaml:"retry"` } // For returns the name of the check @@ -49,3 +52,20 @@ func (c *Config) Validate() error { return nil } + +// Enrich adds the global targets to the configuration +func (c *Config) Enrich(ctx context.Context, targets []checks.GlobalTarget) { + log := logger.FromContext(ctx) + for _, t := range targets { + hostname, err := t.Hostname() + if err != nil { + log.ErrorContext(ctx, "Failed to get hostname from target", "target", t, "error", err) + continue + } + + if !slices.Contains(c.Targets, hostname) { + log.DebugContext(ctx, "Adding target to DNS check", "target", hostname) + c.Targets = append(c.Targets, hostname) + } + } +} diff --git a/pkg/checks/health/config.go b/pkg/checks/health/config.go index 524ad296..9eb12d05 100644 --- a/pkg/checks/health/config.go +++ b/pkg/checks/health/config.go @@ -5,11 +5,14 @@ package health import ( + "context" "fmt" "net/url" + "slices" "time" "github.com/telekom/sparrow/internal/helper" + "github.com/telekom/sparrow/internal/logger" "github.com/telekom/sparrow/pkg/checks" ) @@ -21,9 +24,9 @@ const ( // Config defines the configuration parameters for a health check type Config struct { Targets []string `json:"targets,omitempty" yaml:"targets,omitempty"` + Retry helper.RetryConfig `json:"retry" yaml:"retry"` Interval time.Duration `json:"interval" yaml:"interval"` Timeout time.Duration `json:"timeout" yaml:"timeout"` - Retry helper.RetryConfig `json:"retry" yaml:"retry"` } // For returns the name of the check @@ -54,3 +57,21 @@ func (c *Config) Validate() error { return nil } + +// Enrich adds the global targets to the configuration +func (c *Config) Enrich(ctx context.Context, targets []checks.GlobalTarget) { + log := logger.FromContext(ctx) + for _, t := range targets { + u, err := t.URL() + if err != nil { + log.ErrorContext(ctx, "Failed to get URL from target", "target", t, "error", err) + continue + } + + target := u.String() + if !slices.Contains(c.Targets, target) { + log.DebugContext(ctx, "Adding target to health check", "target", target) + c.Targets = append(c.Targets, target) + } + } +} diff --git a/pkg/checks/latency/config.go b/pkg/checks/latency/config.go index 2cf85efe..6623454a 100644 --- a/pkg/checks/latency/config.go +++ b/pkg/checks/latency/config.go @@ -5,11 +5,14 @@ package latency import ( + "context" "fmt" "net/url" + "slices" "time" "github.com/telekom/sparrow/internal/helper" + "github.com/telekom/sparrow/internal/logger" "github.com/telekom/sparrow/pkg/checks" ) @@ -54,3 +57,21 @@ func (c *Config) Validate() error { return nil } + +// Enrich adds the global targets to the configuration +func (c *Config) Enrich(ctx context.Context, targets []checks.GlobalTarget) { + log := logger.FromContext(ctx) + for _, t := range targets { + u, err := t.URL() + if err != nil { + log.ErrorContext(ctx, "Failed to get URL from target", "target", t, "error", err) + continue + } + + target := u.String() + if !slices.Contains(c.Targets, target) { + log.DebugContext(ctx, "Adding target to health check", "target", target) + c.Targets = append(c.Targets, target) + } + } +} diff --git a/pkg/checks/runtime/config.go b/pkg/checks/runtime/config.go index 8fe43d02..a21ab974 100644 --- a/pkg/checks/runtime/config.go +++ b/pkg/checks/runtime/config.go @@ -5,8 +5,10 @@ package runtime import ( + "context" "errors" + "github.com/telekom/sparrow/internal/logger" "github.com/telekom/sparrow/pkg/checks" "github.com/telekom/sparrow/pkg/checks/dns" "github.com/telekom/sparrow/pkg/checks/health" @@ -57,6 +59,16 @@ func (c Config) Iter() []checks.Runtime { return configs } +// Enrich enriches the configuration with the provided [GlobalTarget]s. +func (c Config) Enrich(ctx context.Context, targets []checks.GlobalTarget) Config { + l := logger.FromContext(ctx) + for _, cfg := range c.Iter() { + l.DebugContext(ctx, "Enriching check configuration", "check", cfg.For()) + cfg.Enrich(ctx, targets) + } + return c +} + // size returns the number of checks configured func (c Config) size() int { size := 0 diff --git a/pkg/checks/traceroute/config.go b/pkg/checks/traceroute/config.go index 05ce1580..20143441 100644 --- a/pkg/checks/traceroute/config.go +++ b/pkg/checks/traceroute/config.go @@ -5,12 +5,15 @@ package traceroute import ( + "context" "fmt" "net" "net/url" + "slices" "time" "github.com/telekom/sparrow/internal/helper" + "github.com/telekom/sparrow/internal/logger" "github.com/telekom/sparrow/pkg/checks" ) @@ -53,3 +56,26 @@ func (c *Config) Validate() error { } return nil } + +// Enrich adds the global targets to the configuration +func (c *Config) Enrich(ctx context.Context, targets []checks.GlobalTarget) { + log := logger.FromContext(ctx) + for _, t := range targets { + u, err := t.URL() + if err != nil { + log.ErrorContext(ctx, "Failed to get URL from target", "target", t, "error", err) + continue + } + + // Error handling is not necessary here, as the URL has been validated before. + port, _ := t.Port() + if !slices.ContainsFunc(c.Targets, func(t Target) bool { + return t.Addr == u.Hostname() && t.Port == port + }) { + c.Targets = append(c.Targets, Target{ + Addr: u.Hostname(), + Port: port, + }) + } + } +} diff --git a/pkg/sparrow/run.go b/pkg/sparrow/run.go index 95c2354c..22296ece 100644 --- a/pkg/sparrow/run.go +++ b/pkg/sparrow/run.go @@ -7,14 +7,12 @@ package sparrow import ( "context" "fmt" - "net/url" - "slices" - "strings" "sync" "time" "github.com/telekom/sparrow/internal/logger" "github.com/telekom/sparrow/pkg/api" + "github.com/telekom/sparrow/pkg/checks" "github.com/telekom/sparrow/pkg/checks/runtime" "github.com/telekom/sparrow/pkg/config" "github.com/telekom/sparrow/pkg/db" @@ -126,36 +124,23 @@ func (s *Sparrow) Run(ctx context.Context) error { // enrichTargets updates the targets of the sparrow's checks with the // global targets. Per default, the two target lists are merged. func (s *Sparrow) enrichTargets(ctx context.Context, cfg runtime.Config) runtime.Config { - l := logger.FromContext(ctx) if cfg.Empty() || s.tarMan == nil { return cfg } - - for _, gt := range s.tarMan.GetTargets() { - u, err := url.Parse(gt.Url) + var gts []checks.GlobalTarget + for _, t := range s.tarMan.GetTargets() { + hostname, err := t.Hostname() if err != nil { - l.Error("Failed to parse global target URL", "error", err, "url", gt.Url) + logger.FromContext(ctx).ErrorContext(ctx, "Failed to get hostname from target", "target", t, "error", err) continue } - - // split off hostWithoutPort because it could contain a port - hostWithoutPort := strings.Split(u.Host, ":")[0] - if hostWithoutPort == s.config.SparrowName { + // We don't need to enrich the configs with the own hostname + if s.config.SparrowName == hostname { continue } - - if cfg.HasHealthCheck() && !slices.Contains(cfg.Health.Targets, u.String()) { - cfg.Health.Targets = append(cfg.Health.Targets, u.String()) - } - if cfg.HasLatencyCheck() && !slices.Contains(cfg.Latency.Targets, u.String()) { - cfg.Latency.Targets = append(cfg.Latency.Targets, u.String()) - } - if cfg.HasDNSCheck() && !slices.Contains(cfg.Dns.Targets, hostWithoutPort) { - cfg.Dns.Targets = append(cfg.Dns.Targets, hostWithoutPort) - } + gts = append(gts, t) } - - return cfg + return cfg.Enrich(ctx, gts) } // shutdown shuts down the sparrow and all managed components gracefully. diff --git a/pkg/sparrow/run_test.go b/pkg/sparrow/run_test.go index b111ba72..2605ec22 100644 --- a/pkg/sparrow/run_test.go +++ b/pkg/sparrow/run_test.go @@ -16,6 +16,7 @@ import ( "github.com/telekom/sparrow/pkg/checks/health" "github.com/telekom/sparrow/pkg/checks/latency" "github.com/telekom/sparrow/pkg/checks/runtime" + "github.com/telekom/sparrow/pkg/checks/traceroute" "github.com/telekom/sparrow/pkg/config" "github.com/telekom/sparrow/pkg/sparrow/targets" "github.com/telekom/sparrow/pkg/sparrow/targets/interactor" @@ -43,7 +44,7 @@ func TestSparrow_Run_FullComponentStart(t *testing.T) { }, Config: interactor.Config{ Gitlab: gitlab.Config{ - BaseURL: "https://gitlab.com", + BaseURL: "https://telekom.com", Token: "my-cool-token", ProjectID: 42, }, @@ -163,19 +164,19 @@ func TestSparrow_enrichTargets(t *testing.T) { name: "config with targets (health + latency)", config: runtime.Config{ Health: &health.Config{ - Targets: []string{"https://gitlab.com"}, + Targets: []string{"https://telekom.com"}, }, Latency: &latency.Config{ - Targets: []string{"https://gitlab.com"}, + Targets: []string{"https://telekom.com"}, }, }, globalTargets: gt, expected: runtime.Config{ Health: &health.Config{ - Targets: []string{"https://gitlab.com", testTarget}, + Targets: []string{"https://telekom.com", testTarget}, }, Latency: &latency.Config{ - Targets: []string{"https://gitlab.com", testTarget}, + Targets: []string{"https://telekom.com", testTarget}, }, }, }, @@ -183,13 +184,32 @@ func TestSparrow_enrichTargets(t *testing.T) { name: "config with targets (dns)", config: runtime.Config{ Dns: &dns.Config{ - Targets: []string{"gitlab.com"}, + Targets: []string{"telekom.com"}, }, }, globalTargets: gt, expected: runtime.Config{ Dns: &dns.Config{ - Targets: []string{"gitlab.com", "localhost.de"}, + Targets: []string{"telekom.com", "localhost.de"}, + }, + }, + }, + { + name: "config with targets (traceroute)", + config: runtime.Config{ + Traceroute: &traceroute.Config{ + Targets: []traceroute.Target{ + {Addr: "telekom.com", Port: 443}, + }, + }, + }, + globalTargets: gt, + expected: runtime.Config{ + Traceroute: &traceroute.Config{ + Targets: []traceroute.Target{ + {Addr: "telekom.com", Port: 443}, + {Addr: "localhost.de", Port: 443}, + }, }, }, }, @@ -215,7 +235,7 @@ func TestSparrow_enrichTargets(t *testing.T) { }, }, globalTargets: append(gt, checks.GlobalTarget{ - Url: "https://sparrow.com", + Url: "https://sparrow.telekom.com", LastSeen: now, }), expected: runtime.Config{ @@ -233,16 +253,16 @@ func TestSparrow_enrichTargets(t *testing.T) { }, globalTargets: []checks.GlobalTarget{ { - Url: "http://az1.sparrow.com", + Url: "http://az1.sparrow.telekom.com", LastSeen: now, }, { - Url: "https://az2.sparrow.com", + Url: "https://az2.sparrow.telekom.com", }, }, expected: runtime.Config{ Dns: &dns.Config{ - Targets: []string{"az1.sparrow.com", "az2.sparrow.com"}, + Targets: []string{"az1.sparrow.telekom.com", "az2.sparrow.telekom.com"}, }, }, }, @@ -255,7 +275,7 @@ func TestSparrow_enrichTargets(t *testing.T) { Targets: tt.globalTargets, }, config: &config.Config{ - SparrowName: "sparrow.com", + SparrowName: "sparrow.telekom.com", }, } got := s.enrichTargets(context.Background(), tt.config) From 0ccbc93cf18a99fe73e51878df3cb7090facca82 Mon Sep 17 00:00:00 2001 From: lvlcn-t <75443136+lvlcn-t@users.noreply.github.com> Date: Sun, 23 Mar 2025 21:26:01 +0100 Subject: [PATCH 2/2] chore: update target representation in error logs This commit uses the target.String() representation in error logs to make the error messages more readable. Signed-off-by: lvlcn-t <75443136+lvlcn-t@users.noreply.github.com> --- pkg/checks/dns/config.go | 2 +- pkg/checks/health/config.go | 2 +- pkg/checks/latency/config.go | 6 +++--- pkg/checks/traceroute/config.go | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/pkg/checks/dns/config.go b/pkg/checks/dns/config.go index d540be0c..ca872a18 100644 --- a/pkg/checks/dns/config.go +++ b/pkg/checks/dns/config.go @@ -59,7 +59,7 @@ func (c *Config) Enrich(ctx context.Context, targets []checks.GlobalTarget) { for _, t := range targets { hostname, err := t.Hostname() if err != nil { - log.ErrorContext(ctx, "Failed to get hostname from target", "target", t, "error", err) + log.ErrorContext(ctx, "Failed to get hostname from target", "target", t.String(), "error", err) continue } diff --git a/pkg/checks/health/config.go b/pkg/checks/health/config.go index 9eb12d05..ecbde0c6 100644 --- a/pkg/checks/health/config.go +++ b/pkg/checks/health/config.go @@ -64,7 +64,7 @@ func (c *Config) Enrich(ctx context.Context, targets []checks.GlobalTarget) { for _, t := range targets { u, err := t.URL() if err != nil { - log.ErrorContext(ctx, "Failed to get URL from target", "target", t, "error", err) + log.ErrorContext(ctx, "Failed to get URL from target", "target", t.String(), "error", err) continue } diff --git a/pkg/checks/latency/config.go b/pkg/checks/latency/config.go index 6623454a..64c01e28 100644 --- a/pkg/checks/latency/config.go +++ b/pkg/checks/latency/config.go @@ -24,9 +24,9 @@ const ( // Config defines the configuration parameters for a latency check type Config struct { Targets []string `json:"targets,omitempty" yaml:"targets,omitempty"` + Retry helper.RetryConfig `json:"retry" yaml:"retry"` Interval time.Duration `json:"interval" yaml:"interval"` Timeout time.Duration `json:"timeout" yaml:"timeout"` - Retry helper.RetryConfig `json:"retry" yaml:"retry"` } // For returns the name of the check @@ -64,13 +64,13 @@ func (c *Config) Enrich(ctx context.Context, targets []checks.GlobalTarget) { for _, t := range targets { u, err := t.URL() if err != nil { - log.ErrorContext(ctx, "Failed to get URL from target", "target", t, "error", err) + log.ErrorContext(ctx, "Failed to get URL from target", "target", t.String(), "error", err) continue } target := u.String() if !slices.Contains(c.Targets, target) { - log.DebugContext(ctx, "Adding target to health check", "target", target) + log.DebugContext(ctx, "Adding target to latency check", "target", target) c.Targets = append(c.Targets, target) } } diff --git a/pkg/checks/traceroute/config.go b/pkg/checks/traceroute/config.go index 20143441..5ab9d954 100644 --- a/pkg/checks/traceroute/config.go +++ b/pkg/checks/traceroute/config.go @@ -63,7 +63,7 @@ func (c *Config) Enrich(ctx context.Context, targets []checks.GlobalTarget) { for _, t := range targets { u, err := t.URL() if err != nil { - log.ErrorContext(ctx, "Failed to get URL from target", "target", t, "error", err) + log.ErrorContext(ctx, "Failed to get URL from target", "target", t.String(), "error", err) continue }