Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ffapi] Renaming Metrics Server to Monitoring and Allowing Additional Routes #167

Merged
merged 2 commits into from
Feb 17, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 26 additions & 17 deletions pkg/ffapi/apiserver.go
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright © 2024 Kaleido, Inc.
// Copyright © 2025 Kaleido, Inc.
//
// SPDX-License-Identifier: Apache-2.0
//
Expand Down Expand Up @@ -61,9 +61,9 @@ type apiServer[T any] struct {
apiDynamicPublicURLHeader string
alwaysPaginate bool
handleYAML bool
metricsEnabled bool
monitoringEnabled bool
metricsPath string
metricsPublicURL string
monitoringPublicURL string
mux *mux.Router

APIServerOptions[T]
Expand All @@ -72,10 +72,11 @@ type apiServer[T any] struct {
type APIServerOptions[T any] struct {
MetricsRegistry metric.MetricsRegistry
Routes []*Route
MonitoringRoutes []*Route
EnrichRequest func(r *APIRequest) (T, error)
Description string
APIConfig config.Section
MetricsConfig config.Section
MonitoringConfig config.Section
CORSConfig config.Section
FavIcon16 []byte
FavIcon32 []byte
Expand All @@ -99,8 +100,8 @@ func NewAPIServer[T any](ctx context.Context, options APIServerOptions[T]) APISe
maxFilterSkip: options.APIConfig.GetUint64(ConfAPIMaxFilterSkip),
requestTimeout: options.APIConfig.GetDuration(ConfAPIRequestTimeout),
requestMaxTimeout: options.APIConfig.GetDuration(ConfAPIRequestMaxTimeout),
metricsEnabled: options.MetricsConfig.GetBool(ConfMetricsServerEnabled),
metricsPath: options.MetricsConfig.GetString(ConfMetricsServerPath),
monitoringEnabled: options.MonitoringConfig.GetBool(ConfMonitoringServerEnabled),
metricsPath: options.MonitoringConfig.GetString(ConfMonitoringServerMetricsPath),
alwaysPaginate: options.APIConfig.GetBool(ConfAPIAlwaysPaginate),
handleYAML: options.HandleYAML,
apiDynamicPublicURLHeader: options.APIConfig.GetString(ConfAPIDynamicPublicURLHeader),
Expand Down Expand Up @@ -142,7 +143,7 @@ func (as *apiServer[T]) Serve(ctx context.Context) (err error) {
}()

httpErrChan := make(chan error)
metricsErrChan := make(chan error)
monitoringErrChan := make(chan error)

apiHTTPServer, err := httpserver.NewHTTPServer(ctx, "api", as.MuxRouter(ctx), httpErrChan, as.APIConfig, as.CORSConfig, &httpserver.ServerOptions{
MaximumRequestTimeout: as.requestMaxTimeout,
Expand All @@ -153,20 +154,20 @@ func (as *apiServer[T]) Serve(ctx context.Context) (err error) {
as.apiPublicURL = buildPublicURL(as.APIConfig, apiHTTPServer.Addr())
go apiHTTPServer.ServeHTTP(ctx)

if as.metricsEnabled {
metricsHTTPServer, err := httpserver.NewHTTPServer(ctx, "metrics", as.createMetricsMuxRouter(ctx), metricsErrChan, as.MetricsConfig, as.CORSConfig, &httpserver.ServerOptions{
if as.monitoringEnabled {
monitoringHTTPServer, err := httpserver.NewHTTPServer(ctx, "monitoring", as.createMonitoringMuxRouter(ctx), monitoringErrChan, as.MonitoringConfig, as.CORSConfig, &httpserver.ServerOptions{
MaximumRequestTimeout: as.requestMaxTimeout,
})
if err != nil {
return err
}
as.metricsPublicURL = buildPublicURL(as.MetricsConfig, apiHTTPServer.Addr())
go metricsHTTPServer.ServeHTTP(ctx)
as.monitoringPublicURL = buildPublicURL(as.MonitoringConfig, apiHTTPServer.Addr())
go monitoringHTTPServer.ServeHTTP(ctx)
}

started = true
close(as.started)
return as.waitForServerStop(httpErrChan, metricsErrChan)
return as.waitForServerStop(httpErrChan, monitoringErrChan)
}

func (as *apiServer[T]) Started() <-chan struct{} {
Expand All @@ -177,11 +178,11 @@ func (as *apiServer[T]) APIPublicURL() string {
return as.apiPublicURL
}

func (as *apiServer[T]) waitForServerStop(httpErrChan, metricsErrChan chan error) error {
func (as *apiServer[T]) waitForServerStop(httpErrChan, monitoringErrChan chan error) error {
select {
case err := <-httpErrChan:
return err
case err := <-metricsErrChan:
case err := <-monitoringErrChan:
return err
}
}
Expand Down Expand Up @@ -242,7 +243,7 @@ func (as *apiServer[T]) createMuxRouter(ctx context.Context) *mux.Router {
r := mux.NewRouter().UseEncodedPath()
hf := as.handlerFactory()

if as.metricsEnabled {
if as.monitoringEnabled {
h, _ := as.MetricsRegistry.GetHTTPMetricsInstrumentationsMiddlewareForSubsystem(ctx, APIServerMetricsSubSystemName)
r.Use(h)
}
Expand Down Expand Up @@ -293,12 +294,20 @@ func (as *apiServer[T]) notFoundHandler(res http.ResponseWriter, req *http.Reque
return 404, i18n.NewError(req.Context(), i18n.Msg404NotFound)
}

func (as *apiServer[T]) createMetricsMuxRouter(ctx context.Context) *mux.Router {
r := mux.NewRouter()
func (as *apiServer[T]) createMonitoringMuxRouter(ctx context.Context) *mux.Router {
r := mux.NewRouter().UseEncodedPath()
hf := as.handlerFactory() // TODO separate factory for monitoring ??

h, err := as.MetricsRegistry.HTTPHandler(ctx, promhttp.HandlerOpts{})
if err != nil {
panic(err)
}
r.Path(as.metricsPath).Handler(h)

for _, route := range as.MonitoringRoutes {
r.HandleFunc(route.Path, as.routeHandler(hf, route)).Methods(route.Method)
}

r.NotFoundHandler = hf.APIWrapper(as.notFoundHandler)
return r
}
14 changes: 7 additions & 7 deletions pkg/ffapi/apiserver_config.go
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright © 2023 Kaleido, Inc.
// Copyright © 2025 Kaleido, Inc.
//
// SPDX-License-Identifier: Apache-2.0
//
Expand All @@ -22,8 +22,8 @@ import (
)

var (
ConfMetricsServerEnabled = "enabled"
ConfMetricsServerPath = "/metrics"
ConfMonitoringServerEnabled = "enabled"
ConfMonitoringServerMetricsPath = "metricsPath"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was just flat out bug


ConfAPIDefaultFilterLimit = "defaultFilterLimit"
ConfAPIMaxFilterLimit = "maxFilterLimit"
Expand All @@ -34,7 +34,7 @@ var (
ConfAPIDynamicPublicURLHeader = "dynamicPublicURLHeader"
)

func InitAPIServerConfig(apiConfig, metricsConfig, corsConfig config.Section) {
func InitAPIServerConfig(apiConfig, monitoringConfig, corsConfig config.Section) {
httpserver.InitHTTPConfig(apiConfig, 5000)
apiConfig.AddKnownKey(ConfAPIDefaultFilterLimit, 25)
apiConfig.AddKnownKey(ConfAPIMaxFilterLimit, 100)
Expand All @@ -46,7 +46,7 @@ func InitAPIServerConfig(apiConfig, metricsConfig, corsConfig config.Section) {

httpserver.InitCORSConfig(corsConfig)

httpserver.InitHTTPConfig(metricsConfig, 6000)
metricsConfig.AddKnownKey(ConfMetricsServerEnabled, true)
metricsConfig.AddKnownKey(ConfMetricsServerPath, "/metrics")
httpserver.InitHTTPConfig(monitoringConfig, 6000)
monitoringConfig.AddKnownKey(ConfMonitoringServerEnabled, true)
monitoringConfig.AddKnownKey(ConfMonitoringServerMetricsPath, "/metrics")
}
32 changes: 16 additions & 16 deletions pkg/ffapi/apiserver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -114,17 +114,17 @@ var utAPIRoute2 = &Route{
func initUTConfig() (config.Section, config.Section, config.Section) {
config.RootConfigReset()
apiConfig := config.RootSection("ut.api")
metricsConfig := config.RootSection("ut.metrics")
monitoringConfig := config.RootSection("ut.monitoringConfig")
corsConfig := config.RootSection("ut.cors")
InitAPIServerConfig(apiConfig, metricsConfig, corsConfig)
InitAPIServerConfig(apiConfig, monitoringConfig, corsConfig)
apiConfig.Set(httpserver.HTTPConfPort, 0)
metricsConfig.Set(httpserver.HTTPConfPort, 0)
return apiConfig, metricsConfig, corsConfig
monitoringConfig.Set(httpserver.HTTPConfPort, 0)
return apiConfig, monitoringConfig, corsConfig
}

func newTestAPIServer(t *testing.T, start bool) (*utManager, *apiServer[*utManager], func()) {
ctx, cancelCtx := context.WithCancel(context.Background())
apiConfig, metricsConfig, corsConfig := initUTConfig()
apiConfig, monitoringConfig, corsConfig := initUTConfig()
um := &utManager{t: t}
as := NewAPIServer(ctx, APIServerOptions[*utManager]{
MetricsRegistry: metric.NewPrometheusMetricsRegistry("ut"),
Expand All @@ -135,10 +135,10 @@ func newTestAPIServer(t *testing.T, start bool) (*utManager, *apiServer[*utManag
// request and that's the "T" on the APIServer
return um, um.mockEnrichErr
},
Description: "unit testing",
APIConfig: apiConfig,
MetricsConfig: metricsConfig,
CORSConfig: corsConfig,
Description: "unit testing",
APIConfig: apiConfig,
MonitoringConfig: monitoringConfig,
CORSConfig: corsConfig,
})
done := make(chan struct{})
if start {
Expand Down Expand Up @@ -382,11 +382,11 @@ func TestAPIServerFailServe(t *testing.T) {

}

func TestAPIServerFailServeMetrics(t *testing.T) {
func TestAPIServerFailServeMonitoring(t *testing.T) {
_, as, done := newTestAPIServer(t, false)
defer done()

as.MetricsConfig.Set(httpserver.HTTPConfAddress, "!badness")
as.MonitoringConfig.Set(httpserver.HTTPConfAddress, "!badness")
err := as.Serve(context.Background())
assert.Regexp(t, "FF00151", err)

Expand All @@ -408,15 +408,15 @@ func TestWaitForServerStop(t *testing.T) {
}

func TestBadRoute(t *testing.T) {
apiConfig, metricsConfig, corsConfig := initUTConfig()
apiConfig, monitoringConfig, corsConfig := initUTConfig()
as := NewAPIServer(context.Background(), APIServerOptions[*utManager]{
MetricsRegistry: metric.NewPrometheusMetricsRegistry("ut"),
Routes: []*Route{{
Extensions: &APIServerRouteExt[string]{}, // T does not match *utManager
}},
APIConfig: apiConfig,
MetricsConfig: metricsConfig,
CORSConfig: corsConfig,
APIConfig: apiConfig,
MonitoringConfig: monitoringConfig,
CORSConfig: corsConfig,
})
assert.Panics(t, func() { as.Serve(context.Background()) })
}
Expand All @@ -425,5 +425,5 @@ func TestBadMetrics(t *testing.T) {
_, as, done := newTestAPIServer(t, false)
defer done()
as.MetricsRegistry = metric.NewPrometheusMetricsRegistry("wrong")
assert.Panics(t, func() { as.createMetricsMuxRouter(context.Background()) })
assert.Panics(t, func() { as.createMonitoringMuxRouter(context.Background()) })
}