Add prometheus - #5
Conversation
| fn() | ||
| } | ||
| log.Printf("initiating graceful shutdown of http server at %s", s.listener.Addr().String()) | ||
| if s.listener != nil { |
There was a problem hiding this comment.
Normal shutdown can terminate the process here.
I reproduced this against f19a29a with a focused package test: Run() starts Serve() in a goroutine, then Shutdown() manually closes s.listener before calling s.server.Shutdown(...). That makes Serve() return:
accept tcp 127.0.0.1:...: use of closed network connection
That error is not http.ErrServerClosed, so the goroutine reaches log.Fatalf("failed to serve: %v", err) and exits the process during a normal lifecycle shutdown.
The same path also ignores the caller-provided shutdown context by calling s.server.Shutdown(context.Background()).
Minimal fix is to let http.Server.Shutdown(ctx) close the listener and honor the provided context:
func (s *RunnableImpl) Shutdown(ctx context.Context) error {
for _, fn := range s.beforeShutdownFns {
fn()
}
log.Printf("initiating graceful shutdown of http server at %s", s.listener.Addr().String())
if err := s.server.Shutdown(ctx); err != nil {
return err
}
log.Printf("http server gracefully stopped")
return nil
}Focused verification:
- current PR code fails with
failed to serve: accept tcp ...: use of closed network connection - the change above passes
go test ./internal/common/http/server
go test ./... is currently blocked by missing generated proto/pb, so I scoped verification to the affected HTTP package.
There was a problem hiding this comment.
Thank you for the feedback. I have pushed another commit to address the comment. Would be grateful if you could review it again.
Thanks again
This PR adds Prometheus metrics to the remote signer to have monitoring of this components
Metrics include