Skip to content

Commit

Permalink
Create and shutdown HTTP server (for metrics) gracefully
Browse files Browse the repository at this point in the history
  • Loading branch information
mostafa committed Sep 24, 2023
1 parent ab1bab5 commit 2b21fd5
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 2 deletions.
23 changes: 21 additions & 2 deletions cmd/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ var (
globalConfigFile string
conf *config.Config
pluginRegistry *plugin.Registry
metricsServer *http.Server

UsageReportURL = "localhost:59091"

Expand All @@ -72,6 +73,7 @@ func StopGracefully(
pluginTimeoutCtx context.Context,
sig os.Signal,
metricsMerger *metrics.Merger,
metricsServer *http.Server,
pluginRegistry *plugin.Registry,
logger zerolog.Logger,
servers map[string]*network.Server,
Expand Down Expand Up @@ -110,6 +112,15 @@ func StopGracefully(
logger.Info().Msg("Stopped metrics merger")
span.AddEvent("Stopped metrics merger")
}
if metricsServer != nil {
if err := metricsServer.Shutdown(context.Background()); err != nil {

Check failure on line 116 in cmd/run.go

View workflow job for this annotation

GitHub Actions / Test GatewayD

Non-inherited new context, use function like `context.WithXXX` instead (contextcheck)
logger.Error().Err(err).Msg("Failed to stop metrics server")
span.RecordError(err)
} else {
logger.Info().Msg("Stopped metrics server")
span.AddEvent("Stopped metrics server")
}
}
for name, server := range servers {
logger.Info().Str("name", name).Msg("Stopping server")
server.Shutdown() //nolint:contextcheck
Expand Down Expand Up @@ -378,6 +389,7 @@ var runCmd = &cobra.Command{
if conf.Plugin.EnableMetricsMerger && metricsMerger != nil {
handler = mergedMetricsHandler(handler)
}

// Check if the metrics server is already running before registering the handler.
if _, err = http.Get(address); err != nil { //nolint:gosec
http.Handle(metricsConfig.Path, gziphandler.GzipHandler(handler))
Expand All @@ -386,9 +398,15 @@ var runCmd = &cobra.Command{
span.RecordError(err)
}

// Create a new metrics server.
metricsServer = &http.Server{

Check failure on line 402 in cmd/run.go

View workflow job for this annotation

GitHub Actions / Test GatewayD

G112: Potential Slowloris Attack because ReadHeaderTimeout is not configured in the http.Server (gosec)
Addr: metricsConfig.Address,
Handler: handler,
}

// Start the metrics server.
//nolint:gosec

Check failure on line 408 in cmd/run.go

View workflow job for this annotation

GitHub Actions / Test GatewayD

directive `//nolint:gosec` is unused for linter "gosec" (nolintlint)
if err = http.ListenAndServe(
metricsConfig.Address, nil); err != nil {
if err = metricsServer.ListenAndServe(); err != http.ErrServerClosed {

Check failure on line 409 in cmd/run.go

View workflow job for this annotation

GitHub Actions / Test GatewayD

comparing with != will fail on wrapped errors. Use errors.Is to check for a specific error (errorlint)
logger.Error().Err(err).Msg("Failed to start metrics server")
span.RecordError(err)
}
Expand Down Expand Up @@ -730,6 +748,7 @@ var runCmd = &cobra.Command{
pluginTimeoutCtx,
sig,
metricsMerger,
metricsServer,
pluginRegistry,
logger,
servers,
Expand Down
2 changes: 2 additions & 0 deletions cmd/run_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ func Test_runCmd(t *testing.T) {
nil,
nil,
nil,
nil,
loggers[config.Default],
servers,
stopChan,
Expand Down Expand Up @@ -115,6 +116,7 @@ func Test_runCmdWithCachePlugin(t *testing.T) {
nil,
nil,
nil,
nil,
loggers[config.Default],
servers,
stopChan,
Expand Down

0 comments on commit 2b21fd5

Please sign in to comment.