diff --git a/rest-api/api/cmd/api/main.go b/rest-api/api/cmd/api/main.go index 8e44f8f87b..10f34bfeb1 100644 --- a/rest-api/api/cmd/api/main.go +++ b/rest-api/api/cmd/api/main.go @@ -102,7 +102,7 @@ func main() { mconfig := cfg.GetMetricsConfig() if mconfig.Enabled { // Initialize Prometheus Echo instance - ep := capis.InitMetricsServer(e, cfg) + ep := capis.InitMetricsServer(e, mconfig.Namespace) // Start Prometheus server log.Info().Msg("starting Metrics server") diff --git a/rest-api/api/internal/config/config.go b/rest-api/api/internal/config/config.go index 33851c52a9..a0a81a2e14 100644 --- a/rest-api/api/internal/config/config.go +++ b/rest-api/api/internal/config/config.go @@ -105,6 +105,8 @@ const ( ConfigMetricsEnabled = "metrics.enabled" // ConfigMetricsPort specifies the port for Prometheus metrics ConfigMetricsPort = "metrics.port" + // ConfigMetricsNamespace specifies the prefix for every exposed metric name + ConfigMetricsNamespace = "metrics.namespace" // ConfigTracingEnabled is a feature flag for tracing ConfigTracingEnabled = "tracing.enabled" @@ -256,6 +258,7 @@ func NewConfig() *Config { c.v.SetDefault(ConfigMetricsEnabled, true) c.v.SetDefault(ConfigMetricsPort, 9360) + c.v.SetDefault(ConfigMetricsNamespace, DefaultMetricsNamespace) c.v.SetDefault(ConfigTracingEnabled, false) @@ -539,7 +542,7 @@ func (c *Config) GetSiteConfig() *SiteConfig { // GetMetricsConfig returns the Metrics config func (c *Config) GetMetricsConfig() *MetricsConfig { - return NewMetricsConfig(c.GetMetricsEnabled(), c.GetMetricsPort()) + return NewMetricsConfig(c.GetMetricsEnabled(), c.GetMetricsPort(), c.GetMetricsNamespace()) } // GetRateLimiterConfig returns the rate limiter config @@ -968,6 +971,17 @@ func (c *Config) GetMetricsPort() int { return c.v.GetInt(ConfigMetricsPort) } +// GetMetricsNamespace gets the prefix applied to every exposed metric name. +// An explicitly empty value falls back to the default, since echoprometheus +// substitutes its own "echo" prefix for an empty one. +func (c *Config) GetMetricsNamespace() string { + namespace := c.v.GetString(ConfigMetricsNamespace) + if namespace == "" { + return DefaultMetricsNamespace + } + return namespace +} + // GetTracingEnabled gets the enabled field for tracing func (c *Config) GetTracingEnabled() bool { return c.v.GetBool(ConfigTracingEnabled) diff --git a/rest-api/api/internal/config/metrics.go b/rest-api/api/internal/config/metrics.go index 00b69b9622..8c4f10c7fd 100644 --- a/rest-api/api/internal/config/metrics.go +++ b/rest-api/api/internal/config/metrics.go @@ -7,10 +7,17 @@ import ( "fmt" ) -// TemporalConfig holds configuration for Temporal communication +// DefaultMetricsNamespace prefixes every metric this server exposes and matches +// its nico-rest-api Helm service name. Operators override it with +// metrics.namespace. Deliberately independent of api.name, which is the URL path +// segment callers route on and has no bearing on how these series are named. +const DefaultMetricsNamespace = "nico_rest_api" + +// MetricsConfig holds configuration of Metrics type MetricsConfig struct { - Enabled bool - Port int + Enabled bool + Port int + Namespace string } // GetListenAddr returns the local address for listen socket. @@ -19,9 +26,10 @@ func (mcfg *MetricsConfig) GetListenAddr() string { } // NewMetricsConfig initializes and returns a configuration object for managing Metrics -func NewMetricsConfig(enabled bool, port int) *MetricsConfig { +func NewMetricsConfig(enabled bool, port int, namespace string) *MetricsConfig { return &MetricsConfig{ - Enabled: enabled, - Port: port, + Enabled: enabled, + Port: port, + Namespace: namespace, } } diff --git a/rest-api/api/internal/config/metrics_test.go b/rest-api/api/internal/config/metrics_test.go index 11cc67d20a..bef77a50ae 100644 --- a/rest-api/api/internal/config/metrics_test.go +++ b/rest-api/api/internal/config/metrics_test.go @@ -5,17 +5,15 @@ package config import ( "testing" + + "github.com/stretchr/testify/assert" ) func TestMetricsConfig(t *testing.T) { type args struct { - enabled bool - port int - } - - mcfg := MetricsConfig{ - Enabled: true, - Port: 6930, + enabled bool + port int + namespace string } tests := []struct { @@ -26,23 +24,71 @@ func TestMetricsConfig(t *testing.T) { { name: "initialize Metrics config", args: args{ - enabled: true, - port: mcfg.Port, + enabled: true, + port: 6930, + namespace: DefaultMetricsNamespace, + }, + want: &MetricsConfig{ + Enabled: true, + Port: 6930, + Namespace: DefaultMetricsNamespace, + }, + }, + { + name: "initialize Metrics config with an overridden namespace", + args: args{ + enabled: true, + port: 6930, + namespace: "acme_api", + }, + want: &MetricsConfig{ + Enabled: true, + Port: 6930, + Namespace: "acme_api", }, - want: &mcfg, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got := NewMetricsConfig(tt.args.enabled, tt.args.port) + got := NewMetricsConfig(tt.args.enabled, tt.args.port, tt.args.namespace) + + assert.Equal(t, tt.want, got) + assert.Equal(t, tt.want.GetListenAddr(), got.GetListenAddr()) + }) + } +} - if p := got.Port; p != tt.want.Port { - t.Errorf("got.Port = %v, want %v", p, tt.want.Port) - } +func TestConfig_GetMetricsNamespace(t *testing.T) { + tests := []struct { + name string + configure func(c *Config) + want string + }{ + { + name: "unset falls back to the default", + configure: func(c *Config) {}, + want: DefaultMetricsNamespace, + }, + { + // An operator who blanks the key gets the default rather than + // echoprometheus substituting its own "echo" prefix. + name: "explicitly empty falls back to the default", + configure: func(c *Config) { c.v.Set(ConfigMetricsNamespace, "") }, + want: DefaultMetricsNamespace, + }, + { + name: "override is honored", + configure: func(c *Config) { c.v.Set(ConfigMetricsNamespace, "acme_api") }, + want: "acme_api", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + c := NewConfig() + tt.configure(c) + defer c.v.Set(ConfigMetricsNamespace, DefaultMetricsNamespace) - if got := got.GetListenAddr(); got != tt.want.GetListenAddr() { - t.Errorf("GetListenAddr() = %v, want %v", got, tt.want.GetListenAddr()) - } + assert.Equal(t, tt.want, c.GetMetricsNamespace()) }) } } diff --git a/rest-api/api/internal/server/server.go b/rest-api/api/internal/server/server.go index e3cf5c2d3b..a341546ff8 100644 --- a/rest-api/api/internal/server/server.go +++ b/rest-api/api/internal/server/server.go @@ -300,12 +300,14 @@ func InitAPIServer(cfg *config.Config, dbSession *cdb.Session, tc tsdkClient.Cli return e } -func InitMetricsServer(e *echo.Echo, cfg *config.Config) *echo.Echo { +func InitMetricsServer(e *echo.Echo, namespace string) *echo.Echo { ep := echo.New() ep.HideBanner = true conf := echoPrometheus.MiddlewareConfig{ - Subsystem: fmt.Sprintf("%s_api", cfg.GetAPIName()), + // The prefix has to go in Subsystem, since echoprometheus substitutes + // its own "echo" for an empty one. + Subsystem: namespace, Skipper: api.MetricsURLSkipper, } diff --git a/rest-api/api/internal/server/server_test.go b/rest-api/api/internal/server/server_test.go index da4d2d396f..0a1c1dc56d 100644 --- a/rest-api/api/internal/server/server_test.go +++ b/rest-api/api/internal/server/server_test.go @@ -25,6 +25,7 @@ import ( cdbm "github.com/NVIDIA/infra-controller/rest-api/db/pkg/db/model" cdbu "github.com/NVIDIA/infra-controller/rest-api/db/pkg/util" echo "github.com/labstack/echo/v4" + "github.com/prometheus/client_golang/prometheus" "github.com/stretchr/testify/assert" temporalClient "go.temporal.io/sdk/client" tmocks "go.temporal.io/sdk/mocks" @@ -124,8 +125,7 @@ func Test_InitTemporalClients(t *testing.T) { func Test_InitMetricsServer(t *testing.T) { type args struct { - e *echo.Echo - cfg *config.Config + e *echo.Echo } tests := []struct { name string @@ -134,14 +134,32 @@ func Test_InitMetricsServer(t *testing.T) { { name: "test initMetricsServer success", args: args{ - e: echo.New(), - cfg: common.GetTestConfig(), + e: echo.New(), }, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - InitMetricsServer(tt.args.e, tt.args.cfg) + // A tracked route, since MetricsURLSkipper only records /v2/ and /metrics. + tt.args.e.GET("/v2/probe", func(c echo.Context) error { + return c.NoContent(http.StatusOK) + }) + + InitMetricsServer(tt.args.e, config.DefaultMetricsNamespace) + + tt.args.e.ServeHTTP(httptest.NewRecorder(), httptest.NewRequest(http.MethodGet, "/v2/probe", nil)) + + // The prefix is the published contract. An empty Subsystem would + // silently produce echo_requests_total instead. + families, err := prometheus.DefaultGatherer.Gather() + assert.NoError(t, err) + + names := make([]string, 0, len(families)) + for _, family := range families { + names = append(names, family.GetName()) + } + assert.Contains(t, names, "nico_rest_api_requests_total") + assert.Contains(t, names, "nico_rest_api_request_duration_seconds") }) } } diff --git a/rest-api/cert-manager/pkg/certs/server.go b/rest-api/cert-manager/pkg/certs/server.go index 14d6baf9b8..5ec30b1f4f 100644 --- a/rest-api/cert-manager/pkg/certs/server.go +++ b/rest-api/cert-manager/pkg/certs/server.go @@ -54,7 +54,7 @@ func NewServerWithIssuer(ctx context.Context, o Options, certIssuer CertificateI appService.AddHealthRoute(ctx) appService.AddVersionRoute(ctx) appService.AddMetricsRoute(ctx) - appService.Use(core.NewHTTPMiddleware(ctx, core.WithRequestMetrics("cloud_cert_manager"))...) + appService.Use(core.NewHTTPMiddleware(ctx, core.WithRequestMetrics("nico_rest_cert_manager"))...) appService.Path("/v1/pki/ca").Handler(s.PKICACertificateHandler(ctx)).Methods("GET") appService.Path("/v1/pki/ca/pem").Handler(s.PKICACertificateHandler(ctx)).Methods("GET") appService.Path("/v1/pki/cloud-cert").Handler(s.PKICloudCertificateHandler(ctx)).Methods("POST") diff --git a/rest-api/cert-manager/pkg/core/httpservice.go b/rest-api/cert-manager/pkg/core/httpservice.go index b78f8cb8f1..95a17834ec 100644 --- a/rest-api/cert-manager/pkg/core/httpservice.go +++ b/rest-api/cert-manager/pkg/core/httpservice.go @@ -308,11 +308,21 @@ type httpMiddleware struct { // HTTPMiddlewareOption defines a middleware option type HTTPMiddlewareOption func(*httpMiddleware) +// MetricsNamespaceEnv overrides the prefix a caller passes to WithRequestMetrics. +// These services read no config file, so an environment variable is the only +// override available to an operator. +const MetricsNamespaceEnv = "METRICS_NAMESPACE" + // WithRequestMetrics enables service specific metrics regarding // request duration and count. By default these metrics are disabled. +// serverName is the prefix applied to the metric names, and METRICS_NAMESPACE +// takes precedence over it when set. func WithRequestMetrics(serverName string) HTTPMiddlewareOption { return func(h *httpMiddleware) { h.latencyMetricsName = serverName + if namespace := os.Getenv(MetricsNamespaceEnv); namespace != "" { + h.latencyMetricsName = namespace + } } } diff --git a/rest-api/cert-manager/pkg/core/httpservice_test.go b/rest-api/cert-manager/pkg/core/httpservice_test.go index 25c0da9ac3..2dc16a5f21 100644 --- a/rest-api/cert-manager/pkg/core/httpservice_test.go +++ b/rest-api/cert-manager/pkg/core/httpservice_test.go @@ -184,6 +184,45 @@ func TestHTTPServiceStart(t *testing.T) { } } +func TestWithRequestMetrics(t *testing.T) { + tests := []struct { + name string + serverName string + envValue string + want string + }{ + { + name: "uses the supplied prefix when the environment is unset", + serverName: "nico_rest_cert_manager", + want: "nico_rest_cert_manager", + }, + { + name: "METRICS_NAMESPACE takes precedence", + serverName: "nico_rest_cert_manager", + envValue: "acme_cert_manager", + want: "acme_cert_manager", + }, + { + // An empty value is indistinguishable from unset, and an empty prefix + // would expose bare names like "http_duration_seconds". + name: "an empty METRICS_NAMESPACE leaves the supplied prefix alone", + serverName: "nico_rest_cert_manager", + envValue: "", + want: "nico_rest_cert_manager", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Setenv(MetricsNamespaceEnv, tt.envValue) + + h := &httpMiddleware{} + WithRequestMetrics(tt.serverName)(h) + + assert.Equal(t, tt.want, h.latencyMetricsName) + }) + } +} + func Test_telemetryMiddleware(t *testing.T) { otel.SetTracerProvider(sdktrace.NewTracerProvider()) diff --git a/rest-api/go.mod b/rest-api/go.mod index 4166bcd2fa..d2795491c7 100644 --- a/rest-api/go.mod +++ b/rest-api/go.mod @@ -13,10 +13,8 @@ require ( github.com/DATA-DOG/go-sqlmock v1.5.2 github.com/NVIDIA/infra-controller/rest-api/sdk/standard v0.0.0-00010101000000-000000000000 github.com/Nerzal/gocloak/v13 v13.9.0 - github.com/PagerDuty/go-pagerduty v1.8.0 github.com/avast/retry-go/v4 v4.7.0 github.com/creack/pty v1.1.24 - github.com/bufbuild/buf v1.72.0 github.com/deckarep/golang-set/v2 v2.8.0 github.com/felixge/httpsnoop v1.1.0 github.com/fsnotify/fsnotify v1.9.0 @@ -28,7 +26,6 @@ require ( github.com/gogo/status v1.1.1 github.com/golang-jwt/jwt/v5 v5.3.0 github.com/golang/mock v1.6.0 - github.com/golangci/golangci-lint/v2 v2.12.2 github.com/google/jsonschema-go v0.4.2 github.com/google/uuid v1.6.0 github.com/gorilla/mux v1.8.1 @@ -44,7 +41,6 @@ require ( github.com/labstack/echo/v4 v4.15.0 github.com/lib/pq v1.12.3 github.com/metal-stack/v v1.0.3 - github.com/mgechev/revive v1.15.0 github.com/mitchellh/mapstructure v1.5.0 github.com/modelcontextprotocol/go-sdk v1.4.1 github.com/pkg/errors v0.9.1 @@ -95,7 +91,6 @@ require ( golang.org/x/term v0.45.0 golang.org/x/time v0.14.0 google.golang.org/grpc v1.79.3 - google.golang.org/grpc/cmd/protoc-gen-go-grpc v1.6.1 google.golang.org/protobuf v1.36.11 gopkg.in/fsnotify.v1 v1.4.7 gopkg.in/yaml.v2 v2.4.0 @@ -167,6 +162,7 @@ require ( github.com/bombsimon/wsl/v5 v5.8.0 // indirect github.com/breml/bidichk v0.3.3 // indirect github.com/breml/errchkjson v0.4.1 // indirect + github.com/bufbuild/buf v1.72.0 // indirect github.com/bufbuild/protocompile v0.14.2-0.20260716165721-bb5762d29672 // indirect github.com/bufbuild/protoplugin v0.0.0-20260414125817-25d1d281b46b // indirect github.com/buger/jsonparser v1.1.2 // indirect @@ -247,6 +243,7 @@ require ( github.com/golangci/dupl v0.0.0-20260401084720-c99c5cf5c202 // indirect github.com/golangci/go-printf-func-name v0.1.1 // indirect github.com/golangci/gofmt v0.0.0-20250106114630-d62b90e6713d // indirect + github.com/golangci/golangci-lint/v2 v2.12.2 // indirect github.com/golangci/golines v0.15.0 // indirect github.com/golangci/misspell v0.8.0 // indirect github.com/golangci/plugin-module-register v0.1.2 // indirect @@ -258,7 +255,6 @@ require ( github.com/google/gnostic-models v0.7.0 // indirect github.com/google/go-cmp v0.7.0 // indirect github.com/google/go-containerregistry v0.21.7 // indirect - github.com/google/go-querystring v1.2.0 // indirect github.com/gordonklaus/ineffassign v0.2.0 // indirect github.com/gostaticanalysis/analysisutil v0.7.1 // indirect github.com/gostaticanalysis/comment v1.5.0 // indirect @@ -319,6 +315,7 @@ require ( github.com/mattn/go-isatty v0.0.23 // indirect github.com/mattn/go-runewidth v0.0.23 // indirect github.com/mgechev/dots v1.0.0 // indirect + github.com/mgechev/revive v1.15.0 // indirect github.com/mitchellh/go-homedir v1.1.0 // indirect github.com/moby/docker-image-spec v1.3.1 // indirect github.com/moby/go-archive v0.2.0 // indirect @@ -447,6 +444,7 @@ require ( golang.org/x/tools v0.48.0 // indirect google.golang.org/genproto/googleapis/api v0.0.0-20260715232425-e75dac1f907d // indirect google.golang.org/genproto/googleapis/rpc v0.0.0-20260715232425-e75dac1f907d // indirect + google.golang.org/grpc/cmd/protoc-gen-go-grpc v1.6.1 // indirect gopkg.in/evanphx/json-patch.v4 v4.13.0 // indirect gopkg.in/inf.v0 v0.9.1 // indirect gopkg.in/validator.v2 v2.0.1 // indirect diff --git a/rest-api/go.sum b/rest-api/go.sum index 1640361701..8a3392e62c 100644 --- a/rest-api/go.sum +++ b/rest-api/go.sum @@ -92,8 +92,6 @@ github.com/Nerzal/gocloak/v13 v13.9.0 h1:YWsJsdM5b0yhM2Ba3MLydiOlujkBry4TtdzfIzS github.com/Nerzal/gocloak/v13 v13.9.0/go.mod h1:YYuDcXZ7K2zKECyVP7pPqjKxx2AzYSpKDj8d6GuyM10= github.com/OpenPeeDeeP/depguard/v2 v2.2.1 h1:vckeWVESWp6Qog7UZSARNqfu/cZqvki8zsuj3piCMx4= github.com/OpenPeeDeeP/depguard/v2 v2.2.1/go.mod h1:q4DKzC4UcVaAvcfd41CZh0PWpGgzrVxUYBlgKNGquUo= -github.com/PagerDuty/go-pagerduty v1.8.0 h1:MTFqTffIcAervB83U7Bx6HERzLbyaSPL/+oxH3zyluI= -github.com/PagerDuty/go-pagerduty v1.8.0/go.mod h1:nzIeAqyFSJAFkjWKvMzug0JtwDg+V+UoCWjFrfFH5mI= github.com/alecthomas/assert/v2 v2.11.0 h1:2Q9r3ki8+JYXvGsDyBXwH3LcJ+WK5D0gc5E8vS6K3D0= github.com/alecthomas/assert/v2 v2.11.0/go.mod h1:Bze95FyfUr7x34QZrjL+XP+0qgp/zg8yS+TtBj1WA3k= github.com/alecthomas/chroma/v2 v2.24.1 h1:m5ffpfZbIb++k8AqFEKy9uVgY12xIQtBsQlc6DfZJQM= @@ -400,13 +398,10 @@ github.com/google/go-cmp v0.4.0/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/ github.com/google/go-cmp v0.5.2/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= github.com/google/go-cmp v0.5.4/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= github.com/google/go-cmp v0.5.8/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY= -github.com/google/go-cmp v0.6.0/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY= github.com/google/go-cmp v0.7.0 h1:wk8382ETsv4JYUZwIsn6YpYiWiBsYLSJiTsyBybVuN8= github.com/google/go-cmp v0.7.0/go.mod h1:pXiqmnSA92OHEEa9HXL2W4E7lf9JzCmGVUdgjX3N/iU= github.com/google/go-containerregistry v0.21.7 h1:/vPFuVXDjtFREsVArW+0h1CIl5urnOhzei4X2DMW9IU= github.com/google/go-containerregistry v0.21.7/go.mod h1:kjSbt7/zMsKLWfnHrIvKvhXHUw91jbe9DNjPPJ32gXE= -github.com/google/go-querystring v1.2.0 h1:yhqkPbu2/OH+V9BfpCVPZkNmUXhb2gBxJArfhIxNtP0= -github.com/google/go-querystring v1.2.0/go.mod h1:8IFJqpSRITyJ8QhQ13bmbeMBDfmeEJZD5A0egEOmkqU= github.com/google/gofuzz v1.0.0/go.mod h1:dBl0BpW6vV/+mYPU4Po3pmUjxk6FQPldtuIdl/M65Eg= github.com/google/jsonschema-go v0.4.2 h1:tmrUohrwoLZZS/P3x7ex0WAVknEkBZM46iALbcqoRA8= github.com/google/jsonschema-go v0.4.2/go.mod h1:r5quNTdLOYEz95Ru18zA0ydNbBuYoo9tgaYcxEYhJVE= diff --git a/rest-api/site-agent/pkg/components/config/config_manager.go b/rest-api/site-agent/pkg/components/config/config_manager.go index 6320a1c2a3..135bcc97c2 100644 --- a/rest-api/site-agent/pkg/components/config/config_manager.go +++ b/rest-api/site-agent/pkg/components/config/config_manager.go @@ -179,6 +179,7 @@ func NewElektraConfig(utMode bool) *conftypes.Config { // General config flag.StringVar(&conf.MetricsPort, "metricsPort", os.Getenv("METRICS_PORT"), "Metrics port number") + flag.StringVar(&conf.MetricsNamespace, "metricsNamespace", os.Getenv("METRICS_NAMESPACE"), "Prefix applied to every exposed metric name") flag.StringVar(&conf.Temporal.Host, "temporalHost", os.Getenv("TEMPORAL_HOST"), "Temporal hostname/IP") flag.StringVar(&conf.Temporal.Port, "temporalPort", os.Getenv("TEMPORAL_PORT"), "Temporal port") flag.StringVar(&enableDebug, "enableDebug", os.Getenv("ENABLE_DEBUG"), "Debug log level setting") @@ -314,6 +315,12 @@ func NewElektraConfig(utMode bool) *conftypes.Config { log.Info().Interface("config", conf).Msg("Config Manager: Config loaded") flag.Parse() + + // Set default metrics namespace if not specified + if conf.MetricsNamespace == "" { + conf.MetricsNamespace = conftypes.DefaultMetricsNamespace + } + return conf } diff --git a/rest-api/site-agent/pkg/components/managers/bootstrap/bootstrap.go b/rest-api/site-agent/pkg/components/managers/bootstrap/bootstrap.go index 3665a21a56..8a57493c84 100644 --- a/rest-api/site-agent/pkg/components/managers/bootstrap/bootstrap.go +++ b/rest-api/site-agent/pkg/components/managers/bootstrap/bootstrap.go @@ -44,11 +44,10 @@ import ( var ( // ErrInvalidBootstrapSecret invalid bootstrap secret ErrInvalidBootstrapSecret = errors.New("invalid bootstrap secret") - // CertExpirationMetric is a prometheus metric for Site Agent Temporal certificate expiration - CertExpirationMetric = promauto.NewGauge(prometheus.GaugeOpts{ - Name: "temporal_cert_expiration", - Help: "The expiration date of the Temporal certificate", - }) + // CertExpirationMetric is a prometheus metric for Site Agent Temporal + // certificate expiration. Registered in Init rather than here, because the + // namespace comes from config that is not loaded yet at package init. + CertExpirationMetric prometheus.Gauge ) const ( @@ -144,6 +143,14 @@ func initK8sClient(ns string) coreV1Types.SecretInterface { func (bs *BoostrapAPI) Init() { ManagerAccess.Data.EB.Log.Info().Msg("Boostrap: Initializing the Site bootstrap manager") + // Registered ahead of the early returns below so every pod exposes the + // series, which is what a package-level promauto var used to do. + CertExpirationMetric = promauto.NewGauge(prometheus.GaugeOpts{ + Namespace: ManagerAccess.Conf.EB.MetricsNamespace, + Name: "temporal_cert_expiration", + Help: "The expiration date of the Temporal certificate", + }) + // Only master pod of the statefulset should run the bootstrap if !ManagerAccess.Conf.EB.IsMasterPod { return @@ -155,7 +162,7 @@ func (bs *BoostrapAPI) Init() { prometheus.MustRegister( prometheus.NewCounterFunc(prometheus.CounterOpts{ - Namespace: "elektra_site_agent", + Namespace: ManagerAccess.Conf.EB.MetricsNamespace, Name: MetricCredDnloadAttempt, Help: "Credentials download attempted for Site Agent", }, @@ -165,7 +172,7 @@ func (bs *BoostrapAPI) Init() { prometheus.MustRegister( prometheus.NewCounterFunc(prometheus.CounterOpts{ - Namespace: "elektra_site_agent", + Namespace: ManagerAccess.Conf.EB.MetricsNamespace, Name: MetricCredDnloadSucc, Help: "Credentials download succeeded for Site Agent", }, diff --git a/rest-api/site-agent/pkg/components/managers/coregrpc/init.go b/rest-api/site-agent/pkg/components/managers/coregrpc/init.go index 839d7f4946..601878a4cf 100644 --- a/rest-api/site-agent/pkg/components/managers/coregrpc/init.go +++ b/rest-api/site-agent/pkg/components/managers/coregrpc/init.go @@ -23,7 +23,7 @@ func (coregrpc *API) Init() { prometheus.MustRegister( prometheus.NewGaugeFunc(prometheus.GaugeOpts{ - Namespace: "elektra_site_agent", + Namespace: ManagerAccess.Conf.EB.MetricsNamespace, Name: MetricCoreGrpcStatus, Help: "Core gRPC health status", }, diff --git a/rest-api/site-agent/pkg/components/managers/coregrpc/metrics.go b/rest-api/site-agent/pkg/components/managers/coregrpc/metrics.go index 8aefce447a..29b58cf823 100644 --- a/rest-api/site-agent/pkg/components/managers/coregrpc/metrics.go +++ b/rest-api/site-agent/pkg/components/managers/coregrpc/metrics.go @@ -14,7 +14,6 @@ import ( ) const ( - metricsNamespace = "elektra_site_agent" metricCarbideGrpcLatency = "carbide_grpc_client_latency_seconds" metricWorkflowLatency = "workflow_latency_seconds" ) @@ -27,7 +26,7 @@ func makeGrpcClientMetrics() client.Metrics { metrics := &grpcClientMetrics{ responseLatency: prometheus.NewHistogramVec( prometheus.HistogramOpts{ - Namespace: metricsNamespace, + Namespace: ManagerAccess.Conf.EB.MetricsNamespace, Name: metricCarbideGrpcLatency, Help: "Response latency of each RPC", Buckets: []float64{0.0005, 0.001, 0.005, 0.010, 0.025, 0.050, 0.100, 0.250, 0.500, 1.0, 2.5, 5.0, 10.0}, @@ -71,7 +70,7 @@ func newWorkflowMetrics() coregrpctypes.WorkflowMetrics { metrics := &wflowMetrics{ latency: prometheus.NewHistogramVec( prometheus.HistogramOpts{ - Namespace: metricsNamespace, + Namespace: ManagerAccess.Conf.EB.MetricsNamespace, Name: metricWorkflowLatency, Help: "Latency of each workflow", Buckets: []float64{0.0005, 0.001, 0.005, 0.010, 0.025, 0.050, 0.100, 0.250, 0.500, 1.0, 2.5, 5.0, 10.0}, diff --git a/rest-api/site-agent/pkg/components/managers/flowgrpc/init.go b/rest-api/site-agent/pkg/components/managers/flowgrpc/init.go index 8a82b642dc..ecb16ea4e5 100644 --- a/rest-api/site-agent/pkg/components/managers/flowgrpc/init.go +++ b/rest-api/site-agent/pkg/components/managers/flowgrpc/init.go @@ -29,7 +29,7 @@ func (flowgrpc *API) Init() { ManagerAccess.Data.EB.Log.Info().Msg("Flow: Initializing Flow gRPC client manager") gauge := prometheus.NewGaugeFunc(prometheus.GaugeOpts{ - Namespace: "elektra_site_agent", + Namespace: ManagerAccess.Conf.EB.MetricsNamespace, Name: MetricFlowStatus, Help: "Flow gRPC health status", }, diff --git a/rest-api/site-agent/pkg/components/managers/flowgrpc/metrics.go b/rest-api/site-agent/pkg/components/managers/flowgrpc/metrics.go index 31bc0d2a48..07fcdb6f28 100644 --- a/rest-api/site-agent/pkg/components/managers/flowgrpc/metrics.go +++ b/rest-api/site-agent/pkg/components/managers/flowgrpc/metrics.go @@ -15,7 +15,6 @@ import ( ) const ( - metricsNamespace = "elektra_site_agent" metricFlowGrpcLatency = "flow_grpc_client_latency_seconds" metricFlowWorkflowLatency = "flow_workflow_latency_seconds" ) @@ -28,7 +27,7 @@ func makeGrpcClientMetrics() client.Metrics { metrics := &grpcClientMetrics{ responseLatency: prometheus.NewHistogramVec( prometheus.HistogramOpts{ - Namespace: metricsNamespace, + Namespace: ManagerAccess.Conf.EB.MetricsNamespace, Name: metricFlowGrpcLatency, Help: "Response latency of each RPC", Buckets: []float64{0.0005, 0.001, 0.005, 0.010, 0.025, 0.050, 0.100, 0.250, 0.500, 1.0, 2.5, 5.0, 10.0}, @@ -76,7 +75,7 @@ func newWorkflowMetrics() flowgrpctypes.WorkflowMetrics { metrics := &wflowMetrics{ latency: prometheus.NewHistogramVec( prometheus.HistogramOpts{ - Namespace: metricsNamespace, + Namespace: ManagerAccess.Conf.EB.MetricsNamespace, Name: metricFlowWorkflowLatency, Help: "Latency of each workflow", Buckets: []float64{0.0005, 0.001, 0.005, 0.010, 0.025, 0.050, 0.100, 0.250, 0.500, 1.0, 2.5, 5.0, 10.0}, diff --git a/rest-api/site-agent/pkg/components/managers/manager.go b/rest-api/site-agent/pkg/components/managers/manager.go index 268202d7bd..ab8df017ec 100644 --- a/rest-api/site-agent/pkg/components/managers/manager.go +++ b/rest-api/site-agent/pkg/components/managers/manager.go @@ -131,9 +131,9 @@ func (Managers *Manager) Init() { ManagerAccess.Data.EB.Log.Info().Msg("Managers: Initializing all the managers") // register version metric (build_version, build_date) versionGauge := prometheus.NewGaugeVec(prometheus.GaugeOpts{ - Namespace: "elektra_site_agent", + Namespace: ManagerAccess.Conf.EB.MetricsNamespace, Name: "version", - Help: "version of the elektra_site_agent", + Help: "version of the Site Agent", }, []string{"build_version", "build_date"}) prometheus.MustRegister(versionGauge) // set the value once, since it does not change @@ -141,9 +141,9 @@ func (Managers *Manager) Init() { // register health status metric prometheus.MustRegister( prometheus.NewCounterFunc(prometheus.CounterOpts{ - Namespace: "elektra_site_agent", + Namespace: ManagerAccess.Conf.EB.MetricsNamespace, Name: "health_status", - Help: "health status of the elektra_site_agent", + Help: "health status of the Site Agent", }, func() float64 { return float64(ManagerAccess.Data.EB.HealthStatus.Load()) diff --git a/rest-api/site-agent/pkg/components/managers/workflow/init.go b/rest-api/site-agent/pkg/components/managers/workflow/init.go index f3c115300d..7d054e4f70 100644 --- a/rest-api/site-agent/pkg/components/managers/workflow/init.go +++ b/rest-api/site-agent/pkg/components/managers/workflow/init.go @@ -27,9 +27,9 @@ func (wflow *API) Init() { prometheus.MustRegister( prometheus.NewCounterFunc(prometheus.CounterOpts{ - Namespace: "elektra_site_agent", + Namespace: ManagerAccess.Conf.EB.MetricsNamespace, Name: MetricTemporalConnStatus, - Help: "temporal health status of the elektra_site_agent", + Help: "Temporal health status of the Site Agent", }, func() float64 { return float64(ManagerAccess.Data.EB.Managers.Workflow.State.HealthStatus.Load()) @@ -38,9 +38,9 @@ func (wflow *API) Init() { prometheus.MustRegister( prometheus.NewCounterFunc(prometheus.CounterOpts{ - Namespace: "elektra_site_agent", + Namespace: ManagerAccess.Conf.EB.MetricsNamespace, Name: MetricTemporalConnAttempted, - Help: "temporal connection attempted of elektra_site_agent", + Help: "Temporal connections attempted by the Site Agent", }, func() float64 { return float64(ManagerAccess.Data.EB.Managers.Workflow.State.ConnectionAttempted.Load()) @@ -48,9 +48,9 @@ func (wflow *API) Init() { prometheus.MustRegister( prometheus.NewCounterFunc(prometheus.CounterOpts{ - Namespace: "elektra_site_agent", + Namespace: ManagerAccess.Conf.EB.MetricsNamespace, Name: MetricTemporalConnSucc, - Help: "temporal connection succeded of elektra_site_agent", + Help: "Temporal connections succeeded by the Site Agent", }, func() float64 { return float64(ManagerAccess.Data.EB.Managers.Workflow.State.ConnectionSucc.Load()) diff --git a/rest-api/site-agent/pkg/conftypes/conftypes.go b/rest-api/site-agent/pkg/conftypes/conftypes.go index 6c4f4aaa13..8bc8852125 100644 --- a/rest-api/site-agent/pkg/conftypes/conftypes.go +++ b/rest-api/site-agent/pkg/conftypes/conftypes.go @@ -14,6 +14,12 @@ import ( // in which the app is running. type RunInEnvironment int +// DefaultMetricsNamespace prefixes every metric the Site Agent exposes and +// matches its nico-rest-site-agent Helm service name. Operators override it with +// METRICS_NAMESPACE, which the config manager resolves once so the six manager +// packages that declare metrics all read the same value. +const DefaultMetricsNamespace = "nico_rest_site_agent" + const ( // RunningInUnknown - Running In Unknown Env RunningInUnknown RunInEnvironment = iota @@ -108,6 +114,7 @@ type Config struct { PodNamespace string `json:"podNamespace"` TemporalSecret string `json:"temporalSecret"` MetricsPort string `json:"metricsPort"` + MetricsNamespace string `json:"metricsNamespace"` SiteVersion string `json:"siteVersion"` CloudVersion string `json:"cloudVersion"` RunningIn RunInEnvironment diff --git a/rest-api/site-manager/pkg/sitemgr/manager.go b/rest-api/site-manager/pkg/sitemgr/manager.go index ba0fdab440..4b9bb2a344 100644 --- a/rest-api/site-manager/pkg/sitemgr/manager.go +++ b/rest-api/site-manager/pkg/sitemgr/manager.go @@ -113,7 +113,7 @@ func newSiteManager(ctx context.Context, o Options, c crdclient.Interface) (*Sit appService.AddHealthRoute(ctx) appService.AddVersionRoute(ctx) appService.AddMetricsRoute(ctx) - appService.Use(core.NewHTTPMiddleware(ctx, core.WithRequestMetrics("cloud_site_manager"))...) + appService.Use(core.NewHTTPMiddleware(ctx, core.WithRequestMetrics("nico_rest_site_manager"))...) appService.Path("/v1/site").Handler(s.siteCreateHandler()).Methods("POST") appService.Path("/v1/site/{uuid}").Handler(s.siteGetHandler()).Methods("GET") appService.Path("/v1/site/roll/{uuid}").Handler(s.siteRollHandler()).Methods("POST") diff --git a/rest-api/workflow/cmd/workflow/main.go b/rest-api/workflow/cmd/workflow/main.go index 447fa471db..9c38261794 100644 --- a/rest-api/workflow/cmd/workflow/main.go +++ b/rest-api/workflow/cmd/workflow/main.go @@ -333,6 +333,46 @@ func main() { w.RegisterWorkflow(nvLinkLogicalPartitionWorkflow.UpdateNVLinkLogicalPartitionInventory) } + // Metric setup has to precede the activity registrations below, because the + // activities hold their own metric handles and the worker will not accept a + // registration once it is running. Only serving can wait for the goroutine + // further down. + mconfig := cfg.GetMetricsConfig() + + var reg *prometheus.Registry + var siteHealthMetrics *cwm.SiteHealthMetrics + + if mconfig.Enabled { + reg = prometheus.NewRegistry() + reg.MustRegister(collectors.NewGoCollector()) + + // Register core metrics + cm := cwm.NewCoreMetrics(reg, mconfig.Namespace) + // TODO: Set version here when available + cm.Info.With(prometheus.Labels{"version": "unknown", "namespace": tcfg.Namespace}).Set(1) + + // Published by the Site health monitor cron, which runs on the Cloud queue. + siteHealthMetrics = cwm.NewSiteHealthMetrics(reg, mconfig.Namespace) + + if tcfg.Namespace == cwfn.SiteNamespace { + // The inventory workflows that report these metrics only run here. + + // Register common inventory metrics activity + inventoryMetricsManager := cwm.NewManageInventoryMetrics(reg, dbSession, mconfig.Namespace) + w.RegisterActivity(inventoryMetricsManager) + + // Register inventory operation metrics activity + vpcLifecycleMetricsManager := vpcActivity.NewManageVpcLifecycleMetrics(reg, dbSession, mconfig.Namespace) + w.RegisterActivity(&vpcLifecycleMetricsManager) + + subnetLifecycleMetricsManager := subnetActivity.NewManageSubnetLifecycleMetrics(reg, dbSession, mconfig.Namespace) + w.RegisterActivity(&subnetLifecycleMetricsManager) + + instanceLifecycleMetricsManager := instanceActivity.NewManageInstanceLifecycleMetrics(reg, dbSession, mconfig.Namespace) + w.RegisterActivity(&instanceLifecycleMetricsManager) + } + } + // Register activities // Common activities machineManager := machineActivity.NewManageMachine(dbSession, siteClientPool) @@ -347,7 +387,7 @@ func main() { instanceManager := instanceActivity.NewManageInstance(dbSession, siteClientPool, tc, cfg) w.RegisterActivity(&instanceManager) - siteManager := siteActivity.NewManageSite(dbSession, siteClientPool, tc, cfg) + siteManager := siteActivity.NewManageSite(dbSession, siteClientPool, tc, cfg, siteHealthMetrics) w.RegisterActivity(&siteManager) sshKeyGroupManager := sshKeyGroupActivity.NewManageSSHKeyGroup(dbSession, siteClientPool) @@ -426,36 +466,11 @@ func main() { }() } - mconfig := cfg.GetMetricsConfig() if mconfig.Enabled { // Serve Prometheus metrics go func() { log.Info().Msg("starting Prometheus metrics server") - reg := prometheus.NewRegistry() - reg.MustRegister(collectors.NewGoCollector()) - - // Register core metrics - cm := cwm.NewCoreMetrics(reg) - // TODO: Set version here when available - cm.Info.With(prometheus.Labels{"version": "unknown", "namespace": tcfg.Namespace}).Set(1) - - if tcfg.Namespace == cwfn.SiteNamespace { - // Register common inventory metrics activity - inventoryMetricsManager := cwm.NewManageInventoryMetrics(reg, dbSession) - w.RegisterActivity(&inventoryMetricsManager) - - // Register inventory operation metrics activity - vpcLifecycleMetricsManager := vpcActivity.NewManageVpcLifecycleMetrics(reg, dbSession) - w.RegisterActivity(&vpcLifecycleMetricsManager) - - subnetLifecycleMetricsManager := subnetActivity.NewManageSubnetLifecycleMetrics(reg, dbSession) - w.RegisterActivity(&subnetLifecycleMetricsManager) - - instanceLifecycleMetricsManager := instanceActivity.NewManageInstanceLifecycleMetrics(reg, dbSession) - w.RegisterActivity(&instanceLifecycleMetricsManager) - } - promHandler := promhttp.HandlerFor(reg, promhttp.HandlerOpts{Registry: reg}) http.Handle("/metrics", promHandler) diff --git a/rest-api/workflow/internal/config/config.go b/rest-api/workflow/internal/config/config.go index 63feb28466..5dafcfa59e 100644 --- a/rest-api/workflow/internal/config/config.go +++ b/rest-api/workflow/internal/config/config.go @@ -81,11 +81,6 @@ const ( // ConfigNotificationsSlackWebhookURLPath specifies file path to read Slack webhook URL ConfigNotificationsSlackWebhookURLPath = "notifications.slack.webhookURLPath" - // ConfigNotificationsPagerDutyIntegrationKey specifies the PagerDuty integration key - ConfigNotificationsPagerDutyIntegrationKey = "notifications.pagerduty.integrationKey" - // ConfigNotificationsPagerDutyIntegrationKeyPath specifies file path to read PagerDuty integration key - ConfigNotificationsPagerDutyIntegrationKeyPath = "notifications.pagerduty.integrationKeyPath" - // ConfigSiteManagerEndpoint is the service endpoint for site manager ConfigSiteManagerEndpoint = "siteManager.svcEndpoint" @@ -93,6 +88,8 @@ const ( ConfigMetricsEnabled = "metrics.enabled" // ConfigMetricsPort specifies the port for Prometheus metrics ConfigMetricsPort = "metrics.port" + // ConfigMetricsNamespace specifies the prefix for every exposed metric name + ConfigMetricsNamespace = "metrics.namespace" // ConfigHealthzEnabled is a feature flag for health check endpoint ConfigHealthzEnabled = "healthz.enabled" @@ -147,6 +144,7 @@ func NewConfig() *Config { c.v.SetDefault(ConfigMetricsEnabled, true) c.v.SetDefault(ConfigMetricsPort, 9360) + c.v.SetDefault(ConfigMetricsNamespace, DefaultMetricsNamespace) c.v.SetDefault(ConfigHealthzEnabled, true) c.v.SetDefault(ConfigHealthzPort, 8899) @@ -176,10 +174,6 @@ func NewConfig() *Config { c.setNotificationsSlackWebhookURL() } - if c.GetNotificationsPagerDutyIntegrationKeyPath() != "" { - c.setNotificationsPagerDutyIntegrationKey() - } - c.setTemporalNamespace() c.setTemporalQueue() @@ -288,18 +282,6 @@ func (c *Config) setNotificationsSlackWebhookURL() { } } -// setNotificationsPagerDutyIntegrationKey sets the PagerDuty integration key by reading from integration key path -func (c *Config) setNotificationsPagerDutyIntegrationKey() { - log.Warn().Str("notifications.pagerduty.integrationKeyPath", c.GetNotificationsPagerDutyIntegrationKeyPath()).Msg("setting PagerDuty integration key by reading from integration key path") - - integrationKeyBytes, err := os.ReadFile(c.GetNotificationsPagerDutyIntegrationKeyPath()) - if err != nil { - log.Err(err).Str("notifications.pagerduty.integrationKeyPath", c.GetNotificationsPagerDutyIntegrationKeyPath()).Msg("failed to read PagerDuty integration key from file") - } else { - c.v.Set(ConfigNotificationsPagerDutyIntegrationKey, string(integrationKeyBytes)) - } -} - // setTemporalNamespace sets the namespace for the temporal client func (c *Config) setTemporalNamespace() { // Check for env var override @@ -340,7 +322,7 @@ func (c *Config) GetTemporalConfig() (*cconfig.TemporalConfig, error) { // GetMetricsConfig returns the Metrics config func (c *Config) GetMetricsConfig() *MetricsConfig { - return NewMetricsConfig(c.GetMetricsEnabled(), c.GetMetricsPort()) + return NewMetricsConfig(c.GetMetricsEnabled(), c.GetMetricsPort(), c.GetMetricsNamespace()) } // GetHealthzConfig returns the Healthz config @@ -515,26 +497,6 @@ func (c *Config) GetNotificationsSlackWebhookURLPath() string { return c.v.GetString(ConfigNotificationsSlackWebhookURLPath) } -// GetNotificationsPagerDutyEnabled returns if PagerDuty notifications are enabled -func (c *Config) GetNotificationsPagerDutyEnabled() bool { - return c.GetNotificationsPagerDutyIntegrationKey() != "" || c.GetNotificationsPagerDutyIntegrationKeyPath() != "" -} - -// GetNotificationsPagerDutyIntegrationKey gets the PagerDuty integration key -func (c *Config) GetNotificationsPagerDutyIntegrationKey() string { - return c.v.GetString(ConfigNotificationsPagerDutyIntegrationKey) -} - -// SetNotificationsPagerDutyIntegrationKey sets the PagerDuty integration key -func (c *Config) SetNotificationsPagerDutyIntegrationKey(value string) { - c.v.Set(ConfigNotificationsPagerDutyIntegrationKey, value) -} - -// GetNotificationsPagerDutyIntegrationKeyPath gets the file path to read PagerDuty integration key -func (c *Config) GetNotificationsPagerDutyIntegrationKeyPath() string { - return c.v.GetString(ConfigNotificationsPagerDutyIntegrationKeyPath) -} - // SetSiteManagerEndpoint sets the endpoint func (c *Config) SetSiteManagerEndpoint(value string) { c.v.Set(ConfigSiteManagerEndpoint, value) @@ -555,6 +517,17 @@ func (c *Config) GetMetricsPort() int { return c.v.GetInt(ConfigMetricsPort) } +// GetMetricsNamespace gets the prefix applied to every exposed metric name. +// An explicitly empty value falls back to the default rather than exposing +// unprefixed names that would collide with another service's. +func (c *Config) GetMetricsNamespace() string { + namespace := c.v.GetString(ConfigMetricsNamespace) + if namespace == "" { + return DefaultMetricsNamespace + } + return namespace +} + // GetHealthzEnabled gets the enabled field for Healthz func (c *Config) GetHealthzEnabled() bool { return c.v.GetBool(ConfigHealthzEnabled) diff --git a/rest-api/workflow/internal/config/metrics.go b/rest-api/workflow/internal/config/metrics.go index 128a176955..c16536923c 100644 --- a/rest-api/workflow/internal/config/metrics.go +++ b/rest-api/workflow/internal/config/metrics.go @@ -7,10 +7,16 @@ import ( "fmt" ) +// DefaultMetricsNamespace prefixes every metric this worker exposes and matches +// its nico-rest-workflow Helm service name. Operators override it with +// metrics.namespace. +const DefaultMetricsNamespace = "nico_rest_workflow" + // MetricsConfig holds configuration of Metrics type MetricsConfig struct { - Enabled bool - Port int + Enabled bool + Port int + Namespace string } // GetListenAddr returns the local address for listen socket. @@ -19,9 +25,10 @@ func (mcfg *MetricsConfig) GetListenAddr() string { } // NewMetricsConfig initializes and returns a configuration object for managing Metrics -func NewMetricsConfig(enabled bool, port int) *MetricsConfig { +func NewMetricsConfig(enabled bool, port int, namespace string) *MetricsConfig { return &MetricsConfig{ - Enabled: enabled, - Port: port, + Enabled: enabled, + Port: port, + Namespace: namespace, } } diff --git a/rest-api/workflow/internal/config/metrics_test.go b/rest-api/workflow/internal/config/metrics_test.go index 11cc67d20a..10171ffd4f 100644 --- a/rest-api/workflow/internal/config/metrics_test.go +++ b/rest-api/workflow/internal/config/metrics_test.go @@ -5,17 +5,15 @@ package config import ( "testing" + + "github.com/stretchr/testify/assert" ) func TestMetricsConfig(t *testing.T) { type args struct { - enabled bool - port int - } - - mcfg := MetricsConfig{ - Enabled: true, - Port: 6930, + enabled bool + port int + namespace string } tests := []struct { @@ -26,23 +24,71 @@ func TestMetricsConfig(t *testing.T) { { name: "initialize Metrics config", args: args{ - enabled: true, - port: mcfg.Port, + enabled: true, + port: 6930, + namespace: DefaultMetricsNamespace, + }, + want: &MetricsConfig{ + Enabled: true, + Port: 6930, + Namespace: DefaultMetricsNamespace, + }, + }, + { + name: "initialize Metrics config with an overridden namespace", + args: args{ + enabled: true, + port: 6930, + namespace: "acme_workflow", + }, + want: &MetricsConfig{ + Enabled: true, + Port: 6930, + Namespace: "acme_workflow", }, - want: &mcfg, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got := NewMetricsConfig(tt.args.enabled, tt.args.port) + got := NewMetricsConfig(tt.args.enabled, tt.args.port, tt.args.namespace) + + assert.Equal(t, tt.want, got) + assert.Equal(t, tt.want.GetListenAddr(), got.GetListenAddr()) + }) + } +} - if p := got.Port; p != tt.want.Port { - t.Errorf("got.Port = %v, want %v", p, tt.want.Port) - } +func TestConfig_GetMetricsNamespace(t *testing.T) { + tests := []struct { + name string + configure func(c *Config) + want string + }{ + { + name: "unset falls back to the default", + configure: func(c *Config) {}, + want: DefaultMetricsNamespace, + }, + { + // An operator who blanks the key gets the default rather than + // unprefixed metric names that would collide with another service. + name: "explicitly empty falls back to the default", + configure: func(c *Config) { c.v.Set(ConfigMetricsNamespace, "") }, + want: DefaultMetricsNamespace, + }, + { + name: "override is honored", + configure: func(c *Config) { c.v.Set(ConfigMetricsNamespace, "acme_workflow") }, + want: "acme_workflow", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + c := NewConfig() + tt.configure(c) + defer c.v.Set(ConfigMetricsNamespace, DefaultMetricsNamespace) - if got := got.GetListenAddr(); got != tt.want.GetListenAddr() { - t.Errorf("GetListenAddr() = %v, want %v", got, tt.want.GetListenAddr()) - } + assert.Equal(t, tt.want, c.GetMetricsNamespace()) }) } } diff --git a/rest-api/workflow/internal/inventory/options.go b/rest-api/workflow/internal/inventory/options.go new file mode 100644 index 0000000000..623f1278df --- /dev/null +++ b/rest-api/workflow/internal/inventory/options.go @@ -0,0 +1,64 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +// Package inventory holds the settings every Cloud inventory workflow shares. +package inventory + +import ( + "time" + + "go.temporal.io/sdk/temporal" + "go.temporal.io/sdk/workflow" +) + +const ( + // ActivityStartToCloseTimeout bounds one page of inventory, which the Site + // Agent caps at 25 objects. + // + // The floor is the work itself. The heaviest pages are Instance Type, + // Machine, and Instance, which reach roughly 3,100 sequential database round + // trips for a full page because they loop over each object's capabilities or + // interfaces. At 5ms per round trip that is about 15s, so this leaves roughly + // four times the measured worst case. + // + // The ceiling is the retry budget. A page that exhausts ActivityMaximumAttempts + // costs 2 x this + ActivityInitialInterval, and that total has to stay inside + // cutil.DefaultInventoryReceiptInterval so a fully retried cycle does not run + // into the next one. 60s puts the total at 125s against a 180s interval. + // + // Anything that waits on another service inside the activity is bound by this + // too, so an inner deadline longer than this can never be reached. Site Agent + // SSH Key Group sync is the one place that happens, and it sets its own + // timeout rather than using this. + ActivityStartToCloseTimeout = 60 * time.Second + + // ActivityMaximumAttempts retries a page once. A second failure means the page + // is not landing, and retrying past that only delays the next cycle, which + // carries the same data anyway. + ActivityMaximumAttempts = 2 + + // ActivityInitialInterval is the wait before the single retry. + ActivityInitialInterval = 5 * time.Second + + // ActivityBackoffCoefficient grows the wait between attempts. + ActivityBackoffCoefficient = 2.0 + + // ActivityMaximumInterval caps the wait between attempts. + ActivityMaximumInterval = 30 * time.Second +) + +// ActivityOptions returns the Temporal activity options every inventory workflow +// uses. Shared rather than repeated per workflow, because the timeouts only mean +// something as a set: they are chosen against one page size and one retry budget, +// and a workflow that drifts from them silently opts out of both. +func ActivityOptions() workflow.ActivityOptions { + return workflow.ActivityOptions{ + StartToCloseTimeout: ActivityStartToCloseTimeout, + RetryPolicy: &temporal.RetryPolicy{ + InitialInterval: ActivityInitialInterval, + BackoffCoefficient: ActivityBackoffCoefficient, + MaximumInterval: ActivityMaximumInterval, + MaximumAttempts: ActivityMaximumAttempts, + }, + } +} diff --git a/rest-api/workflow/internal/metrics/core.go b/rest-api/workflow/internal/metrics/core.go index 5d0303259d..36d08eb4a3 100644 --- a/rest-api/workflow/internal/metrics/core.go +++ b/rest-api/workflow/internal/metrics/core.go @@ -7,19 +7,15 @@ import ( "github.com/prometheus/client_golang/prometheus" ) -const ( - MetricsNamespace = "cloud_workflow" -) - type coreMetrics struct { Info *prometheus.GaugeVec } // NewCoreMetrics creates a new coreMetrics struct and registers the metrics with the provided registerer -func NewCoreMetrics(reg prometheus.Registerer) *coreMetrics { +func NewCoreMetrics(reg prometheus.Registerer, namespace string) *coreMetrics { m := &coreMetrics{ Info: prometheus.NewGaugeVec(prometheus.GaugeOpts{ - Namespace: MetricsNamespace, + Namespace: namespace, Name: "info", Help: "Information about the Cloud/Site worker", }, []string{"version", "namespace"}), diff --git a/rest-api/workflow/internal/metrics/inventory.go b/rest-api/workflow/internal/metrics/inventory.go index 9a0f1e9c85..2c2e043d33 100644 --- a/rest-api/workflow/internal/metrics/inventory.go +++ b/rest-api/workflow/internal/metrics/inventory.go @@ -11,7 +11,6 @@ import ( "github.com/prometheus/client_golang/prometheus" cdb "github.com/NVIDIA/infra-controller/rest-api/db/pkg/db" - cdbm "github.com/NVIDIA/infra-controller/rest-api/db/pkg/db/model" ) const ( @@ -37,9 +36,9 @@ type InventoryObjectLifecycleEvent struct { // ManageInventoryMetrics is a wrapper for managing inventory metrics activities type ManageInventoryMetrics struct { - dbSession *cdb.Session - latency *prometheus.HistogramVec - siteIDNameMap map[uuid.UUID]string + dbSession *cdb.Session + latency *prometheus.HistogramVec + siteNames *SiteNameCache } // RecordLatency is a Temporal activity that records the latency of inventory processing activities @@ -51,37 +50,33 @@ func (mim *ManageInventoryMetrics) RecordLatency(ctx context.Context, siteID uui status = InventoryStatusFailed } - // Cache site name to avoid repeated DB call - siteName, ok := mim.siteIDNameMap[siteID] - if !ok { - siteDAO := cdbm.NewSiteDAO(mim.dbSession) - site, err := siteDAO.GetByID(context.Background(), nil, siteID, nil, false) - if err != nil { - return err - } - siteName = site.Name - mim.siteIDNameMap[siteID] = siteName + siteName, err := mim.siteNames.Get(ctx, mim.dbSession, siteID) + if err != nil { + return err } - mim.latency.WithLabelValues(siteName, activity, status).Observe(duration.Seconds()) + mim.latency.WithLabelValues(siteName, siteID.String(), activity, status).Observe(duration.Seconds()) return nil } // InitInventoryMetrics initializes inventory activity metrics -func NewManageInventoryMetrics(reg prometheus.Registerer, dbSession *cdb.Session) ManageInventoryMetrics { - inventoryMetrics := ManageInventoryMetrics{ +func NewManageInventoryMetrics(reg prometheus.Registerer, dbSession *cdb.Session, namespace string) *ManageInventoryMetrics { + inventoryMetrics := &ManageInventoryMetrics{ dbSession: dbSession, latency: prometheus.NewHistogramVec( prometheus.HistogramOpts{ - Namespace: MetricsNamespace, + Namespace: namespace, Name: "inventory_latency_seconds", - Help: "Latency of each inventory call", - Buckets: []float64{0.0005, 0.001, 0.005, 0.010, 0.025, 0.050, 0.100, 0.250, 0.500, 1.0, 2.5, 5.0, 10.0}, + Help: "Latency of each inventory call, measured across the whole workflow including activity retries", + // Top bucket covers a fully retried run under the shared inventory + // budget, which is 2 x 60s plus 5s of backoff. Only SSH Key Group sets + // its own longer timeout, so its slowest runs fall in +Inf. + Buckets: []float64{0.0005, 0.001, 0.005, 0.010, 0.025, 0.050, 0.100, 0.250, 0.500, 1.0, 2.5, 5.0, 10.0, 30.0, 60.0, 125.0}, }, - []string{"site", "activity", "status"}), + []string{"site", "site_id", "activity", "status"}), - siteIDNameMap: map[uuid.UUID]string{}, + siteNames: NewSiteNameCache(), } reg.MustRegister(inventoryMetrics.latency) diff --git a/rest-api/workflow/internal/metrics/inventory_test.go b/rest-api/workflow/internal/metrics/inventory_test.go index f9ede80b30..9d4c40cf33 100644 --- a/rest-api/workflow/internal/metrics/inventory_test.go +++ b/rest-api/workflow/internal/metrics/inventory_test.go @@ -5,6 +5,7 @@ package metrics import ( "context" + "sync" "testing" "time" @@ -35,16 +36,47 @@ func TestManageInventoryMetrics_RecordLatency(t *testing.T) { reg := prometheus.NewRegistry() reg.MustRegister(collectors.NewGoCollector()) - inventoryMetricsManager := NewManageInventoryMetrics(reg, dbSession) + inventoryMetricsManager := NewManageInventoryMetrics(reg, dbSession, "nico_rest_workflow") - err := inventoryMetricsManager.RecordLatency(context.Background(), site.ID, "test-workflow", false, time.Second) - assert.NoError(t, err) + t.Run("records an observation and caches the Site name", func(t *testing.T) { + err := inventoryMetricsManager.RecordLatency(context.Background(), site.ID, "test-workflow", false, time.Second) + assert.NoError(t, err) - metrics, err := reg.Gather() - assert.NoError(t, err) - assert.Equal(t, "test-workflow", *metrics[0].Metric[0].Label[0].Value) - assert.Equal(t, site.Name, *metrics[0].Metric[0].Label[1].Value) - assert.Equal(t, InventoryStatusSuccess, *metrics[0].Metric[0].Label[2].Value) + util.TestAssertMetricExistsTimes(t, reg, "nico_rest_workflow_inventory_latency_seconds", 1, map[string]string{ + "activity": "test-workflow", + "site": site.Name, + "site_id": site.ID.String(), + "status": InventoryStatusSuccess, + }, 0) - assert.Equal(t, 1, len(inventoryMetricsManager.siteIDNameMap)) + assert.Equal(t, 1, inventoryMetricsManager.siteNames.Len()) + }) + + // The worker dispatches this activity concurrently against one registered + // instance, and a cold cache is what makes that racy: every caller misses, + // reads the Site, then writes the same map. Run under -race. + t.Run("resolves the Site name under concurrent callers", func(t *testing.T) { + const callers = 16 + + concurrentReg := prometheus.NewRegistry() + concurrentManager := NewManageInventoryMetrics(concurrentReg, dbSession, "nico_rest_workflow") + + var wg sync.WaitGroup + for range callers { + wg.Add(1) + go func() { + defer wg.Done() + cerr := concurrentManager.RecordLatency(context.Background(), site.ID, "test-workflow", false, time.Second) + assert.NoError(t, cerr) + }() + } + wg.Wait() + + metrics, err := concurrentReg.Gather() + assert.NoError(t, err) + assert.Len(t, metrics, 1) + assert.Equal(t, "nico_rest_workflow_inventory_latency_seconds", metrics[0].GetName()) + assert.Len(t, metrics[0].Metric, 1) + assert.Equal(t, uint64(callers), metrics[0].Metric[0].Histogram.GetSampleCount()) + }) } diff --git a/rest-api/workflow/internal/metrics/site.go b/rest-api/workflow/internal/metrics/site.go new file mode 100644 index 0000000000..6c21fa53a3 --- /dev/null +++ b/rest-api/workflow/internal/metrics/site.go @@ -0,0 +1,93 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +package metrics + +import ( + "time" + + "github.com/google/uuid" + "github.com/prometheus/client_golang/prometheus" +) + +// SiteHealthReport is one monitored Site's health signals for a single monitor +// cycle. Both timestamps are nil when the Site has never reported that signal. +type SiteHealthReport struct { + SiteID uuid.UUID + SiteName string + InventoryReceived *time.Time + AgentCertExpiry *time.Time +} + +// SiteHealthMetrics publishes per-Site health signals gathered by the Site +// health monitor cron. +// +// The cron is a single Temporal chain, so one Cloud worker replica runs each +// cycle and only that replica updates its own gauges. With +// cloudWorker.replicaCount above 1, Prometheus scrapes replicas whose values are +// as old as the last cycle they won, so an alert on these gauges has to collapse +// the replicas first, as in max by (site_id) (...). A replica scaled up +// mid-flight reports nothing until it wins a cycle. +type SiteHealthMetrics struct { + lastInventoryReceipt *prometheus.GaugeVec + agentCertExpiry *prometheus.GaugeVec +} + +// SetSiteHealth republishes every gauge for every monitored Site, as Unix +// timestamps that operators compare against time(). A signal the Site has never +// reported is published as 0 rather than left absent, so a Site that never +// connects is as visible as one that stops, and 0 reads as overdue under the +// same comparison that covers a real timestamp. +// +// Both vectors are rebuilt from the caller's set on each cycle. Without that, a +// deleted or de-registered Site would keep its last value and age into an alert +// nothing can clear. +func (shm *SiteHealthMetrics) SetSiteHealth(reports []SiteHealthReport) { + // Nil when the worker was built with metrics disabled, and in activity tests. + if shm == nil { + return + } + + shm.lastInventoryReceipt.Reset() + shm.agentCertExpiry.Reset() + + for _, report := range reports { + site, siteID := report.SiteName, report.SiteID.String() + + var inventoryReceived float64 + if report.InventoryReceived != nil { + inventoryReceived = float64(report.InventoryReceived.Unix()) + } + shm.lastInventoryReceipt.WithLabelValues(site, siteID).Set(inventoryReceived) + + var agentCertExpiry float64 + if report.AgentCertExpiry != nil { + agentCertExpiry = float64(report.AgentCertExpiry.Unix()) + } + shm.agentCertExpiry.WithLabelValues(site, siteID).Set(agentCertExpiry) + } +} + +// NewSiteHealthMetrics initializes per-Site health metrics +func NewSiteHealthMetrics(reg prometheus.Registerer, namespace string) *SiteHealthMetrics { + siteHealthMetrics := &SiteHealthMetrics{ + lastInventoryReceipt: prometheus.NewGaugeVec( + prometheus.GaugeOpts{ + Namespace: namespace, + Name: "site_last_inventory_receipt_timestamp_seconds", + Help: "Unix timestamp of the last Machine inventory received from a monitored Site, or 0 if none has been received. Published by one worker replica per cycle, so aggregate with max by (site_id)", + }, + []string{"site", "site_id"}), + + agentCertExpiry: prometheus.NewGaugeVec( + prometheus.GaugeOpts{ + Namespace: namespace, + Name: "site_agent_cert_expiry_timestamp_seconds", + Help: "Unix timestamp at which a monitored Site's Site Agent Temporal certificate expires, or 0 if the Site has never reported one. Published by one worker replica per cycle, so aggregate with max by (site_id)", + }, + []string{"site", "site_id"}), + } + reg.MustRegister(siteHealthMetrics.lastInventoryReceipt, siteHealthMetrics.agentCertExpiry) + + return siteHealthMetrics +} diff --git a/rest-api/workflow/internal/metrics/sitename.go b/rest-api/workflow/internal/metrics/sitename.go new file mode 100644 index 0000000000..117bdf330c --- /dev/null +++ b/rest-api/workflow/internal/metrics/sitename.go @@ -0,0 +1,66 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +package metrics + +import ( + "context" + "sync" + + "github.com/google/uuid" + + cdb "github.com/NVIDIA/infra-controller/rest-api/db/pkg/db" + cdbm "github.com/NVIDIA/infra-controller/rest-api/db/pkg/db/model" +) + +// SiteNameCache resolves Site names for metric labels without a DB read per +// observation. It is shared by every metrics activity rather than reimplemented +// per package, because the worker runs activities concurrently against one +// registered instance and an unsynchronized map here is a concurrent write that +// kills the process. +// +// A name is cached for the life of the worker. A Site renamed after its first +// observation keeps reporting the old name until the worker restarts, and the +// restart then splits its history across two values of the site label. Every +// metric using this cache also carries site_id, which stays stable across a +// rename, so that is the label to aggregate on. +type SiteNameCache struct { + mutex sync.RWMutex + names map[uuid.UUID]string +} + +// Get returns the Site's name, reading it from the DB on the first call for +// that Site. Concurrent callers may both miss and read the same Site; that +// costs a duplicate query rather than holding the write lock across the DB call. +func (snc *SiteNameCache) Get(ctx context.Context, dbSession *cdb.Session, siteID uuid.UUID) (string, error) { + snc.mutex.RLock() + name, ok := snc.names[siteID] + snc.mutex.RUnlock() + if ok { + return name, nil + } + + siteDAO := cdbm.NewSiteDAO(dbSession) + site, err := siteDAO.GetByID(ctx, nil, siteID, nil, false) + if err != nil { + return "", err + } + + snc.mutex.Lock() + snc.names[siteID] = site.Name + snc.mutex.Unlock() + + return site.Name, nil +} + +// Len returns the number of cached Sites. Exists for tests. +func (snc *SiteNameCache) Len() int { + snc.mutex.RLock() + defer snc.mutex.RUnlock() + return len(snc.names) +} + +// NewSiteNameCache returns a cache ready for concurrent use +func NewSiteNameCache() *SiteNameCache { + return &SiteNameCache{names: map[uuid.UUID]string{}} +} diff --git a/rest-api/workflow/pkg/activity/instance/instance.go b/rest-api/workflow/pkg/activity/instance/instance.go index c3e633de09..854c46cb20 100644 --- a/rest-api/workflow/pkg/activity/instance/instance.go +++ b/rest-api/workflow/pkg/activity/instance/instance.go @@ -1422,7 +1422,7 @@ func NewManageInstance(dbSession *cdb.Session, siteClientPool *sc.ClientPool, tc type ManageInstanceLifecycleMetrics struct { dbSession *cdb.Session statusTransitionTime *prometheus.GaugeVec - siteIDNameMap map[uuid.UUID]string + siteNames *cwm.SiteNameCache } // RecordInstanceStatusTransitionMetrics is a Temporal activity that records duration of important status transitions for Instances @@ -1431,16 +1431,10 @@ func (milm ManageInstanceLifecycleMetrics) RecordInstanceStatusTransitionMetrics logger.Info().Msg("starting activity") - siteName, ok := milm.siteIDNameMap[siteID] - if !ok { - siteDAO := cdbm.NewSiteDAO(milm.dbSession) - site, err := siteDAO.GetByID(context.Background(), nil, siteID, nil, false) - if err != nil { - logger.Error().Err(err).Str("Site ID", siteID.String()).Msg("failed to retrieve Site from DB") - return err - } - siteName = site.Name - milm.siteIDNameMap[siteID] = siteName + siteName, err := milm.siteNames.Get(ctx, milm.dbSession, siteID) + if err != nil { + logger.Error().Err(err).Str("Site ID", siteID.String()).Msg("failed to retrieve Site from DB") + return err } logger.Info().Int("EventCount", len(instanceLifecycleEvents)).Str("Site Name", siteName).Msg("processing instance lifecycle events") @@ -1481,7 +1475,7 @@ func (milm ManageInstanceLifecycleMetrics) RecordInstanceStatusTransitionMetrics // Only emit metric if we have exactly 1 Ready and at least 1 Pending if readySD != nil && pendingSD != nil && readyStatusCount == 1 { dur := readySD.Created.Sub(pendingSD.Created) - milm.statusTransitionTime.WithLabelValues(siteName, cwm.InventoryOperationTypeCreate, cdbm.InstanceStatusPending, cdbm.InstanceStatusReady).Set(dur.Seconds()) + milm.statusTransitionTime.WithLabelValues(siteName, siteID.String(), cwm.InventoryOperationTypeCreate, cdbm.InstanceStatusPending, cdbm.InstanceStatusReady).Set(dur.Seconds()) metricsRecorded++ logger.Info(). Str("Instance ID", event.ObjectID.String()). @@ -1507,7 +1501,7 @@ func (milm ManageInstanceLifecycleMetrics) RecordInstanceStatusTransitionMetrics if terminatingSD != nil { // Calculate duration from Terminating status to deletion time dur := event.Deleted.Sub(terminatingSD.Created) - milm.statusTransitionTime.WithLabelValues(siteName, cwm.InventoryOperationTypeDelete, cdbm.InstanceStatusTerminating, cdbm.InstanceStatusTerminated).Set(dur.Seconds()) + milm.statusTransitionTime.WithLabelValues(siteName, siteID.String(), cwm.InventoryOperationTypeDelete, cdbm.InstanceStatusTerminating, cdbm.InstanceStatusTerminated).Set(dur.Seconds()) metricsRecorded++ logger.Info(). Str("Instance ID", event.ObjectID.String()). @@ -1527,17 +1521,17 @@ func (milm ManageInstanceLifecycleMetrics) RecordInstanceStatusTransitionMetrics } // NewManageInstanceLifecycleMetrics returns a new ManageInstanceLifecycleMetrics activity -func NewManageInstanceLifecycleMetrics(reg prometheus.Registerer, dbSession *cdb.Session) ManageInstanceLifecycleMetrics { +func NewManageInstanceLifecycleMetrics(reg prometheus.Registerer, dbSession *cdb.Session, namespace string) ManageInstanceLifecycleMetrics { inventoryMetrics := ManageInstanceLifecycleMetrics{ dbSession: dbSession, statusTransitionTime: prometheus.NewGaugeVec( prometheus.GaugeOpts{ - Namespace: cwm.MetricsNamespace, + Namespace: namespace, Name: "instance_operation_latency_seconds", Help: "Current latency of instance operations", }, - []string{"site", "operation_type", "from_status", "to_status"}), - siteIDNameMap: map[uuid.UUID]string{}, + []string{"site", "site_id", "operation_type", "from_status", "to_status"}), + siteNames: cwm.NewSiteNameCache(), } reg.MustRegister(inventoryMetrics.statusTransitionTime) return inventoryMetrics diff --git a/rest-api/workflow/pkg/activity/instance/instance_test.go b/rest-api/workflow/pkg/activity/instance/instance_test.go index 34da9823a3..7a78af8d6f 100644 --- a/rest-api/workflow/pkg/activity/instance/instance_test.go +++ b/rest-api/workflow/pkg/activity/instance/instance_test.go @@ -2854,7 +2854,7 @@ func Test_InstanceMetrics_Create_PendingToReady(t *testing.T) { site := util.TestSetupSite(t, dbSession) reg := prometheus.NewRegistry() - lifecycleMetrics := NewManageInstanceLifecycleMetrics(reg, dbSession) + lifecycleMetrics := NewManageInstanceLifecycleMetrics(reg, dbSession, "nico_rest_workflow") testInstanceID := uuid.New() // Set precise timestamps @@ -2877,7 +2877,7 @@ func Test_InstanceMetrics_Create_PendingToReady(t *testing.T) { assert.NoError(t, err) // Verify metric was emitted with correct duration - util.TestAssertMetricExistsTimes(t, reg, "cloud_workflow_instance_operation_latency_seconds", 1, map[string]string{ + util.TestAssertMetricExistsTimes(t, reg, "nico_rest_workflow_instance_operation_latency_seconds", 1, map[string]string{ "operation_type": "create", "from_status": cdbm.InstanceStatusPending, "to_status": cdbm.InstanceStatusReady, @@ -2892,7 +2892,7 @@ func Test_InstanceMetrics_Create_PendingErrorReady(t *testing.T) { site := util.TestSetupSite(t, dbSession) reg := prometheus.NewRegistry() - lifecycleMetrics := NewManageInstanceLifecycleMetrics(reg, dbSession) + lifecycleMetrics := NewManageInstanceLifecycleMetrics(reg, dbSession, "nico_rest_workflow") testInstanceID := uuid.New() // Set precise timestamps @@ -2919,7 +2919,7 @@ func Test_InstanceMetrics_Create_PendingErrorReady(t *testing.T) { assert.NoError(t, err) // Verify metric was emitted with correct duration - util.TestAssertMetricExistsTimes(t, reg, "cloud_workflow_instance_operation_latency_seconds", 1, map[string]string{ + util.TestAssertMetricExistsTimes(t, reg, "nico_rest_workflow_instance_operation_latency_seconds", 1, map[string]string{ "operation_type": "create", "from_status": cdbm.InstanceStatusPending, "to_status": cdbm.InstanceStatusReady, @@ -2934,7 +2934,7 @@ func Test_InstanceMetrics_Create_ReadyErrorReady(t *testing.T) { site := util.TestSetupSite(t, dbSession) reg := prometheus.NewRegistry() - lifecycleMetrics := NewManageInstanceLifecycleMetrics(reg, dbSession) + lifecycleMetrics := NewManageInstanceLifecycleMetrics(reg, dbSession, "nico_rest_workflow") testInstanceID := uuid.New() // Set precise timestamps @@ -2964,7 +2964,7 @@ func Test_InstanceMetrics_Create_ReadyErrorReady(t *testing.T) { assert.NoError(t, err) // Verify NO metric was emitted (duplicate ready status) - util.TestAssertMetricExistsTimes(t, reg, "cloud_workflow_instance_operation_latency_seconds", 0, nil, 0) + util.TestAssertMetricExistsTimes(t, reg, "nico_rest_workflow_instance_operation_latency_seconds", 0, nil, 0) } // Test Instance Metrics - DELETE operations @@ -2976,7 +2976,7 @@ func Test_InstanceMetrics_Delete_TerminatingOnly(t *testing.T) { site := util.TestSetupSite(t, dbSession) reg := prometheus.NewRegistry() - lifecycleMetrics := NewManageInstanceLifecycleMetrics(reg, dbSession) + lifecycleMetrics := NewManageInstanceLifecycleMetrics(reg, dbSession, "nico_rest_workflow") testInstanceID := uuid.New() // Set precise timestamps @@ -2996,7 +2996,7 @@ func Test_InstanceMetrics_Delete_TerminatingOnly(t *testing.T) { assert.NoError(t, err) // Verify metric was emitted with correct duration - util.TestAssertMetricExistsTimes(t, reg, "cloud_workflow_instance_operation_latency_seconds", 1, map[string]string{ + util.TestAssertMetricExistsTimes(t, reg, "nico_rest_workflow_instance_operation_latency_seconds", 1, map[string]string{ "operation_type": "delete", "from_status": cdbm.InstanceStatusTerminating, "to_status": cdbm.InstanceStatusTerminated, @@ -3011,7 +3011,7 @@ func Test_InstanceMetrics_Delete_MultipleTerminating(t *testing.T) { site := util.TestSetupSite(t, dbSession) reg := prometheus.NewRegistry() - lifecycleMetrics := NewManageInstanceLifecycleMetrics(reg, dbSession) + lifecycleMetrics := NewManageInstanceLifecycleMetrics(reg, dbSession, "nico_rest_workflow") testInstanceID := uuid.New() // Set precise timestamps @@ -3039,7 +3039,7 @@ func Test_InstanceMetrics_Delete_MultipleTerminating(t *testing.T) { assert.NoError(t, err) // Verify metric was emitted (should use first terminating timestamp, duration 300ms) - util.TestAssertMetricExistsTimes(t, reg, "cloud_workflow_instance_operation_latency_seconds", 1, map[string]string{ + util.TestAssertMetricExistsTimes(t, reg, "nico_rest_workflow_instance_operation_latency_seconds", 1, map[string]string{ "operation_type": "delete", "from_status": cdbm.InstanceStatusTerminating, "to_status": cdbm.InstanceStatusTerminated, @@ -3054,7 +3054,7 @@ func Test_InstanceMetrics_Delete_NoTerminating(t *testing.T) { site := util.TestSetupSite(t, dbSession) reg := prometheus.NewRegistry() - lifecycleMetrics := NewManageInstanceLifecycleMetrics(reg, dbSession) + lifecycleMetrics := NewManageInstanceLifecycleMetrics(reg, dbSession, "nico_rest_workflow") testInstanceID := uuid.New() // Set precise timestamps @@ -3073,5 +3073,5 @@ func Test_InstanceMetrics_Delete_NoTerminating(t *testing.T) { assert.NoError(t, err) // Verify NO metric was emitted (no terminating status found) - util.TestAssertMetricExistsTimes(t, reg, "cloud_workflow_instance_operation_latency_seconds", 0, nil, 0) + util.TestAssertMetricExistsTimes(t, reg, "nico_rest_workflow_instance_operation_latency_seconds", 0, nil, 0) } diff --git a/rest-api/workflow/pkg/activity/site/site.go b/rest-api/workflow/pkg/activity/site/site.go index f8fce7f5a5..5f5aacbcb2 100644 --- a/rest-api/workflow/pkg/activity/site/site.go +++ b/rest-api/workflow/pkg/activity/site/site.go @@ -30,6 +30,7 @@ import ( csm "github.com/NVIDIA/infra-controller/rest-api/site-manager/pkg/sitemgr" "github.com/NVIDIA/infra-controller/rest-api/workflow/internal/config" + cwm "github.com/NVIDIA/infra-controller/rest-api/workflow/internal/metrics" sc "github.com/NVIDIA/infra-controller/rest-api/workflow/pkg/client/site" "github.com/NVIDIA/infra-controller/rest-api/workflow/pkg/queue" "github.com/NVIDIA/infra-controller/rest-api/workflow/pkg/util" @@ -67,10 +68,11 @@ func getSiteFabricIPBlockLockID(dbSite *cdbm.Site) uint64 { // ManageSite is an activity wrapper for managing Site lifecycle that allows // injecting DB access type ManageSite struct { - dbSession *cdb.Session - siteClientPool *sc.ClientPool - tc client.Client - cfg *config.Config + dbSession *cdb.Session + siteClientPool *sc.ClientPool + tc client.Client + cfg *config.Config + siteHealthMetrics *cwm.SiteHealthMetrics } // Activity functions @@ -613,14 +615,44 @@ func (mst ManageSite) MonitorInventoryReceiptForAllSites(ctx context.Context) er // Get all Sites siteDAO := cdbm.NewSiteDAO(mst.dbSession) - sites, _, err := siteDAO.GetAll(ctx, nil, cdbm.SiteFilterInput{Statuses: []string{string(cdbm.SiteStatusRegistered)}}, cdbp.PageInput{Limit: ccu.GetPtr(cdbp.TotalLimit)}, nil) + // Error Sites are included alongside Registered ones because the check below + // moves a disconnected Site to Error. Querying Registered alone would drop it + // from the gauges on the very next cycle, resolving the alert three minutes + // into an outage that is still going. + sites, _, err := siteDAO.GetAll( + ctx, + nil, + cdbm.SiteFilterInput{Statuses: []string{cdbm.SiteStatusRegistered, cdbm.SiteStatusError}}, + cdbp.PageInput{Limit: ccu.GetPtr(cdbp.TotalLimit)}, + nil, + ) if err != nil { logger.Error().Err(err).Msg("failed to retrieve Sites from DB") return err } + // Publish health before the checks below, so the gauges reflect every + // monitored Site even when a later status update fails. + reports := make([]cwm.SiteHealthReport, 0, len(sites)) + for _, site := range sites { + reports = append(reports, cwm.SiteHealthReport{ + SiteID: site.ID, + SiteName: site.Name, + InventoryReceived: site.InventoryReceived, + AgentCertExpiry: site.AgentCertExpiry, + }) + } + mst.siteHealthMetrics.SetSiteHealth(reports) + // Loop through Sites for _, site := range sites { + // Only a Registered Site can trip into Error. An Error Site is already + // reported, so re-running this would repeat the Slack message and add a + // StatusDetail row on every cycle for as long as the outage lasts. + if site.Status != cdbm.SiteStatusRegistered { + continue + } + // Get Site's last inventory receipt if site.InventoryReceived == nil { logger.Warn().Str("Site ID", site.ID.String()).Msg("Site has Registered status but hasn't received inventory yet") @@ -643,29 +675,6 @@ func (mst ManageSite) MonitorInventoryReceiptForAllSites(ctx context.Context) er } } - if mst.cfg.GetNotificationsPagerDutyEnabled() { - // Send PagerDuty notification - pc := util.NewPagerDutyClient(mst.cfg.GetNotificationsPagerDutyIntegrationKey()) - customDetails := map[string]string{ - "site_id": site.ID.String(), - "site_name": site.Name, - "threshold_minutes": fmt.Sprintf("%.0f", SiteInventoryReceiptThreshold.Minutes()), - "last_inventory_time": site.InventoryReceived.Format(time.RFC3339), - "time_since_last": time.Since(*site.InventoryReceived).String(), - "description": fmt.Sprintf("Site hasn't received Machine inventory for longer than threshold period of: %v minutes", SiteInventoryReceiptThreshold.Minutes()), - } - err := pc.SendPagerDutyAlertWithDedupeKey( - ctx, - fmt.Sprintf("Site Disconnection Detected: %s", site.Name), - "cloud-workflow-monitor", - fmt.Sprintf("site-disconnection-%s", site.ID.String()), - customDetails, - ) - if err != nil { - logger.Error().Err(err).Msg("failed to send PagerDuty notification for Site down event") - } - } - // Set Site status to error errMsg := fmt.Sprintf("Site hasn't received inventory for longer than threshold period of: %v minutes", SiteInventoryReceiptThreshold.Minutes()) serr := mst.updateSiteStatusInDB(ctx, nil, site.ID, ccu.GetPtr(cdbm.SiteStatusError), &errMsg) @@ -1076,11 +1085,12 @@ func (mst ManageSite) UpdateIPBlocksInDBFromFabricPrefixes(ctx context.Context, } // NewManageSite returns a new ManageSite activity -func NewManageSite(dbSession *cdb.Session, siteClientPool *sc.ClientPool, tc client.Client, cfg *config.Config) ManageSite { +func NewManageSite(dbSession *cdb.Session, siteClientPool *sc.ClientPool, tc client.Client, cfg *config.Config, siteHealthMetrics *cwm.SiteHealthMetrics) ManageSite { return ManageSite{ - dbSession: dbSession, - siteClientPool: siteClientPool, - tc: tc, - cfg: cfg, + dbSession: dbSession, + siteClientPool: siteClientPool, + tc: tc, + cfg: cfg, + siteHealthMetrics: siteHealthMetrics, } } diff --git a/rest-api/workflow/pkg/activity/site/site_test.go b/rest-api/workflow/pkg/activity/site/site_test.go index 55fefcb781..c1b29770dd 100644 --- a/rest-api/workflow/pkg/activity/site/site_test.go +++ b/rest-api/workflow/pkg/activity/site/site_test.go @@ -11,7 +11,6 @@ import ( "net/http" "net/http/httptest" "os" - "reflect" "testing" "time" @@ -25,10 +24,12 @@ import ( cipam "github.com/NVIDIA/infra-controller/rest-api/ipam" corev1 "github.com/NVIDIA/infra-controller/rest-api/proto/core/gen/v1" "github.com/NVIDIA/infra-controller/rest-api/workflow/internal/config" + cwm "github.com/NVIDIA/infra-controller/rest-api/workflow/internal/metrics" sc "github.com/NVIDIA/infra-controller/rest-api/workflow/pkg/client/site" "github.com/NVIDIA/infra-controller/rest-api/workflow/pkg/util" "github.com/golang/mock/gomock" "github.com/google/uuid" + "github.com/prometheus/client_golang/prometheus" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" "github.com/uptrace/bun/extra/bundebug" @@ -369,10 +370,11 @@ func TestManageSite_DeleteSiteComponentsFromDB(t *testing.T) { func TestNewManageSite(t *testing.T) { type args struct { - dbSession *cdb.Session - siteClientPool *sc.ClientPool - tc client.Client - cfg *config.Config + dbSession *cdb.Session + siteClientPool *sc.ClientPool + tc client.Client + cfg *config.Config + siteHealthMetrics *cwm.SiteHealthMetrics } dbSession := &cdb.Session{} @@ -390,6 +392,7 @@ func TestNewManageSite(t *testing.T) { tc := &tmocks.Client{} scp := sc.NewClientPool(tcfg) + shm := cwm.NewSiteHealthMetrics(prometheus.NewRegistry(), "nico_rest_workflow") tests := []struct { name string @@ -399,24 +402,27 @@ func TestNewManageSite(t *testing.T) { { name: "test new ManageSite instantiation", args: args{ - dbSession: dbSession, - siteClientPool: scp, - tc: tc, - cfg: cfg, + dbSession: dbSession, + siteClientPool: scp, + tc: tc, + cfg: cfg, + siteHealthMetrics: shm, }, want: ManageSite{ - dbSession: dbSession, - siteClientPool: scp, - tc: tc, - cfg: cfg, + dbSession: dbSession, + siteClientPool: scp, + tc: tc, + cfg: cfg, + siteHealthMetrics: shm, }, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - if got := NewManageSite(tt.args.dbSession, tt.args.siteClientPool, tc, cfg); !reflect.DeepEqual(got, tt.want) { - t.Errorf("NewManageSite() = %v, want %v", got, tt.want) - } + assert.Equal(t, tt.want, NewManageSite( + tt.args.dbSession, tt.args.siteClientPool, tt.args.tc, + tt.args.cfg, tt.args.siteHealthMetrics, + )) }) } } @@ -439,6 +445,16 @@ func TestManageSite_MonitorInventoryReceiptForAllSites(t *testing.T) { site2 := util.TestBuildSite(t, dbSession, ip, "test-site-2", cdbm.SiteStatusRegistered, cutil.GetPtr(time.Now().Add(-1*time.Hour)), ipu) site3 := util.TestBuildSite(t, dbSession, ip, "test-site-3", cdbm.SiteStatusRegistered, cutil.GetPtr(time.Now()), ipu) site4 := util.TestBuildSite(t, dbSession, ip, "test-site-4", cdbm.SiteStatusRegistered, cutil.GetPtr(time.Now().Add(-1*time.Hour)), ipu) + site5 := util.TestBuildSite(t, dbSession, ip, "test-site-5", cdbm.SiteStatusRegistered, nil, ipu) + + // Only site3 has ever reported a cert expiry, so the rest exercise the + // never-reported case the gauge publishes as 0. + site3CertExpiry := time.Now().Add(30 * 24 * time.Hour) + _, err := cdbm.NewSiteDAO(dbSession).Update(ctx, nil, cdbm.SiteUpdateInput{ + SiteID: site3.ID, + AgentCertExpiry: &site3CertExpiry, + }) + assert.NoError(t, err) tSiteClientPool := testTemporalSiteClientPool(t) assert.NotNil(t, tSiteClientPool) @@ -457,6 +473,11 @@ func TestManageSite_MonitorInventoryReceiptForAllSites(t *testing.T) { cfg2 := config.NewConfig() cfg2.SetNotificationsSlackWebhookURL("") + // One registry across every case, so a later run sees what the earlier ones + // published and can prove a Site keeps or loses its series. + reg := prometheus.NewRegistry() + siteHealthMetrics := cwm.NewSiteHealthMetrics(reg, "nico_rest_workflow") + type fields struct { dbSession *cdb.Session siteClientPool *sc.ClientPool @@ -466,10 +487,13 @@ func TestManageSite_MonitorInventoryReceiptForAllSites(t *testing.T) { ctx context.Context } tests := []struct { - name string - fields fields - args args - wantStatus map[uuid.UUID]string + name string + fields fields + args args + setup func(t *testing.T) + wantStatus map[uuid.UUID]string + wantGauge map[string]float64 + wantCertGauge map[string]float64 }{ { name: "test monitor inventory receipt for all sites with Slack notification", @@ -486,6 +510,20 @@ func TestManageSite_MonitorInventoryReceiptForAllSites(t *testing.T) { site2.ID: cdbm.SiteStatusError, site3.ID: cdbm.SiteStatusRegistered, }, + // site1 is Pending so it is not published at all, and site5 has never + // reported, so it publishes 0 rather than going missing. + wantGauge: map[string]float64{ + site2.Name: float64(site2.InventoryReceived.Unix()), + site3.Name: float64(site3.InventoryReceived.Unix()), + site4.Name: float64(site4.InventoryReceived.Unix()), + site5.Name: 0, + }, + wantCertGauge: map[string]float64{ + site2.Name: 0, + site3.Name: float64(site3CertExpiry.Unix()), + site4.Name: 0, + site5.Name: 0, + }, }, { name: "test monitor inventory receipt for all sites without Slack notification", @@ -500,14 +538,61 @@ func TestManageSite_MonitorInventoryReceiptForAllSites(t *testing.T) { wantStatus: map[uuid.UUID]string{ site4.ID: cdbm.SiteStatusError, }, + // site2 and site4 went to Error in the case above and are still + // disconnected, so they have to keep reporting. Dropping them here + // would resolve the alert while the outage continues. + wantGauge: map[string]float64{ + site2.Name: float64(site2.InventoryReceived.Unix()), + site3.Name: float64(site3.InventoryReceived.Unix()), + site4.Name: float64(site4.InventoryReceived.Unix()), + site5.Name: 0, + }, + wantCertGauge: map[string]float64{ + site2.Name: 0, + site3.Name: float64(site3CertExpiry.Unix()), + site4.Name: 0, + site5.Name: 0, + }, + }, + { + name: "test monitor inventory receipt drops a deleted Site", + fields: fields{ + dbSession: dbSession, + siteClientPool: tSiteClientPool, + cfg: cfg2, + }, + args: args{ + ctx: ctx, + }, + setup: func(t *testing.T) { + derr := cdbm.NewSiteDAO(dbSession).Delete(ctx, nil, site4.ID) + assert.NoError(t, derr) + }, + // A Site that no longer exists is the one case the rebuild has to + // clear, otherwise it ages into an alert nothing can resolve. + wantGauge: map[string]float64{ + site2.Name: float64(site2.InventoryReceived.Unix()), + site3.Name: float64(site3.InventoryReceived.Unix()), + site5.Name: 0, + }, + wantCertGauge: map[string]float64{ + site2.Name: 0, + site3.Name: float64(site3CertExpiry.Unix()), + site5.Name: 0, + }, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + if tt.setup != nil { + tt.setup(t) + } + mst := ManageSite{ - dbSession: tt.fields.dbSession, - siteClientPool: tt.fields.siteClientPool, - cfg: tt.fields.cfg, + dbSession: tt.fields.dbSession, + siteClientPool: tt.fields.siteClientPool, + cfg: tt.fields.cfg, + siteHealthMetrics: siteHealthMetrics, } err := mst.MonitorInventoryReceiptForAllSites(tt.args.ctx) assert.NoError(t, err) @@ -518,138 +603,33 @@ func TestManageSite_MonitorInventoryReceiptForAllSites(t *testing.T) { assert.NoError(t, err) assert.Equal(t, wantStatus, site.Status) } - }) - } -} -func TestManageSite_MonitorInventoryReceiptForAllSites_PagerDutyEnabled(t *testing.T) { - ctx := context.Background() - - dbSession := testSiteInitDB(t) - defer dbSession.Close() - - util.TestSetupSchema(t, dbSession) - - ipOrg := "test-provider-org-1" - ipRoles := []string{"FORGE_PROVIDER_ADMIN"} - - ipu := util.TestBuildUser(t, dbSession, uuid.New().String(), []string{ipOrg}, ipRoles) - ip := util.TestBuildInfrastructureProvider(t, dbSession, "testIP", ipOrg, ipu) - - // Create sites with expired inventory receipt times - site1 := util.TestBuildSite(t, dbSession, ip, "pagerduty-test-site-1", cdbm.SiteStatusRegistered, cutil.GetPtr(time.Now().Add(-2*time.Hour)), ipu) - site2 := util.TestBuildSite(t, dbSession, ip, "pagerduty-test-site-2", cdbm.SiteStatusRegistered, cutil.GetPtr(time.Now().Add(-30*time.Minute)), ipu) - - tSiteClientPool := testTemporalSiteClientPool(t) - assert.NotNil(t, tSiteClientPool) - - temporalsuit := testsuite.WorkflowTestSuite{} - temporalsuit.NewTestWorkflowEnvironment() - - // Create a mock PagerDuty server - pdEventCount := 0 - testPagerDutyServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - // Validate it's a POST request to the right path - assert.Equal(t, http.MethodPost, r.Method) - assert.Equal(t, "/v2/enqueue", r.URL.Path) - - pdEventCount++ - - w.Header().Set("Content-Type", "application/json") - w.WriteHeader(http.StatusAccepted) - w.Write([]byte(`{"status":"success","message":"Event processed","dedup_key":"test-dedup-key"}`)) - })) - defer testPagerDutyServer.Close() - - // Override the default http.Client to redirect PagerDuty requests to our test server - originalTransport := http.DefaultTransport - http.DefaultTransport = &mockPagerDutyTransport{ - testServerURL: testPagerDutyServer.URL, - original: originalTransport, - } - defer func() { - http.DefaultTransport = originalTransport - }() - - // Configure PagerDuty - cfg := config.NewConfig() - cfg.SetNotificationsPagerDutyIntegrationKey("test-integration-key") - - mst := ManageSite{ - dbSession: dbSession, - siteClientPool: tSiteClientPool, - cfg: cfg, + assert.Equal(t, tt.wantGauge, testSiteGauge(t, reg, "nico_rest_workflow_site_last_inventory_receipt_timestamp_seconds")) + assert.Equal(t, tt.wantCertGauge, testSiteGauge(t, reg, "nico_rest_workflow_site_agent_cert_expiry_timestamp_seconds")) + }) } - - err := mst.MonitorInventoryReceiptForAllSites(ctx) - assert.NoError(t, err) - - // Verify site statuses were updated correctly - siteDAO := cdbm.NewSiteDAO(dbSession) - site1Result, err := siteDAO.GetByID(ctx, nil, site1.ID, nil, false) - assert.NoError(t, err) - assert.Equal(t, cdbm.SiteStatusError, site1Result.Status) - - site2Result, err := siteDAO.GetByID(ctx, nil, site2.ID, nil, false) - assert.NoError(t, err) - assert.Equal(t, cdbm.SiteStatusError, site2Result.Status) - - // Assert on PagerDuty events received (both sites should trigger alerts) - assert.Equal(t, 2, pdEventCount, "Expected 2 PagerDuty events but got %d", pdEventCount) } -func TestManageSite_MonitorInventoryReceiptForAllSites_PagerDutyDisabled(t *testing.T) { - ctx := context.Background() - - dbSession := testSiteInitDB(t) - defer dbSession.Close() - - util.TestSetupSchema(t, dbSession) - - ipOrg := "test-provider-org-1" - ipRoles := []string{"FORGE_PROVIDER_ADMIN"} - - ipu := util.TestBuildUser(t, dbSession, uuid.New().String(), []string{ipOrg}, ipRoles) - ip := util.TestBuildInfrastructureProvider(t, dbSession, "testIP", ipOrg, ipu) - - // Create sites with expired inventory receipt times - _ = util.TestBuildSite(t, dbSession, ip, "pagerduty-test-site-3", cdbm.SiteStatusRegistered, cutil.GetPtr(time.Now().Add(-2*time.Hour)), ipu) - _ = util.TestBuildSite(t, dbSession, ip, "pagerduty-test-site-4", cdbm.SiteStatusRegistered, cutil.GetPtr(time.Now().Add(-30*time.Minute)), ipu) - - tSiteClientPool := testTemporalSiteClientPool(t) - assert.NotNil(t, tSiteClientPool) - - temporalsuit := testsuite.WorkflowTestSuite{} - temporalsuit.NewTestWorkflowEnvironment() - - cfg := config.NewConfig() +// testSiteGauge reads a per-Site gauge back as Site name to published value. +func testSiteGauge(t *testing.T, reg *prometheus.Registry, name string) map[string]float64 { + families, err := reg.Gather() + require.NoError(t, err) - mst := ManageSite{ - dbSession: dbSession, - siteClientPool: tSiteClientPool, - cfg: cfg, + values := map[string]float64{} + for _, family := range families { + if family.GetName() != name { + continue + } + for _, metric := range family.GetMetric() { + for _, label := range metric.GetLabel() { + if label.GetName() == "site" { + values[label.GetValue()] = metric.GetGauge().GetValue() + } + } + } } - err := mst.MonitorInventoryReceiptForAllSites(ctx) - assert.NoError(t, err) -} - -// mockPagerDutyTransport intercepts requests to PagerDuty and redirects them to a test server -type mockPagerDutyTransport struct { - testServerURL string - original http.RoundTripper -} - -func (m *mockPagerDutyTransport) RoundTrip(req *http.Request) (*http.Response, error) { - // Intercept requests to PagerDuty's API - if req.URL.Host == "events.pagerduty.com" { - // Redirect to our test server - req.URL.Scheme = "http" - req.URL.Host = m.testServerURL[7:] // Remove "http://" prefix - return m.original.RoundTrip(req) - } - // Pass through all other requests - return m.original.RoundTrip(req) + return values } // MockTemporalClient is a mock for Temporal Client @@ -1242,7 +1222,7 @@ func TestManageSite_DeleteSiteComponentsFromDB_NewResources(t *testing.T) { func TestManageSite_UpdateSiteInDB(t *testing.T) { ctx := context.Background() resources := setupSiteFabricIPBlockTest(t) - mst := NewManageSite(resources.dbSession, nil, nil, nil) + mst := NewManageSite(resources.dbSession, nil, nil, nil, nil) siteDAO := cdbm.NewSiteDAO(resources.dbSession) // The stored Site Agent version every case starts from, so a case that expects it untouched @@ -1596,7 +1576,7 @@ func setupSiteFabricIPBlockTest(t *testing.T) siteFabricIPBlockTestResources { func TestManageSite_UpdateIPBlocksInDBFromFabricPrefixes_CreatesMissingBlocks(t *testing.T) { ctx := context.Background() resources := setupSiteFabricIPBlockTest(t) - mst := NewManageSite(resources.dbSession, nil, nil, nil) + mst := NewManageSite(resources.dbSession, nil, nil, nil, nil) err := mst.UpdateIPBlocksInDBFromFabricPrefixes(ctx, resources.site.ID, []string{ "10.0.1.12/16", @@ -1638,7 +1618,7 @@ func TestManageSite_UpdateIPBlocksInDBFromFabricPrefixes_CreatesMissingBlocks(t func TestManageSite_UpdateIPBlocksInDBFromFabricPrefixes_IsIdempotent(t *testing.T) { ctx := context.Background() resources := setupSiteFabricIPBlockTest(t) - mst := NewManageSite(resources.dbSession, nil, nil, nil) + mst := NewManageSite(resources.dbSession, nil, nil, nil, nil) prefixes := []string{"10.42.0.0/16", "2001:db8:42::/64"} require.NoError(t, mst.UpdateIPBlocksInDBFromFabricPrefixes(ctx, resources.site.ID, prefixes)) @@ -1658,7 +1638,7 @@ func TestManageSite_UpdateIPBlocksInDBFromFabricPrefixes_IsIdempotent(t *testing func TestManageSite_UpdateIPBlocksInDBFromFabricPrefixes_LeavesExistingManualBlock(t *testing.T) { ctx := context.Background() resources := setupSiteFabricIPBlockTest(t) - mst := NewManageSite(resources.dbSession, nil, nil, nil) + mst := NewManageSite(resources.dbSession, nil, nil, nil, nil) existing := util.TestBuildBuildIPBlock( t, @@ -1687,7 +1667,7 @@ func TestManageSite_UpdateIPBlocksInDBFromFabricPrefixes_LeavesExistingManualBlo func TestManageSite_UpdateIPBlocksInDBFromFabricPrefixes_CreatesDatacenterOnlyBlockWhenOtherRoutingTypeExists(t *testing.T) { ctx := context.Background() resources := setupSiteFabricIPBlockTest(t) - mst := NewManageSite(resources.dbSession, nil, nil, nil) + mst := NewManageSite(resources.dbSession, nil, nil, nil, nil) existing := util.TestBuildBuildIPBlock( t, @@ -1729,7 +1709,7 @@ func TestManageSite_UpdateIPBlocksInDBFromFabricPrefixes_CreatesDatacenterOnlyBl func TestManageSite_UpdateIPBlocksInDBFromFabricPrefixes_ReturnsErrorWhenFabricBlockLockHeld(t *testing.T) { ctx := context.Background() resources := setupSiteFabricIPBlockTest(t) - mst := NewManageSite(resources.dbSession, nil, nil, nil) + mst := NewManageSite(resources.dbSession, nil, nil, nil, nil) err := cdb.WithTx(ctx, resources.dbSession, func(tx *cdb.Tx) error { require.NoError(t, tx.AcquireAdvisoryLock(ctx, getSiteFabricIPBlockLockID(resources.site), false)) @@ -1748,7 +1728,7 @@ func TestManageSite_UpdateIPBlocksInDBFromFabricPrefixes_ReturnsErrorWhenFabricB func TestManageSite_UpdateIPBlocksInDBFromFabricPrefixes_InvalidPrefixDoesNotCreateBlocks(t *testing.T) { ctx := context.Background() resources := setupSiteFabricIPBlockTest(t) - mst := NewManageSite(resources.dbSession, nil, nil, nil) + mst := NewManageSite(resources.dbSession, nil, nil, nil, nil) err := mst.UpdateIPBlocksInDBFromFabricPrefixes(ctx, resources.site.ID, []string{"not-a-cidr"}) require.Error(t, err) @@ -1760,7 +1740,7 @@ func TestManageSite_UpdateIPBlocksInDBFromFabricPrefixes_InvalidPrefixDoesNotCre func TestManageSite_UpdateIPBlocksInDBFromFabricPrefixes_NoPrefixesIsNoOp(t *testing.T) { ctx := context.Background() resources := setupSiteFabricIPBlockTest(t) - mst := NewManageSite(resources.dbSession, nil, nil, nil) + mst := NewManageSite(resources.dbSession, nil, nil, nil, nil) require.NoError(t, mst.UpdateIPBlocksInDBFromFabricPrefixes(ctx, resources.site.ID, nil)) @@ -1771,7 +1751,7 @@ func TestManageSite_UpdateIPBlocksInDBFromFabricPrefixes_NoPrefixesIsNoOp(t *tes func TestManageSite_UpdateIPBlocksInDBFromFabricPrefixes_UnknownSiteReturnsError(t *testing.T) { ctx := context.Background() resources := setupSiteFabricIPBlockTest(t) - mst := NewManageSite(resources.dbSession, nil, nil, nil) + mst := NewManageSite(resources.dbSession, nil, nil, nil, nil) err := mst.UpdateIPBlocksInDBFromFabricPrefixes(ctx, uuid.New(), []string{"10.0.0.0/16"}) require.ErrorIs(t, err, cdb.ErrDoesNotExist) diff --git a/rest-api/workflow/pkg/activity/subnet/subnet.go b/rest-api/workflow/pkg/activity/subnet/subnet.go index 6a74cdaf7e..4a414e7b79 100644 --- a/rest-api/workflow/pkg/activity/subnet/subnet.go +++ b/rest-api/workflow/pkg/activity/subnet/subnet.go @@ -401,7 +401,7 @@ func NewManageSubnet(dbSession *cdb.Session, siteClientPool *sc.ClientPool, tc c type ManageSubnetLifecycleMetrics struct { dbSession *cdb.Session statusTransitionTime *prometheus.GaugeVec - siteIDNameMap map[uuid.UUID]string + siteNames *cwm.SiteNameCache } // RecordSubnetStatusTransitionMetrics is a Temporal activity that records duration of important status transitions for Subnets @@ -410,17 +410,10 @@ func (mslm ManageSubnetLifecycleMetrics) RecordSubnetStatusTransitionMetrics(ctx logger.Info().Msg("starting activity") - // Cache site name to avoid repeated DB call - siteName, ok := mslm.siteIDNameMap[siteID] - if !ok { - siteDAO := cdbm.NewSiteDAO(mslm.dbSession) - site, err := siteDAO.GetByID(context.Background(), nil, siteID, nil, false) - if err != nil { - logger.Error().Err(err).Str("Site ID", siteID.String()).Msg("failed to retrieve Site from DB") - return err - } - siteName = site.Name - mslm.siteIDNameMap[siteID] = siteName + siteName, err := mslm.siteNames.Get(ctx, mslm.dbSession, siteID) + if err != nil { + logger.Error().Err(err).Str("Site ID", siteID.String()).Msg("failed to retrieve Site from DB") + return err } logger.Info().Int("EventCount", len(subnetLifecycleEvents)).Str("Site Name", siteName).Msg("processing subnet lifecycle events") @@ -462,7 +455,7 @@ func (mslm ManageSubnetLifecycleMetrics) RecordSubnetStatusTransitionMetrics(ctx // Only emit metric if we have exactly 1 Ready and at least 1 Pending if readySD != nil && pendingSD != nil && readyStatusCount == 1 { dur := readySD.Created.Sub(pendingSD.Created) - mslm.statusTransitionTime.WithLabelValues(siteName, cwm.InventoryOperationTypeCreate, cdbm.SubnetStatusPending, cdbm.SubnetStatusReady).Set(dur.Seconds()) + mslm.statusTransitionTime.WithLabelValues(siteName, siteID.String(), cwm.InventoryOperationTypeCreate, cdbm.SubnetStatusPending, cdbm.SubnetStatusReady).Set(dur.Seconds()) metricsRecorded++ logger.Info(). Str("Subnet ID", event.ObjectID.String()). @@ -488,7 +481,7 @@ func (mslm ManageSubnetLifecycleMetrics) RecordSubnetStatusTransitionMetrics(ctx if deletingSD != nil { // Calculate duration from Deleting status to deletion time dur := event.Deleted.Sub(deletingSD.Created) - mslm.statusTransitionTime.WithLabelValues(siteName, cwm.InventoryOperationTypeDelete, cdbm.SubnetStatusDeleting, cdbm.SubnetStatusDeleted).Set(dur.Seconds()) + mslm.statusTransitionTime.WithLabelValues(siteName, siteID.String(), cwm.InventoryOperationTypeDelete, cdbm.SubnetStatusDeleting, cdbm.SubnetStatusDeleted).Set(dur.Seconds()) metricsRecorded++ logger.Info(). Str("Subnet ID", event.ObjectID.String()). @@ -508,18 +501,18 @@ func (mslm ManageSubnetLifecycleMetrics) RecordSubnetStatusTransitionMetrics(ctx } // NewManageSubnetLifecycleMetrics returns a new ManageSubnetLifecycleMetrics activity -func NewManageSubnetLifecycleMetrics(reg prometheus.Registerer, dbSession *cdb.Session) ManageSubnetLifecycleMetrics { +func NewManageSubnetLifecycleMetrics(reg prometheus.Registerer, dbSession *cdb.Session, namespace string) ManageSubnetLifecycleMetrics { lifecycleMetrics := ManageSubnetLifecycleMetrics{ dbSession: dbSession, statusTransitionTime: prometheus.NewGaugeVec( prometheus.GaugeOpts{ - Namespace: cwm.MetricsNamespace, + Namespace: namespace, Name: "subnet_operation_latency_seconds", Help: "Current latency of subnet operations", }, - []string{"site", "operation_type", "from_status", "to_status"}), + []string{"site", "site_id", "operation_type", "from_status", "to_status"}), - siteIDNameMap: map[uuid.UUID]string{}, + siteNames: cwm.NewSiteNameCache(), } reg.MustRegister(lifecycleMetrics.statusTransitionTime) diff --git a/rest-api/workflow/pkg/activity/subnet/subnet_test.go b/rest-api/workflow/pkg/activity/subnet/subnet_test.go index 54dd96e4d9..4f74040a7a 100644 --- a/rest-api/workflow/pkg/activity/subnet/subnet_test.go +++ b/rest-api/workflow/pkg/activity/subnet/subnet_test.go @@ -726,7 +726,7 @@ func Test_SubnetMetrics_Create_PendingToReady(t *testing.T) { site := util.TestSetupSite(t, dbSession) reg := prometheus.NewRegistry() - lifecycleMetrics := NewManageSubnetLifecycleMetrics(reg, dbSession) + lifecycleMetrics := NewManageSubnetLifecycleMetrics(reg, dbSession, "nico_rest_workflow") testSubnetID := uuid.New() // Set precise timestamps @@ -750,7 +750,7 @@ func Test_SubnetMetrics_Create_PendingToReady(t *testing.T) { assert.NoError(t, err) // Verify metric was emitted with correct duration (150ms) - util.TestAssertMetricExistsTimes(t, reg, "cloud_workflow_subnet_operation_latency_seconds", 1, map[string]string{ + util.TestAssertMetricExistsTimes(t, reg, "nico_rest_workflow_subnet_operation_latency_seconds", 1, map[string]string{ "operation_type": "create", "from_status": cdbm.SubnetStatusPending, "to_status": cdbm.SubnetStatusReady, @@ -765,7 +765,7 @@ func Test_SubnetMetrics_Create_PendingErrorReady(t *testing.T) { site := util.TestSetupSite(t, dbSession) reg := prometheus.NewRegistry() - lifecycleMetrics := NewManageSubnetLifecycleMetrics(reg, dbSession) + lifecycleMetrics := NewManageSubnetLifecycleMetrics(reg, dbSession, "nico_rest_workflow") testSubnetID := uuid.New() // Set precise timestamps @@ -793,7 +793,7 @@ func Test_SubnetMetrics_Create_PendingErrorReady(t *testing.T) { assert.NoError(t, err) // Verify metric was emitted with duration t3-t1 (250ms) - util.TestAssertMetricExistsTimes(t, reg, "cloud_workflow_subnet_operation_latency_seconds", 1, map[string]string{ + util.TestAssertMetricExistsTimes(t, reg, "nico_rest_workflow_subnet_operation_latency_seconds", 1, map[string]string{ "operation_type": "create", "from_status": cdbm.SubnetStatusPending, "to_status": cdbm.SubnetStatusReady, @@ -808,7 +808,7 @@ func Test_SubnetMetrics_Create_ReadyErrorReady(t *testing.T) { site := util.TestSetupSite(t, dbSession) reg := prometheus.NewRegistry() - lifecycleMetrics := NewManageSubnetLifecycleMetrics(reg, dbSession) + lifecycleMetrics := NewManageSubnetLifecycleMetrics(reg, dbSession, "nico_rest_workflow") testSubnetID := uuid.New() // Set precise timestamps @@ -835,7 +835,7 @@ func Test_SubnetMetrics_Create_ReadyErrorReady(t *testing.T) { assert.NoError(t, err) // Verify NO metric was emitted (duplicate ready, no pending->ready transition) - util.TestAssertMetricExistsTimes(t, reg, "cloud_workflow_subnet_operation_latency_seconds", 0, nil, 0) + util.TestAssertMetricExistsTimes(t, reg, "nico_rest_workflow_subnet_operation_latency_seconds", 0, nil, 0) } // Test Subnet Metrics - DELETE operations @@ -847,7 +847,7 @@ func Test_SubnetMetrics_Delete_DeletingOnly(t *testing.T) { site := util.TestSetupSite(t, dbSession) reg := prometheus.NewRegistry() - lifecycleMetrics := NewManageSubnetLifecycleMetrics(reg, dbSession) + lifecycleMetrics := NewManageSubnetLifecycleMetrics(reg, dbSession, "nico_rest_workflow") testSubnetID := uuid.New() // Set precise timestamps @@ -867,7 +867,7 @@ func Test_SubnetMetrics_Delete_DeletingOnly(t *testing.T) { assert.NoError(t, err) // Verify metric was emitted with correct duration (180ms) - util.TestAssertMetricExistsTimes(t, reg, "cloud_workflow_subnet_operation_latency_seconds", 1, map[string]string{ + util.TestAssertMetricExistsTimes(t, reg, "nico_rest_workflow_subnet_operation_latency_seconds", 1, map[string]string{ "operation_type": "delete", "from_status": cdbm.SubnetStatusDeleting, "to_status": cdbm.SubnetStatusDeleted, @@ -882,7 +882,7 @@ func Test_SubnetMetrics_Delete_MultipleDeletingTerminating(t *testing.T) { site := util.TestSetupSite(t, dbSession) reg := prometheus.NewRegistry() - lifecycleMetrics := NewManageSubnetLifecycleMetrics(reg, dbSession) + lifecycleMetrics := NewManageSubnetLifecycleMetrics(reg, dbSession, "nico_rest_workflow") testSubnetID := uuid.New() // Set precise timestamps @@ -910,7 +910,7 @@ func Test_SubnetMetrics_Delete_MultipleDeletingTerminating(t *testing.T) { assert.NoError(t, err) // Verify metric was emitted (should use first deleting timestamp, duration 350ms) - util.TestAssertMetricExistsTimes(t, reg, "cloud_workflow_subnet_operation_latency_seconds", 1, map[string]string{ + util.TestAssertMetricExistsTimes(t, reg, "nico_rest_workflow_subnet_operation_latency_seconds", 1, map[string]string{ "operation_type": "delete", "from_status": cdbm.SubnetStatusDeleting, "to_status": cdbm.SubnetStatusDeleted, @@ -925,7 +925,7 @@ func Test_SubnetMetrics_Delete_NoDeleting(t *testing.T) { site := util.TestSetupSite(t, dbSession) reg := prometheus.NewRegistry() - lifecycleMetrics := NewManageSubnetLifecycleMetrics(reg, dbSession) + lifecycleMetrics := NewManageSubnetLifecycleMetrics(reg, dbSession, "nico_rest_workflow") testSubnetID := uuid.New() // Set precise timestamps @@ -944,5 +944,5 @@ func Test_SubnetMetrics_Delete_NoDeleting(t *testing.T) { assert.NoError(t, err) // Verify NO metric was emitted (no deleting status found) - util.TestAssertMetricExistsTimes(t, reg, "cloud_workflow_subnet_operation_latency_seconds", 0, nil, 0) + util.TestAssertMetricExistsTimes(t, reg, "nico_rest_workflow_subnet_operation_latency_seconds", 0, nil, 0) } diff --git a/rest-api/workflow/pkg/activity/vpc/vpc.go b/rest-api/workflow/pkg/activity/vpc/vpc.go index a3715ba586..71efaccc20 100644 --- a/rest-api/workflow/pkg/activity/vpc/vpc.go +++ b/rest-api/workflow/pkg/activity/vpc/vpc.go @@ -704,7 +704,7 @@ func NewManageVpc(dbSession *cdb.Session, siteClientPool *sc.ClientPool, tc clie type ManageVpcLifecycleMetrics struct { dbSession *cdb.Session statusTransitionTime *prometheus.GaugeVec - siteIDNameMap map[uuid.UUID]string + siteNames *cwm.SiteNameCache } // RecordVpcStatusTransitionMetrics is a Temporal activity that records duration of important status transitions for VPCs @@ -713,17 +713,10 @@ func (mvlm ManageVpcLifecycleMetrics) RecordVpcStatusTransitionMetrics(ctx conte logger.Info().Msg("starting activity") - // Cache site name to avoid repeated DB call - siteName, ok := mvlm.siteIDNameMap[siteID] - if !ok { - siteDAO := cdbm.NewSiteDAO(mvlm.dbSession) - site, err := siteDAO.GetByID(context.Background(), nil, siteID, nil, false) - if err != nil { - logger.Error().Err(err).Str("Site ID", siteID.String()).Msg("failed to retrieve Site from DB") - return err - } - siteName = site.Name - mvlm.siteIDNameMap[siteID] = siteName + siteName, err := mvlm.siteNames.Get(ctx, mvlm.dbSession, siteID) + if err != nil { + logger.Error().Err(err).Str("Site ID", siteID.String()).Msg("failed to retrieve Site from DB") + return err } logger.Info().Int("EventCount", len(vpcLifecycleEvents)).Str("Site Name", siteName).Msg("processing vpc lifecycle events") @@ -758,7 +751,7 @@ func (mvlm ManageVpcLifecycleMetrics) RecordVpcStatusTransitionMetrics(ctx conte // Calculate duration from Deleting status to deletion time duration := event.Deleted.Sub(deletingStatusDetail.Created) // Note: VPC doesn't have VpcStatusDeleted constant, so we use string "Deleted" - mvlm.statusTransitionTime.WithLabelValues(siteName, cwm.InventoryOperationTypeDelete, cdbm.VpcStatusDeleting, "Deleted").Set(duration.Seconds()) + mvlm.statusTransitionTime.WithLabelValues(siteName, siteID.String(), cwm.InventoryOperationTypeDelete, cdbm.VpcStatusDeleting, "Deleted").Set(duration.Seconds()) metricsRecorded++ logger.Info(). Str("VPC ID", event.ObjectID.String()). @@ -779,18 +772,18 @@ func (mvlm ManageVpcLifecycleMetrics) RecordVpcStatusTransitionMetrics(ctx conte } // NewManageVpcLifecycleMetrics returns a new ManageVpcLifecycleMetrics activity -func NewManageVpcLifecycleMetrics(reg prometheus.Registerer, dbSession *cdb.Session) ManageVpcLifecycleMetrics { +func NewManageVpcLifecycleMetrics(reg prometheus.Registerer, dbSession *cdb.Session, namespace string) ManageVpcLifecycleMetrics { lifecycleMetrics := ManageVpcLifecycleMetrics{ dbSession: dbSession, statusTransitionTime: prometheus.NewGaugeVec( prometheus.GaugeOpts{ - Namespace: cwm.MetricsNamespace, + Namespace: namespace, Name: "vpc_operation_latency_seconds", Help: "Current latency of vpc operations", }, - []string{"site", "operation_type", "from_status", "to_status"}), + []string{"site", "site_id", "operation_type", "from_status", "to_status"}), - siteIDNameMap: map[uuid.UUID]string{}, + siteNames: cwm.NewSiteNameCache(), } reg.MustRegister(lifecycleMetrics.statusTransitionTime) diff --git a/rest-api/workflow/pkg/activity/vpc/vpc_test.go b/rest-api/workflow/pkg/activity/vpc/vpc_test.go index 27527aefce..4183048463 100644 --- a/rest-api/workflow/pkg/activity/vpc/vpc_test.go +++ b/rest-api/workflow/pkg/activity/vpc/vpc_test.go @@ -1331,7 +1331,7 @@ func Test_VpcMetrics_Delete_DeletingOnly(t *testing.T) { site := util.TestSetupSite(t, dbSession) reg := prometheus.NewRegistry() - lifecycleMetrics := NewManageVpcLifecycleMetrics(reg, dbSession) + lifecycleMetrics := NewManageVpcLifecycleMetrics(reg, dbSession, "nico_rest_workflow") testVpcID := uuid.New() // Set precise timestamps @@ -1351,7 +1351,7 @@ func Test_VpcMetrics_Delete_DeletingOnly(t *testing.T) { assert.NoError(t, err) // Verify metric was emitted with correct duration (200ms) - util.TestAssertMetricExistsTimes(t, reg, "cloud_workflow_vpc_operation_latency_seconds", 1, map[string]string{ + util.TestAssertMetricExistsTimes(t, reg, "nico_rest_workflow_vpc_operation_latency_seconds", 1, map[string]string{ "operation_type": "delete", "from_status": cdbm.VpcStatusDeleting, "to_status": "Deleted", @@ -1366,7 +1366,7 @@ func Test_VpcMetrics_Delete_MultipleDeleting(t *testing.T) { site := util.TestSetupSite(t, dbSession) reg := prometheus.NewRegistry() - lifecycleMetrics := NewManageVpcLifecycleMetrics(reg, dbSession) + lifecycleMetrics := NewManageVpcLifecycleMetrics(reg, dbSession, "nico_rest_workflow") testVpcID := uuid.New() // Set precise timestamps @@ -1394,7 +1394,7 @@ func Test_VpcMetrics_Delete_MultipleDeleting(t *testing.T) { assert.NoError(t, err) // Verify metric was emitted (should use first deleting timestamp, duration 300ms) - util.TestAssertMetricExistsTimes(t, reg, "cloud_workflow_vpc_operation_latency_seconds", 1, map[string]string{ + util.TestAssertMetricExistsTimes(t, reg, "nico_rest_workflow_vpc_operation_latency_seconds", 1, map[string]string{ "operation_type": "delete", "from_status": cdbm.VpcStatusDeleting, "to_status": "Deleted", @@ -1409,7 +1409,7 @@ func Test_VpcMetrics_Delete_NoDeleting(t *testing.T) { site := util.TestSetupSite(t, dbSession) reg := prometheus.NewRegistry() - lifecycleMetrics := NewManageVpcLifecycleMetrics(reg, dbSession) + lifecycleMetrics := NewManageVpcLifecycleMetrics(reg, dbSession, "nico_rest_workflow") testVpcID := uuid.New() // Set precise timestamps @@ -1428,5 +1428,5 @@ func Test_VpcMetrics_Delete_NoDeleting(t *testing.T) { assert.NoError(t, err) // Verify NO metric was emitted (no deleting status found) - util.TestAssertMetricExistsTimes(t, reg, "cloud_workflow_vpc_operation_latency_seconds", 0, nil, 0) + util.TestAssertMetricExistsTimes(t, reg, "nico_rest_workflow_vpc_operation_latency_seconds", 0, nil, 0) } diff --git a/rest-api/workflow/pkg/util/pagerduty.go b/rest-api/workflow/pkg/util/pagerduty.go deleted file mode 100644 index aa62e434e5..0000000000 --- a/rest-api/workflow/pkg/util/pagerduty.go +++ /dev/null @@ -1,49 +0,0 @@ -// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. -// SPDX-License-Identifier: Apache-2.0 - -package util - -import ( - "context" - "fmt" - - "github.com/PagerDuty/go-pagerduty" -) - -// PagerDutyClient wraps the official PagerDuty client for sending events -type PagerDutyClient struct { - integrationKey string -} - -// SendPagerDutyAlertWithDedupeKey sends a critical alert to PagerDuty with a custom deduplication key -func (pc PagerDutyClient) SendPagerDutyAlertWithDedupeKey(ctx context.Context, summary, source, dedupKey string, customDetails map[string]string) error { - event := pagerduty.V2Event{ - RoutingKey: pc.integrationKey, - Action: "trigger", - DedupKey: dedupKey, - Payload: &pagerduty.V2Payload{ - Summary: summary, - Source: source, - Severity: "critical", - Details: customDetails, - }, - } - - resp, err := pagerduty.ManageEventWithContext(ctx, event) - if err != nil { - return fmt.Errorf("failed to send PagerDuty event: %w", err) - } - - if resp.Status != "success" { - return fmt.Errorf("PagerDuty event not successful: %s", resp.Status) - } - - return nil -} - -// NewPagerDutyClient creates a new PagerDuty client wrapper -func NewPagerDutyClient(integrationKey string) PagerDutyClient { - return PagerDutyClient{ - integrationKey: integrationKey, - } -} diff --git a/rest-api/workflow/pkg/workflow/dpuextensionservice/update.go b/rest-api/workflow/pkg/workflow/dpuextensionservice/update.go index 479fa55c86..55c84e5d2e 100644 --- a/rest-api/workflow/pkg/workflow/dpuextensionservice/update.go +++ b/rest-api/workflow/pkg/workflow/dpuextensionservice/update.go @@ -5,14 +5,13 @@ package dpuextensionservice import ( "fmt" - "time" + cwi "github.com/NVIDIA/infra-controller/rest-api/workflow/internal/inventory" cwm "github.com/NVIDIA/infra-controller/rest-api/workflow/internal/metrics" "github.com/NVIDIA/infra-controller/rest-api/workflow/pkg/activity/dpuextensionservice" "github.com/google/uuid" "github.com/rs/zerolog/log" - "go.temporal.io/sdk/temporal" "go.temporal.io/sdk/workflow" corev1 "github.com/NVIDIA/infra-controller/rest-api/proto/core/gen/v1" @@ -22,7 +21,7 @@ import ( func UpdateDpuExtensionServiceInventory(ctx workflow.Context, siteID string, dpuExtensionServiceInventory *corev1.DpuExtensionServiceInventory) error { logger := log.With().Str("Workflow", "UpdateDpuExtensionServiceInventory").Str("Site ID", siteID).Logger() - startTime := time.Now() + startTime := workflow.Now(ctx) logger.Info().Msg("starting workflow") @@ -32,19 +31,7 @@ func UpdateDpuExtensionServiceInventory(ctx workflow.Context, siteID string, dpu return err } - // RetryPolicy specifies how to automatically handle retries if an Activity fails. - retrypolicy := &temporal.RetryPolicy{ - InitialInterval: 5 * time.Second, - BackoffCoefficient: 2.0, - MaximumInterval: 30 * time.Second, - MaximumAttempts: 2, - } - options := workflow.ActivityOptions{ - // Timeout options specify when to automatically timeout Activity functions. - StartToCloseTimeout: 30 * time.Second, - // Optionally provide a customized RetryPolicy. - RetryPolicy: retrypolicy, - } + options := cwi.ActivityOptions() ctx = workflow.WithActivityOptions(ctx, options) @@ -58,7 +45,7 @@ func UpdateDpuExtensionServiceInventory(ctx workflow.Context, siteID string, dpu // Record latency for this inventory call var inventoryMetricsManager cwm.ManageInventoryMetrics - serr := workflow.ExecuteActivity(ctx, inventoryMetricsManager.RecordLatency, parsedSiteID, "UpdateDpuExtensionServiceInventory", err != nil, time.Since(startTime)).Get(ctx, nil) + serr := workflow.ExecuteActivity(ctx, inventoryMetricsManager.RecordLatency, parsedSiteID, "UpdateDpuExtensionServiceInventory", err != nil, workflow.Now(ctx).Sub(startTime)).Get(ctx, nil) if serr != nil { logger.Warn().Err(serr).Msg("failed to execute activity: RecordLatency") } diff --git a/rest-api/workflow/pkg/workflow/expectedmachine/update.go b/rest-api/workflow/pkg/workflow/expectedmachine/update.go index dd31e63868..28a0245c2c 100644 --- a/rest-api/workflow/pkg/workflow/expectedmachine/update.go +++ b/rest-api/workflow/pkg/workflow/expectedmachine/update.go @@ -5,14 +5,13 @@ package expectedmachine import ( "fmt" - "time" + cwi "github.com/NVIDIA/infra-controller/rest-api/workflow/internal/inventory" cwm "github.com/NVIDIA/infra-controller/rest-api/workflow/internal/metrics" "github.com/google/uuid" "github.com/rs/zerolog/log" - "go.temporal.io/sdk/temporal" "go.temporal.io/sdk/workflow" corev1 "github.com/NVIDIA/infra-controller/rest-api/proto/core/gen/v1" @@ -23,7 +22,7 @@ import ( func UpdateExpectedMachineInventory(ctx workflow.Context, siteID string, expectedMachineInventory *corev1.ExpectedMachineInventory) (err error) { logger := log.With().Str("Workflow", "UpdateExpectedMachineInventory").Str("Site ID", siteID).Logger() - startTime := time.Now() + startTime := workflow.Now(ctx) logger.Info().Msg("starting workflow") @@ -33,19 +32,7 @@ func UpdateExpectedMachineInventory(ctx workflow.Context, siteID string, expecte return err } - // RetryPolicy specifies how to automatically handle retries if an Activity fails. - retrypolicy := &temporal.RetryPolicy{ - InitialInterval: 5 * time.Second, - BackoffCoefficient: 2.0, - MaximumInterval: 30 * time.Second, - MaximumAttempts: 2, - } - options := workflow.ActivityOptions{ - // Timeout options specify when to automatically timeout Activity functions. - StartToCloseTimeout: 30 * time.Second, - // Optionally provide a customized RetryPolicy. - RetryPolicy: retrypolicy, - } + options := cwi.ActivityOptions() ctx = workflow.WithActivityOptions(ctx, options) @@ -62,7 +49,7 @@ func UpdateExpectedMachineInventory(ctx workflow.Context, siteID string, expecte // Record latency for this inventory call var inventoryMetricsManager cwm.ManageInventoryMetrics - err = workflow.ExecuteActivity(ctx, inventoryMetricsManager.RecordLatency, parsedSiteID, "UpdateExpectedMachineInventory", err != nil, time.Since(startTime)).Get(ctx, nil) + err = workflow.ExecuteActivity(ctx, inventoryMetricsManager.RecordLatency, parsedSiteID, "UpdateExpectedMachineInventory", err != nil, workflow.Now(ctx).Sub(startTime)).Get(ctx, nil) if err != nil { logger.Warn().Err(err).Msg("failed to execute activity: RecordLatency") } diff --git a/rest-api/workflow/pkg/workflow/expectedpowershelf/update.go b/rest-api/workflow/pkg/workflow/expectedpowershelf/update.go index f83c672744..1c1b7d3916 100644 --- a/rest-api/workflow/pkg/workflow/expectedpowershelf/update.go +++ b/rest-api/workflow/pkg/workflow/expectedpowershelf/update.go @@ -5,14 +5,13 @@ package expectedpowershelf import ( "fmt" - "time" + cwi "github.com/NVIDIA/infra-controller/rest-api/workflow/internal/inventory" cwm "github.com/NVIDIA/infra-controller/rest-api/workflow/internal/metrics" "github.com/google/uuid" "github.com/rs/zerolog/log" - "go.temporal.io/sdk/temporal" "go.temporal.io/sdk/workflow" corev1 "github.com/NVIDIA/infra-controller/rest-api/proto/core/gen/v1" @@ -23,7 +22,7 @@ import ( func UpdateExpectedPowerShelfInventory(ctx workflow.Context, siteID string, expectedPowerShelfInventory *corev1.ExpectedPowerShelfInventory) (err error) { logger := log.With().Str("Workflow", "UpdateExpectedPowerShelfInventory").Str("Site ID", siteID).Logger() - startTime := time.Now() + startTime := workflow.Now(ctx) logger.Info().Msg("starting workflow") @@ -33,19 +32,7 @@ func UpdateExpectedPowerShelfInventory(ctx workflow.Context, siteID string, expe return err } - // RetryPolicy specifies how to automatically handle retries if an Activity fails. - retrypolicy := &temporal.RetryPolicy{ - InitialInterval: 5 * time.Second, - BackoffCoefficient: 2.0, - MaximumInterval: 30 * time.Second, - MaximumAttempts: 2, - } - options := workflow.ActivityOptions{ - // Timeout options specify when to automatically timeout Activity functions. - StartToCloseTimeout: 30 * time.Second, - // Optionally provide a customized RetryPolicy. - RetryPolicy: retrypolicy, - } + options := cwi.ActivityOptions() ctx = workflow.WithActivityOptions(ctx, options) @@ -62,7 +49,7 @@ func UpdateExpectedPowerShelfInventory(ctx workflow.Context, siteID string, expe // Record latency for this inventory call var inventoryMetricsManager cwm.ManageInventoryMetrics - err = workflow.ExecuteActivity(ctx, inventoryMetricsManager.RecordLatency, parsedSiteID, "UpdateExpectedPowerShelfInventory", err != nil, time.Since(startTime)).Get(ctx, nil) + err = workflow.ExecuteActivity(ctx, inventoryMetricsManager.RecordLatency, parsedSiteID, "UpdateExpectedPowerShelfInventory", err != nil, workflow.Now(ctx).Sub(startTime)).Get(ctx, nil) if err != nil { logger.Warn().Err(err).Msg("failed to execute activity: RecordLatency") } diff --git a/rest-api/workflow/pkg/workflow/expectedrack/update.go b/rest-api/workflow/pkg/workflow/expectedrack/update.go index bf08147283..d8d2e6cf8f 100644 --- a/rest-api/workflow/pkg/workflow/expectedrack/update.go +++ b/rest-api/workflow/pkg/workflow/expectedrack/update.go @@ -5,14 +5,13 @@ package expectedrack import ( "fmt" - "time" + cwi "github.com/NVIDIA/infra-controller/rest-api/workflow/internal/inventory" cwm "github.com/NVIDIA/infra-controller/rest-api/workflow/internal/metrics" "github.com/google/uuid" "github.com/rs/zerolog/log" - "go.temporal.io/sdk/temporal" "go.temporal.io/sdk/workflow" corev1 "github.com/NVIDIA/infra-controller/rest-api/proto/core/gen/v1" @@ -23,7 +22,7 @@ import ( func UpdateExpectedRackInventory(ctx workflow.Context, siteID string, expectedRackInventory *corev1.ExpectedRackInventory) (err error) { logger := log.With().Str("Workflow", "UpdateExpectedRackInventory").Str("Site ID", siteID).Logger() - startTime := time.Now() + startTime := workflow.Now(ctx) logger.Info().Msg("starting workflow") @@ -33,19 +32,7 @@ func UpdateExpectedRackInventory(ctx workflow.Context, siteID string, expectedRa return err } - // RetryPolicy specifies how to automatically handle retries if an Activity fails. - retrypolicy := &temporal.RetryPolicy{ - InitialInterval: 5 * time.Second, - BackoffCoefficient: 2.0, - MaximumInterval: 30 * time.Second, - MaximumAttempts: 2, - } - options := workflow.ActivityOptions{ - // Timeout options specify when to automatically timeout Activity functions. - StartToCloseTimeout: 30 * time.Second, - // Optionally provide a customized RetryPolicy. - RetryPolicy: retrypolicy, - } + options := cwi.ActivityOptions() ctx = workflow.WithActivityOptions(ctx, options) @@ -62,7 +49,7 @@ func UpdateExpectedRackInventory(ctx workflow.Context, siteID string, expectedRa // Record latency for this inventory call var inventoryMetricsManager cwm.ManageInventoryMetrics - err = workflow.ExecuteActivity(ctx, inventoryMetricsManager.RecordLatency, parsedSiteID, "UpdateExpectedRackInventory", err != nil, time.Since(startTime)).Get(ctx, nil) + err = workflow.ExecuteActivity(ctx, inventoryMetricsManager.RecordLatency, parsedSiteID, "UpdateExpectedRackInventory", err != nil, workflow.Now(ctx).Sub(startTime)).Get(ctx, nil) if err != nil { logger.Warn().Err(err).Msg("failed to execute activity: RecordLatency") } diff --git a/rest-api/workflow/pkg/workflow/expectedswitch/update.go b/rest-api/workflow/pkg/workflow/expectedswitch/update.go index 3bf73ab834..d78e0e528c 100644 --- a/rest-api/workflow/pkg/workflow/expectedswitch/update.go +++ b/rest-api/workflow/pkg/workflow/expectedswitch/update.go @@ -5,14 +5,13 @@ package expectedswitch import ( "fmt" - "time" + cwi "github.com/NVIDIA/infra-controller/rest-api/workflow/internal/inventory" cwm "github.com/NVIDIA/infra-controller/rest-api/workflow/internal/metrics" "github.com/google/uuid" "github.com/rs/zerolog/log" - "go.temporal.io/sdk/temporal" "go.temporal.io/sdk/workflow" corev1 "github.com/NVIDIA/infra-controller/rest-api/proto/core/gen/v1" @@ -23,7 +22,7 @@ import ( func UpdateExpectedSwitchInventory(ctx workflow.Context, siteID string, expectedSwitchInventory *corev1.ExpectedSwitchInventory) (err error) { logger := log.With().Str("Workflow", "UpdateExpectedSwitchInventory").Str("Site ID", siteID).Logger() - startTime := time.Now() + startTime := workflow.Now(ctx) logger.Info().Msg("starting workflow") @@ -33,19 +32,7 @@ func UpdateExpectedSwitchInventory(ctx workflow.Context, siteID string, expected return err } - // RetryPolicy specifies how to automatically handle retries if an Activity fails. - retrypolicy := &temporal.RetryPolicy{ - InitialInterval: 5 * time.Second, - BackoffCoefficient: 2.0, - MaximumInterval: 30 * time.Second, - MaximumAttempts: 2, - } - options := workflow.ActivityOptions{ - // Timeout options specify when to automatically timeout Activity functions. - StartToCloseTimeout: 30 * time.Second, - // Optionally provide a customized RetryPolicy. - RetryPolicy: retrypolicy, - } + options := cwi.ActivityOptions() ctx = workflow.WithActivityOptions(ctx, options) @@ -62,7 +49,7 @@ func UpdateExpectedSwitchInventory(ctx workflow.Context, siteID string, expected // Record latency for this inventory call var inventoryMetricsManager cwm.ManageInventoryMetrics - err = workflow.ExecuteActivity(ctx, inventoryMetricsManager.RecordLatency, parsedSiteID, "UpdateExpectedSwitchInventory", err != nil, time.Since(startTime)).Get(ctx, nil) + err = workflow.ExecuteActivity(ctx, inventoryMetricsManager.RecordLatency, parsedSiteID, "UpdateExpectedSwitchInventory", err != nil, workflow.Now(ctx).Sub(startTime)).Get(ctx, nil) if err != nil { logger.Warn().Err(err).Msg("failed to execute activity: RecordLatency") } diff --git a/rest-api/workflow/pkg/workflow/infinibandpartition/update.go b/rest-api/workflow/pkg/workflow/infinibandpartition/update.go index 78a596f241..815a25d64b 100644 --- a/rest-api/workflow/pkg/workflow/infinibandpartition/update.go +++ b/rest-api/workflow/pkg/workflow/infinibandpartition/update.go @@ -5,14 +5,13 @@ package infinibandpartition import ( "fmt" - "time" + cwi "github.com/NVIDIA/infra-controller/rest-api/workflow/internal/inventory" cwm "github.com/NVIDIA/infra-controller/rest-api/workflow/internal/metrics" "github.com/google/uuid" "github.com/rs/zerolog/log" - "go.temporal.io/sdk/temporal" "go.temporal.io/sdk/workflow" ibpActivity "github.com/NVIDIA/infra-controller/rest-api/workflow/pkg/activity/infinibandpartition" @@ -24,7 +23,7 @@ import ( func UpdateInfiniBandPartitionInventory(ctx workflow.Context, siteID string, ibpInventory *corev1.InfiniBandPartitionInventory) (err error) { logger := log.With().Str("Workflow", "UpdateInfiniBandPartitionInventory").Str("Site ID", siteID).Logger() - startTime := time.Now() + startTime := workflow.Now(ctx) logger.Info().Msg("starting workflow") @@ -34,19 +33,7 @@ func UpdateInfiniBandPartitionInventory(ctx workflow.Context, siteID string, ibp return err } - // RetryPolicy specifies how to automatically handle retries if an Activity fails. - retrypolicy := &temporal.RetryPolicy{ - InitialInterval: 5 * time.Second, - BackoffCoefficient: 2.0, - MaximumInterval: 30 * time.Second, - MaximumAttempts: 2, - } - options := workflow.ActivityOptions{ - // Timeout options specify when to automatically timeout Activity functions. - StartToCloseTimeout: 30 * time.Second, - // Optionally provide a customized RetryPolicy. - RetryPolicy: retrypolicy, - } + options := cwi.ActivityOptions() ctx = workflow.WithActivityOptions(ctx, options) @@ -60,7 +47,7 @@ func UpdateInfiniBandPartitionInventory(ctx workflow.Context, siteID string, ibp // Record latency for this inventory call var inventoryMetricsManager cwm.ManageInventoryMetrics - serr := workflow.ExecuteActivity(ctx, inventoryMetricsManager.RecordLatency, parsedSiteID, "UpdateInfiniBandPartitionInventory", err != nil, time.Since(startTime)).Get(ctx, nil) + serr := workflow.ExecuteActivity(ctx, inventoryMetricsManager.RecordLatency, parsedSiteID, "UpdateInfiniBandPartitionInventory", err != nil, workflow.Now(ctx).Sub(startTime)).Get(ctx, nil) if serr != nil { logger.Warn().Err(serr).Msg("failed to execute activity: RecordLatency") } diff --git a/rest-api/workflow/pkg/workflow/instance/update.go b/rest-api/workflow/pkg/workflow/instance/update.go index 61a91c4c06..894b17b92f 100644 --- a/rest-api/workflow/pkg/workflow/instance/update.go +++ b/rest-api/workflow/pkg/workflow/instance/update.go @@ -5,14 +5,13 @@ package instance import ( "fmt" - "time" + cwi "github.com/NVIDIA/infra-controller/rest-api/workflow/internal/inventory" cwm "github.com/NVIDIA/infra-controller/rest-api/workflow/internal/metrics" "github.com/google/uuid" "github.com/rs/zerolog/log" - "go.temporal.io/sdk/temporal" "go.temporal.io/sdk/workflow" instanceActivity "github.com/NVIDIA/infra-controller/rest-api/workflow/pkg/activity/instance" @@ -24,7 +23,7 @@ import ( func UpdateInstanceInventory(ctx workflow.Context, siteID string, instanceInventory *corev1.InstanceInventory) (err error) { logger := log.With().Str("Workflow", "UpdateInstanceInventory").Str("Site ID", siteID).Logger() - startTime := time.Now() + startTime := workflow.Now(ctx) logger.Info().Msg("starting workflow") @@ -34,19 +33,7 @@ func UpdateInstanceInventory(ctx workflow.Context, siteID string, instanceInvent return err } - // RetryPolicy specifies how to automatically handle retries if an Activity fails. - retryPolicy := &temporal.RetryPolicy{ - InitialInterval: 5 * time.Second, - BackoffCoefficient: 2.0, - MaximumInterval: 30 * time.Second, - MaximumAttempts: 2, - } - options := workflow.ActivityOptions{ - // Timeout options specify when to automatically timeout Activity functions. - StartToCloseTimeout: 30 * time.Second, - // Optionally provide a customized RetryPolicy. - RetryPolicy: retryPolicy, - } + options := cwi.ActivityOptions() ctx = workflow.WithActivityOptions(ctx, options) @@ -69,7 +56,7 @@ func UpdateInstanceInventory(ctx workflow.Context, siteID string, instanceInvent // Record latency for this inventory call var inventoryMetricsManager cwm.ManageInventoryMetrics - serr = workflow.ExecuteActivity(ctx, inventoryMetricsManager.RecordLatency, parsedSiteID, "UpdateInstanceInventory", err != nil, time.Since(startTime)).Get(ctx, nil) + serr = workflow.ExecuteActivity(ctx, inventoryMetricsManager.RecordLatency, parsedSiteID, "UpdateInstanceInventory", err != nil, workflow.Now(ctx).Sub(startTime)).Get(ctx, nil) if serr != nil { logger.Warn().Err(serr).Msg("failed to execute activity: RecordLatency") } diff --git a/rest-api/workflow/pkg/workflow/instancetype/update.go b/rest-api/workflow/pkg/workflow/instancetype/update.go index 11d2dd4dce..c9a3c523a1 100644 --- a/rest-api/workflow/pkg/workflow/instancetype/update.go +++ b/rest-api/workflow/pkg/workflow/instancetype/update.go @@ -5,14 +5,13 @@ package instancetype import ( "fmt" - "time" + cwi "github.com/NVIDIA/infra-controller/rest-api/workflow/internal/inventory" cwm "github.com/NVIDIA/infra-controller/rest-api/workflow/internal/metrics" "github.com/google/uuid" "github.com/rs/zerolog/log" - "go.temporal.io/sdk/temporal" "go.temporal.io/sdk/workflow" instanceTypeActivity "github.com/NVIDIA/infra-controller/rest-api/workflow/pkg/activity/instancetype" @@ -24,7 +23,7 @@ import ( func UpdateInstanceTypeInventory(ctx workflow.Context, siteID string, instanceTypeInventory *corev1.InstanceTypeInventory) (err error) { logger := log.With().Str("Workflow", "UpdateInstanceTypeInventory").Str("Site ID", siteID).Logger() - startTime := time.Now() + startTime := workflow.Now(ctx) logger.Info().Msg("starting workflow") @@ -34,19 +33,7 @@ func UpdateInstanceTypeInventory(ctx workflow.Context, siteID string, instanceTy return err } - // RetryPolicy specifies how to automatically handle retries if an Activity fails. - retrypolicy := &temporal.RetryPolicy{ - InitialInterval: 5 * time.Second, - BackoffCoefficient: 2.0, - MaximumInterval: 30 * time.Second, - MaximumAttempts: 2, - } - options := workflow.ActivityOptions{ - // Timeout options specify when to automatically timeout Activity functions. - StartToCloseTimeout: 30 * time.Second, - // Optionally provide a customized RetryPolicy. - RetryPolicy: retrypolicy, - } + options := cwi.ActivityOptions() ctx = workflow.WithActivityOptions(ctx, options) @@ -60,7 +47,7 @@ func UpdateInstanceTypeInventory(ctx workflow.Context, siteID string, instanceTy // Record latency for this inventory call var inventoryMetricsManager cwm.ManageInventoryMetrics - serr := workflow.ExecuteActivity(ctx, inventoryMetricsManager.RecordLatency, parsedSiteID, "UpdateInstanceTypeInventory", err != nil, time.Since(startTime)).Get(ctx, nil) + serr := workflow.ExecuteActivity(ctx, inventoryMetricsManager.RecordLatency, parsedSiteID, "UpdateInstanceTypeInventory", err != nil, workflow.Now(ctx).Sub(startTime)).Get(ctx, nil) if serr != nil { logger.Warn().Err(serr).Msg("failed to execute activity: RecordLatency") } diff --git a/rest-api/workflow/pkg/workflow/ipxetemplate/update.go b/rest-api/workflow/pkg/workflow/ipxetemplate/update.go index 67b2d9594f..0aaa6e722c 100644 --- a/rest-api/workflow/pkg/workflow/ipxetemplate/update.go +++ b/rest-api/workflow/pkg/workflow/ipxetemplate/update.go @@ -5,16 +5,15 @@ package ipxetemplate import ( "fmt" - "time" corev1 "github.com/NVIDIA/infra-controller/rest-api/proto/core/gen/v1" + cwi "github.com/NVIDIA/infra-controller/rest-api/workflow/internal/inventory" cwm "github.com/NVIDIA/infra-controller/rest-api/workflow/internal/metrics" ipxeTemplateActivity "github.com/NVIDIA/infra-controller/rest-api/workflow/pkg/activity/ipxetemplate" "github.com/google/uuid" "github.com/rs/zerolog/log" - "go.temporal.io/sdk/temporal" "go.temporal.io/sdk/workflow" ) @@ -33,16 +32,7 @@ func UpdateIpxeTemplateInventory(ctx workflow.Context, siteID string, inventory return err } - retrypolicy := &temporal.RetryPolicy{ - InitialInterval: 5 * time.Second, - BackoffCoefficient: 2.0, - MaximumInterval: 30 * time.Second, - MaximumAttempts: 2, - } - options := workflow.ActivityOptions{ - StartToCloseTimeout: 30 * time.Second, - RetryPolicy: retrypolicy, - } + options := cwi.ActivityOptions() ctx = workflow.WithActivityOptions(ctx, options) diff --git a/rest-api/workflow/pkg/workflow/machine/update.go b/rest-api/workflow/pkg/workflow/machine/update.go index 89cbf0db9d..6de0751ac3 100644 --- a/rest-api/workflow/pkg/workflow/machine/update.go +++ b/rest-api/workflow/pkg/workflow/machine/update.go @@ -5,19 +5,18 @@ package machine import ( "fmt" - "time" "github.com/google/uuid" "github.com/rs/zerolog/log" - "go.temporal.io/sdk/temporal" "go.temporal.io/sdk/workflow" corev1 "github.com/NVIDIA/infra-controller/rest-api/proto/core/gen/v1" machineActivity "github.com/NVIDIA/infra-controller/rest-api/workflow/pkg/activity/machine" + cwi "github.com/NVIDIA/infra-controller/rest-api/workflow/internal/inventory" cwm "github.com/NVIDIA/infra-controller/rest-api/workflow/internal/metrics" ) @@ -25,7 +24,7 @@ import ( func UpdateMachineInventory(ctx workflow.Context, siteID string, machineInventory *corev1.MachineInventory) (err error) { logger := log.With().Str("Workflow", "UpdateMachineInventory").Str("Site ID", siteID).Logger() - startTime := time.Now() + startTime := workflow.Now(ctx) logger.Info().Msg("starting workflow") @@ -35,19 +34,7 @@ func UpdateMachineInventory(ctx workflow.Context, siteID string, machineInventor return err } - // RetryPolicy specifies how to automatically handle retries if an Activity fails. - retrypolicy := &temporal.RetryPolicy{ - InitialInterval: 5 * time.Second, - BackoffCoefficient: 2.0, - MaximumInterval: 30 * time.Second, - MaximumAttempts: 2, - } - options := workflow.ActivityOptions{ - // Timeout options specify when to automatically timeout Activity functions. - StartToCloseTimeout: 2 * time.Minute, - // Optionally provide a customized RetryPolicy. - RetryPolicy: retrypolicy, - } + options := cwi.ActivityOptions() ctx = workflow.WithActivityOptions(ctx, options) @@ -61,7 +48,7 @@ func UpdateMachineInventory(ctx workflow.Context, siteID string, machineInventor // Record latency for this inventory call var inventoryMetricsManager cwm.ManageInventoryMetrics - serr := workflow.ExecuteActivity(ctx, inventoryMetricsManager.RecordLatency, parsedSiteID, "UpdateMachineInventory", err != nil, time.Since(startTime)).Get(ctx, nil) + serr := workflow.ExecuteActivity(ctx, inventoryMetricsManager.RecordLatency, parsedSiteID, "UpdateMachineInventory", err != nil, workflow.Now(ctx).Sub(startTime)).Get(ctx, nil) if serr != nil { logger.Warn().Err(serr).Msg("failed to execute activity: RecordLatency") } diff --git a/rest-api/workflow/pkg/workflow/networksecuritygroup/update.go b/rest-api/workflow/pkg/workflow/networksecuritygroup/update.go index 31e82cb982..a35cd97a2b 100644 --- a/rest-api/workflow/pkg/workflow/networksecuritygroup/update.go +++ b/rest-api/workflow/pkg/workflow/networksecuritygroup/update.go @@ -5,14 +5,13 @@ package networksecuritygroup import ( "fmt" - "time" + cwi "github.com/NVIDIA/infra-controller/rest-api/workflow/internal/inventory" cwm "github.com/NVIDIA/infra-controller/rest-api/workflow/internal/metrics" "github.com/google/uuid" "github.com/rs/zerolog/log" - "go.temporal.io/sdk/temporal" "go.temporal.io/sdk/workflow" networkSecurityGroupActivity "github.com/NVIDIA/infra-controller/rest-api/workflow/pkg/activity/networksecuritygroup" @@ -24,7 +23,7 @@ import ( func UpdateNetworkSecurityGroupInventory(ctx workflow.Context, siteID string, networkSecurityGroupInventory *corev1.NetworkSecurityGroupInventory) (err error) { logger := log.With().Str("Workflow", "UpdateNetworkSecurityGroupInventory").Str("Site ID", siteID).Logger() - startTime := time.Now() + startTime := workflow.Now(ctx) logger.Info().Msg("starting workflow") @@ -34,19 +33,7 @@ func UpdateNetworkSecurityGroupInventory(ctx workflow.Context, siteID string, ne return err } - // RetryPolicy specifies how to automatically handle retries if an Activity fails. - retrypolicy := &temporal.RetryPolicy{ - InitialInterval: 5 * time.Second, - BackoffCoefficient: 2.0, - MaximumInterval: 30 * time.Second, - MaximumAttempts: 2, - } - options := workflow.ActivityOptions{ - // Timeout options specify when to automatically timeout Activity functions. - StartToCloseTimeout: 30 * time.Second, - // Optionally provide a customized RetryPolicy. - RetryPolicy: retrypolicy, - } + options := cwi.ActivityOptions() ctx = workflow.WithActivityOptions(ctx, options) @@ -60,7 +47,7 @@ func UpdateNetworkSecurityGroupInventory(ctx workflow.Context, siteID string, ne // Record latency for this inventory call var inventoryMetricsManager cwm.ManageInventoryMetrics - serr := workflow.ExecuteActivity(ctx, inventoryMetricsManager.RecordLatency, parsedSiteID, "UpdateNetworkSecurityGroupInventory", err != nil, time.Since(startTime)).Get(ctx, nil) + serr := workflow.ExecuteActivity(ctx, inventoryMetricsManager.RecordLatency, parsedSiteID, "UpdateNetworkSecurityGroupInventory", err != nil, workflow.Now(ctx).Sub(startTime)).Get(ctx, nil) if serr != nil { logger.Warn().Err(serr).Msg("failed to execute activity: RecordLatency") } diff --git a/rest-api/workflow/pkg/workflow/nvlinklogicalpartition/update.go b/rest-api/workflow/pkg/workflow/nvlinklogicalpartition/update.go index c1ec2f9144..e8c38e116f 100644 --- a/rest-api/workflow/pkg/workflow/nvlinklogicalpartition/update.go +++ b/rest-api/workflow/pkg/workflow/nvlinklogicalpartition/update.go @@ -5,14 +5,13 @@ package nvlinklogicalpartition import ( "fmt" - "time" + cwi "github.com/NVIDIA/infra-controller/rest-api/workflow/internal/inventory" cwm "github.com/NVIDIA/infra-controller/rest-api/workflow/internal/metrics" "github.com/google/uuid" "github.com/rs/zerolog/log" - "go.temporal.io/sdk/temporal" "go.temporal.io/sdk/workflow" corev1 "github.com/NVIDIA/infra-controller/rest-api/proto/core/gen/v1" @@ -23,7 +22,7 @@ import ( func UpdateNVLinkLogicalPartitionInventory(ctx workflow.Context, siteID string, nvlinklogicalpartitionInventory *corev1.NVLinkLogicalPartitionInventory) (err error) { logger := log.With().Str("Workflow", "UpdateNVLinkLogicalPartitionInventory").Str("Site ID", siteID).Logger() - startTime := time.Now() + startTime := workflow.Now(ctx) logger.Info().Msg("starting workflow") @@ -33,20 +32,7 @@ func UpdateNVLinkLogicalPartitionInventory(ctx workflow.Context, siteID string, return err } - // RetryPolicy specifies how to automatically handle retries if an Activity fails. - retrypolicy := &temporal.RetryPolicy{ - InitialInterval: 5 * time.Second, - BackoffCoefficient: 2.0, - MaximumInterval: 30 * time.Second, - MaximumAttempts: 2, - } - - options := workflow.ActivityOptions{ - // Timeout options specify when to automatically timeout Activity functions. - StartToCloseTimeout: 30 * time.Second, - // Optionally provide a customized RetryPolicy. - RetryPolicy: retrypolicy, - } + options := cwi.ActivityOptions() ctx = workflow.WithActivityOptions(ctx, options) @@ -60,7 +46,7 @@ func UpdateNVLinkLogicalPartitionInventory(ctx workflow.Context, siteID string, // Record latency for this inventory call var inventoryMetricsManager cwm.ManageInventoryMetrics - serr := workflow.ExecuteActivity(ctx, inventoryMetricsManager.RecordLatency, parsedSiteID, "UpdateNVLinkLogicalPartitionInventory", err != nil, time.Since(startTime)).Get(ctx, nil) + serr := workflow.ExecuteActivity(ctx, inventoryMetricsManager.RecordLatency, parsedSiteID, "UpdateNVLinkLogicalPartitionInventory", err != nil, workflow.Now(ctx).Sub(startTime)).Get(ctx, nil) if serr != nil { logger.Warn().Err(serr).Msg("failed to execute activity: RecordLatency") } diff --git a/rest-api/workflow/pkg/workflow/operatingsystem/update.go b/rest-api/workflow/pkg/workflow/operatingsystem/update.go index 8155e60bd7..82fb9f7e65 100644 --- a/rest-api/workflow/pkg/workflow/operatingsystem/update.go +++ b/rest-api/workflow/pkg/workflow/operatingsystem/update.go @@ -5,15 +5,14 @@ package operatingsystem import ( "fmt" - "time" + cwi "github.com/NVIDIA/infra-controller/rest-api/workflow/internal/inventory" cwm "github.com/NVIDIA/infra-controller/rest-api/workflow/internal/metrics" osActivity "github.com/NVIDIA/infra-controller/rest-api/workflow/pkg/activity/operatingsystem" "github.com/google/uuid" "github.com/rs/zerolog/log" - "go.temporal.io/sdk/temporal" "go.temporal.io/sdk/workflow" corev1 "github.com/NVIDIA/infra-controller/rest-api/proto/core/gen/v1" @@ -33,19 +32,7 @@ func UpdateOsImageInventory(ctx workflow.Context, siteID string, osImageInventor return err } - // RetryPolicy specifies how to automatically handle retries if an Activity fails. - retryPolicy := &temporal.RetryPolicy{ - InitialInterval: 5 * time.Second, - BackoffCoefficient: 2.0, - MaximumInterval: 30 * time.Second, - MaximumAttempts: 2, - } - options := workflow.ActivityOptions{ - // Timeout options specify when to automatically timeout Activity functions. - StartToCloseTimeout: 30 * time.Second, - // Optionally provide a customized RetryPolicy. - RetryPolicy: retryPolicy, - } + options := cwi.ActivityOptions() ctx = workflow.WithActivityOptions(ctx, options) @@ -95,16 +82,7 @@ func UpdateOperatingSystemInventory(ctx workflow.Context, siteID string, invento return err } - retryPolicy := &temporal.RetryPolicy{ - InitialInterval: 5 * time.Second, - BackoffCoefficient: 2.0, - MaximumInterval: 30 * time.Second, - MaximumAttempts: 2, - } - options := workflow.ActivityOptions{ - StartToCloseTimeout: 30 * time.Second, - RetryPolicy: retryPolicy, - } + options := cwi.ActivityOptions() ctx = workflow.WithActivityOptions(ctx, options) var osManager osActivity.ManageOsImage diff --git a/rest-api/workflow/pkg/workflow/site/update.go b/rest-api/workflow/pkg/workflow/site/update.go index a07bd89043..71e1463836 100644 --- a/rest-api/workflow/pkg/workflow/site/update.go +++ b/rest-api/workflow/pkg/workflow/site/update.go @@ -6,17 +6,16 @@ package site import ( "errors" "fmt" - "time" "github.com/rs/zerolog/log" corev1 "github.com/NVIDIA/infra-controller/rest-api/proto/core/gen/v1" "github.com/google/uuid" - "go.temporal.io/sdk/temporal" "go.temporal.io/sdk/workflow" siteActivity "github.com/NVIDIA/infra-controller/rest-api/workflow/pkg/activity/site" + cwi "github.com/NVIDIA/infra-controller/rest-api/workflow/internal/inventory" cwm "github.com/NVIDIA/infra-controller/rest-api/workflow/internal/metrics" ) @@ -39,15 +38,7 @@ func UpdateSiteConfigInventory(ctx workflow.Context, siteIDStr string, coreBuild return err } - options := workflow.ActivityOptions{ - StartToCloseTimeout: 5 * time.Minute, - RetryPolicy: &temporal.RetryPolicy{ - InitialInterval: 1 * time.Second, - BackoffCoefficient: 2.0, - MaximumInterval: 1 * time.Minute, - MaximumAttempts: 3, - }, - } + options := cwi.ActivityOptions() ctx = workflow.WithActivityOptions(ctx, options) var manageSite siteActivity.ManageSite @@ -102,15 +93,7 @@ func UpdateSiteConfigInventoryV2(ctx workflow.Context, siteIDStr string, invento return err } - options := workflow.ActivityOptions{ - StartToCloseTimeout: 5 * time.Minute, - RetryPolicy: &temporal.RetryPolicy{ - InitialInterval: 1 * time.Second, - BackoffCoefficient: 2.0, - MaximumInterval: 1 * time.Minute, - MaximumAttempts: 3, - }, - } + options := cwi.ActivityOptions() ctx = workflow.WithActivityOptions(ctx, options) coreBuildInfo := inventory.GetCoreBuildInfo() diff --git a/rest-api/workflow/pkg/workflow/sku/update.go b/rest-api/workflow/pkg/workflow/sku/update.go index 080edb36cb..3368918670 100644 --- a/rest-api/workflow/pkg/workflow/sku/update.go +++ b/rest-api/workflow/pkg/workflow/sku/update.go @@ -5,14 +5,13 @@ package sku import ( "fmt" - "time" + cwi "github.com/NVIDIA/infra-controller/rest-api/workflow/internal/inventory" cwm "github.com/NVIDIA/infra-controller/rest-api/workflow/internal/metrics" "github.com/google/uuid" "github.com/rs/zerolog/log" - "go.temporal.io/sdk/temporal" "go.temporal.io/sdk/workflow" corev1 "github.com/NVIDIA/infra-controller/rest-api/proto/core/gen/v1" @@ -23,7 +22,7 @@ import ( func UpdateSkuInventory(ctx workflow.Context, siteID string, skuInventory *corev1.SkuInventory) (err error) { logger := log.With().Str("Workflow", "UpdateSkuInventory").Str("Site ID", siteID).Logger() - startTime := time.Now() + startTime := workflow.Now(ctx) logger.Info().Msg("starting workflow") @@ -33,19 +32,7 @@ func UpdateSkuInventory(ctx workflow.Context, siteID string, skuInventory *corev return err } - // RetryPolicy specifies how to automatically handle retries if an Activity fails. - retrypolicy := &temporal.RetryPolicy{ - InitialInterval: 5 * time.Second, - BackoffCoefficient: 2.0, - MaximumInterval: 30 * time.Second, - MaximumAttempts: 2, - } - options := workflow.ActivityOptions{ - // Timeout options specify when to automatically timeout Activity functions. - StartToCloseTimeout: 30 * time.Second, - // Optionally provide a customized RetryPolicy. - RetryPolicy: retrypolicy, - } + options := cwi.ActivityOptions() ctx = workflow.WithActivityOptions(ctx, options) @@ -62,7 +49,7 @@ func UpdateSkuInventory(ctx workflow.Context, siteID string, skuInventory *corev // Record latency for this inventory call var inventoryMetricsManager cwm.ManageInventoryMetrics - err = workflow.ExecuteActivity(ctx, inventoryMetricsManager.RecordLatency, parsedSiteID, "UpdateSkuInventory", err != nil, time.Since(startTime)).Get(ctx, nil) + err = workflow.ExecuteActivity(ctx, inventoryMetricsManager.RecordLatency, parsedSiteID, "UpdateSkuInventory", err != nil, workflow.Now(ctx).Sub(startTime)).Get(ctx, nil) if err != nil { logger.Warn().Err(err).Msg("failed to execute activity: RecordLatency") } diff --git a/rest-api/workflow/pkg/workflow/sshkeygroup/update.go b/rest-api/workflow/pkg/workflow/sshkeygroup/update.go index 41bcf4be4f..4754dd2718 100644 --- a/rest-api/workflow/pkg/workflow/sshkeygroup/update.go +++ b/rest-api/workflow/pkg/workflow/sshkeygroup/update.go @@ -7,6 +7,7 @@ import ( "fmt" "time" + cwi "github.com/NVIDIA/infra-controller/rest-api/workflow/internal/inventory" cwm "github.com/NVIDIA/infra-controller/rest-api/workflow/internal/metrics" "github.com/google/uuid" @@ -24,7 +25,7 @@ import ( func UpdateSSHKeyGroupInventory(ctx workflow.Context, siteID string, sshKeyGroupInventory *corev1.SSHKeyGroupInventory) (err error) { logger := log.With().Str("Workflow", "UpdateSSHKeyGroupInventory").Str("Site ID", siteID).Logger() - startTime := time.Now() + startTime := workflow.Now(ctx) logger.Info().Msg("starting workflow") @@ -34,18 +35,27 @@ func UpdateSSHKeyGroupInventory(ctx workflow.Context, siteID string, sshKeyGroup return err } - // RetryPolicy specifies how to automatically handle retries if an Activity fails. + // Deliberately not cwi.ActivityOptions. UpdateSSHKeyGroupsInDB is the one + // inventory activity that waits on another service inside its per-object + // loop: it starts a Site workflow per keyset and blocks on it for up to + // cwutil.WorkflowContextTimeout. Under the shared 60s budget that inner + // deadline is unreachable, so the activity would be killed mid-wait and its + // termination path, which cleans up the orphaned Site workflow, would never + // run. This has to stay above that inner wait. + // + // It buys headroom rather than correctness. The wait is per object and the + // page holds up to 25, so a Site slow to apply keysets can still exhaust + // this. Making the sync asynchronous, as Tenant and VPC already do, is what + // would actually bound it. retrypolicy := &temporal.RetryPolicy{ - InitialInterval: 5 * time.Second, - BackoffCoefficient: 2.0, - MaximumInterval: 30 * time.Second, - MaximumAttempts: 2, + InitialInterval: cwi.ActivityInitialInterval, + BackoffCoefficient: cwi.ActivityBackoffCoefficient, + MaximumInterval: cwi.ActivityMaximumInterval, + MaximumAttempts: cwi.ActivityMaximumAttempts, } options := workflow.ActivityOptions{ - // Timeout options specify when to automatically timeout Activity functions. - StartToCloseTimeout: 30 * time.Second, - // Optionally provide a customized RetryPolicy. - RetryPolicy: retrypolicy, + StartToCloseTimeout: 5 * time.Minute, + RetryPolicy: retrypolicy, } ctx = workflow.WithActivityOptions(ctx, options) @@ -70,7 +80,7 @@ func UpdateSSHKeyGroupInventory(ctx workflow.Context, siteID string, sshKeyGroup // Record latency for this inventory call var inventoryMetricsManager cwm.ManageInventoryMetrics - serr := workflow.ExecuteActivity(ctx, inventoryMetricsManager.RecordLatency, parsedSiteID, "UpdateSSHKeyGroupInventory", err != nil, time.Since(startTime)).Get(ctx, nil) + serr := workflow.ExecuteActivity(ctx, inventoryMetricsManager.RecordLatency, parsedSiteID, "UpdateSSHKeyGroupInventory", err != nil, workflow.Now(ctx).Sub(startTime)).Get(ctx, nil) if serr != nil { logger.Warn().Err(serr).Msg("failed to execute activity: RecordLatency") } diff --git a/rest-api/workflow/pkg/workflow/subnet/update.go b/rest-api/workflow/pkg/workflow/subnet/update.go index 5f006605ed..6b857afbe0 100644 --- a/rest-api/workflow/pkg/workflow/subnet/update.go +++ b/rest-api/workflow/pkg/workflow/subnet/update.go @@ -5,14 +5,13 @@ package subnet import ( "fmt" - "time" + cwi "github.com/NVIDIA/infra-controller/rest-api/workflow/internal/inventory" cwm "github.com/NVIDIA/infra-controller/rest-api/workflow/internal/metrics" "github.com/google/uuid" "github.com/rs/zerolog/log" - "go.temporal.io/sdk/temporal" "go.temporal.io/sdk/workflow" subnetActivity "github.com/NVIDIA/infra-controller/rest-api/workflow/pkg/activity/subnet" @@ -24,7 +23,7 @@ import ( func UpdateSubnetInventory(ctx workflow.Context, siteID string, subnetInventory *corev1.SubnetInventory) (err error) { logger := log.With().Str("Workflow", "UpdateSubnetInventory").Str("Site ID", siteID).Logger() - startTime := time.Now() + startTime := workflow.Now(ctx) logger.Info().Msg("starting workflow") @@ -34,19 +33,7 @@ func UpdateSubnetInventory(ctx workflow.Context, siteID string, subnetInventory return err } - // RetryPolicy specifies how to automatically handle retries if an Activity fails. - retrypolicy := &temporal.RetryPolicy{ - InitialInterval: 5 * time.Second, - BackoffCoefficient: 2.0, - MaximumInterval: 30 * time.Second, - MaximumAttempts: 2, - } - options := workflow.ActivityOptions{ - // Timeout options specify when to automatically timeout Activity functions. - StartToCloseTimeout: 30 * time.Second, - // Optionally provide a customized RetryPolicy. - RetryPolicy: retrypolicy, - } + options := cwi.ActivityOptions() ctx = workflow.WithActivityOptions(ctx, options) @@ -69,7 +56,7 @@ func UpdateSubnetInventory(ctx workflow.Context, siteID string, subnetInventory // Record latency for this inventory call var inventoryMetricsManager cwm.ManageInventoryMetrics - serr = workflow.ExecuteActivity(ctx, inventoryMetricsManager.RecordLatency, parsedSiteID, "UpdateSubnetInventory", err != nil, time.Since(startTime)).Get(ctx, nil) + serr = workflow.ExecuteActivity(ctx, inventoryMetricsManager.RecordLatency, parsedSiteID, "UpdateSubnetInventory", err != nil, workflow.Now(ctx).Sub(startTime)).Get(ctx, nil) if serr != nil { logger.Warn().Err(serr).Msg("failed to execute activity: RecordLatency") } diff --git a/rest-api/workflow/pkg/workflow/tenant/update.go b/rest-api/workflow/pkg/workflow/tenant/update.go index ec020d08b3..789cc5013e 100644 --- a/rest-api/workflow/pkg/workflow/tenant/update.go +++ b/rest-api/workflow/pkg/workflow/tenant/update.go @@ -5,14 +5,13 @@ package tenant import ( "fmt" - "time" + cwi "github.com/NVIDIA/infra-controller/rest-api/workflow/internal/inventory" cwm "github.com/NVIDIA/infra-controller/rest-api/workflow/internal/metrics" "github.com/google/uuid" "github.com/rs/zerolog/log" - "go.temporal.io/sdk/temporal" "go.temporal.io/sdk/workflow" tenantActivity "github.com/NVIDIA/infra-controller/rest-api/workflow/pkg/activity/tenant" @@ -24,7 +23,7 @@ import ( func UpdateTenantInventory(ctx workflow.Context, siteID string, tenantInventory *corev1.TenantInventory) (err error) { logger := log.With().Str("Workflow", "UpdateTenantInventory").Str("Site ID", siteID).Logger() - startTime := time.Now() + startTime := workflow.Now(ctx) logger.Info().Msg("starting workflow") @@ -34,19 +33,7 @@ func UpdateTenantInventory(ctx workflow.Context, siteID string, tenantInventory return err } - // RetryPolicy specifies how to automatically handle retries if an Activity fails. - retrypolicy := &temporal.RetryPolicy{ - InitialInterval: 5 * time.Second, - BackoffCoefficient: 2.0, - MaximumInterval: 30 * time.Second, - MaximumAttempts: 2, - } - options := workflow.ActivityOptions{ - // Timeout options specify when to automatically timeout Activity functions. - StartToCloseTimeout: 30 * time.Second, - // Optionally provide a customized RetryPolicy. - RetryPolicy: retrypolicy, - } + options := cwi.ActivityOptions() ctx = workflow.WithActivityOptions(ctx, options) @@ -60,7 +47,7 @@ func UpdateTenantInventory(ctx workflow.Context, siteID string, tenantInventory // Record latency for this inventory call var inventoryMetricsManager cwm.ManageInventoryMetrics - serr := workflow.ExecuteActivity(ctx, inventoryMetricsManager.RecordLatency, parsedSiteID, "UpdateTenantInventory", err != nil, time.Since(startTime)).Get(ctx, nil) + serr := workflow.ExecuteActivity(ctx, inventoryMetricsManager.RecordLatency, parsedSiteID, "UpdateTenantInventory", err != nil, workflow.Now(ctx).Sub(startTime)).Get(ctx, nil) if serr != nil { logger.Warn().Err(serr).Msg("failed to execute activity: RecordLatency") } diff --git a/rest-api/workflow/pkg/workflow/vpc/update.go b/rest-api/workflow/pkg/workflow/vpc/update.go index 15a662c2d8..602ecccf0e 100644 --- a/rest-api/workflow/pkg/workflow/vpc/update.go +++ b/rest-api/workflow/pkg/workflow/vpc/update.go @@ -5,14 +5,13 @@ package vpc import ( "fmt" - "time" "github.com/google/uuid" "github.com/rs/zerolog/log" - "go.temporal.io/sdk/temporal" "go.temporal.io/sdk/workflow" + cwi "github.com/NVIDIA/infra-controller/rest-api/workflow/internal/inventory" cwm "github.com/NVIDIA/infra-controller/rest-api/workflow/internal/metrics" vpcActivity "github.com/NVIDIA/infra-controller/rest-api/workflow/pkg/activity/vpc" @@ -23,7 +22,7 @@ import ( func UpdateVpcInventory(ctx workflow.Context, siteID string, vpcInventory *corev1.VPCInventory) (err error) { logger := log.With().Str("Workflow", "UpdateVpcInventory").Str("Site ID", siteID).Logger() - startTime := time.Now() + startTime := workflow.Now(ctx) logger.Info().Msg("starting workflow") @@ -33,19 +32,7 @@ func UpdateVpcInventory(ctx workflow.Context, siteID string, vpcInventory *corev return err } - // RetryPolicy specifies how to automatically handle retries if an Activity fails. - retrypolicy := &temporal.RetryPolicy{ - InitialInterval: 5 * time.Second, - BackoffCoefficient: 2.0, - MaximumInterval: 30 * time.Second, - MaximumAttempts: 2, - } - options := workflow.ActivityOptions{ - // Timeout options specify when to automatically timeout Activity functions. - StartToCloseTimeout: 30 * time.Second, - // Optionally provide a customized RetryPolicy. - RetryPolicy: retrypolicy, - } + options := cwi.ActivityOptions() ctx = workflow.WithActivityOptions(ctx, options) @@ -68,7 +55,7 @@ func UpdateVpcInventory(ctx workflow.Context, siteID string, vpcInventory *corev // Record latency for this inventory call var inventoryMetricsManager cwm.ManageInventoryMetrics - serr = workflow.ExecuteActivity(ctx, inventoryMetricsManager.RecordLatency, parsedSiteID, "UpdateVpcInventory", err != nil, time.Since(startTime)).Get(ctx, nil) + serr = workflow.ExecuteActivity(ctx, inventoryMetricsManager.RecordLatency, parsedSiteID, "UpdateVpcInventory", err != nil, workflow.Now(ctx).Sub(startTime)).Get(ctx, nil) if serr != nil { logger.Warn().Err(serr).Msg("failed to execute activity: RecordLatency") } diff --git a/rest-api/workflow/pkg/workflow/vpcpeering/update.go b/rest-api/workflow/pkg/workflow/vpcpeering/update.go index 5b0ccc1845..40f8ae1a3e 100644 --- a/rest-api/workflow/pkg/workflow/vpcpeering/update.go +++ b/rest-api/workflow/pkg/workflow/vpcpeering/update.go @@ -5,13 +5,12 @@ package vpcpeering import ( "fmt" - "time" + cwi "github.com/NVIDIA/infra-controller/rest-api/workflow/internal/inventory" cwm "github.com/NVIDIA/infra-controller/rest-api/workflow/internal/metrics" "github.com/google/uuid" "github.com/rs/zerolog/log" - "go.temporal.io/sdk/temporal" "go.temporal.io/sdk/workflow" vpcPeeringActivity "github.com/NVIDIA/infra-controller/rest-api/workflow/pkg/activity/vpcpeering" @@ -23,7 +22,7 @@ import ( func UpdateVpcPeeringInventory(ctx workflow.Context, siteID string, vpcPeeringInventory *corev1.VPCPeeringInventory) error { logger := log.With().Str("Workflow", "UpdateVpcPeeringInventory").Str("Site ID", siteID).Logger() - startTime := time.Now() + startTime := workflow.Now(ctx) logger.Info().Msg("starting workflow") @@ -33,19 +32,7 @@ func UpdateVpcPeeringInventory(ctx workflow.Context, siteID string, vpcPeeringIn return err } - // RetryPolicy specifies how to automatically handle retries if an Activity fails. - retrypolicy := &temporal.RetryPolicy{ - InitialInterval: 5 * time.Second, - BackoffCoefficient: 2.0, - MaximumInterval: 30 * time.Second, - MaximumAttempts: 2, - } - options := workflow.ActivityOptions{ - // Timeout options specify when to automatically timeout Activity functions. - StartToCloseTimeout: 30 * time.Second, - // Optionally provide a customized RetryPolicy. - RetryPolicy: retrypolicy, - } + options := cwi.ActivityOptions() ctx = workflow.WithActivityOptions(ctx, options) @@ -59,7 +46,7 @@ func UpdateVpcPeeringInventory(ctx workflow.Context, siteID string, vpcPeeringIn // Record latency for this inventory call var inventoryMetricsManager cwm.ManageInventoryMetrics - serr := workflow.ExecuteActivity(ctx, inventoryMetricsManager.RecordLatency, parsedSiteID, "UpdateVpcPeeringInventory", err != nil, time.Since(startTime)).Get(ctx, nil) + serr := workflow.ExecuteActivity(ctx, inventoryMetricsManager.RecordLatency, parsedSiteID, "UpdateVpcPeeringInventory", err != nil, workflow.Now(ctx).Sub(startTime)).Get(ctx, nil) if serr != nil { logger.Warn().Err(serr).Msg("failed to execute activity: RecordLatency") } diff --git a/rest-api/workflow/pkg/workflow/vpcprefix/update.go b/rest-api/workflow/pkg/workflow/vpcprefix/update.go index 5c68e0e2d5..e6262a33ac 100644 --- a/rest-api/workflow/pkg/workflow/vpcprefix/update.go +++ b/rest-api/workflow/pkg/workflow/vpcprefix/update.go @@ -5,14 +5,13 @@ package vpcprefix import ( "fmt" - "time" + cwi "github.com/NVIDIA/infra-controller/rest-api/workflow/internal/inventory" cwm "github.com/NVIDIA/infra-controller/rest-api/workflow/internal/metrics" "github.com/google/uuid" "github.com/rs/zerolog/log" - "go.temporal.io/sdk/temporal" "go.temporal.io/sdk/workflow" corev1 "github.com/NVIDIA/infra-controller/rest-api/proto/core/gen/v1" @@ -25,7 +24,7 @@ func UpdateVpcPrefixInventory(ctx workflow.Context, siteID string, vpcPrefixInve logger.Info().Msg("starting workflow") - startTime := time.Now() + startTime := workflow.Now(ctx) parsedSiteID, err := uuid.Parse(siteID) if err != nil { @@ -33,20 +32,7 @@ func UpdateVpcPrefixInventory(ctx workflow.Context, siteID string, vpcPrefixInve return err } - // RetryPolicy specifies how to automatically handle retries if an Activity fails. - retrypolicy := &temporal.RetryPolicy{ - InitialInterval: 5 * time.Second, - BackoffCoefficient: 2.0, - MaximumInterval: 30 * time.Second, - MaximumAttempts: 2, - } - - options := workflow.ActivityOptions{ - // Timeout options specify when to automatically timeout Activity functions. - StartToCloseTimeout: 30 * time.Second, - // Optionally provide a customized RetryPolicy. - RetryPolicy: retrypolicy, - } + options := cwi.ActivityOptions() ctx = workflow.WithActivityOptions(ctx, options) @@ -60,7 +46,7 @@ func UpdateVpcPrefixInventory(ctx workflow.Context, siteID string, vpcPrefixInve // Record latency for this inventory call var inventoryMetricsManager cwm.ManageInventoryMetrics - serr := workflow.ExecuteActivity(ctx, inventoryMetricsManager.RecordLatency, parsedSiteID, "UpdateVpcPrefixInventory", err != nil, time.Since(startTime)).Get(ctx, nil) + serr := workflow.ExecuteActivity(ctx, inventoryMetricsManager.RecordLatency, parsedSiteID, "UpdateVpcPrefixInventory", err != nil, workflow.Now(ctx).Sub(startTime)).Get(ctx, nil) if serr != nil { logger.Warn().Err(serr).Msg("failed to execute activity: RecordLatency") }