Skip to content

Commit

Permalink
chore!: adopt log/slog, drop go-kit/log (#4089)
Browse files Browse the repository at this point in the history
* chore!: adopt log/slog, drop go-kit/log

The bulk of this change set was automated by the following script which
is being used to aid in converting the various exporters/projects to use
slog:

https://gist.github.com/tjhop/49f96fb7ebbe55b12deee0b0312d8434

This commit includes several changes:
- bump exporter-tookit to v0.13.1 for log/slog support
- updates golangci-lint deprecated configs
- enables sloglint linter
- removes old go-kit/log linter configs
- introduce some `if logger == nil { $newLogger }` additions to prevent
  nil references
- converts cluster membership config to use a stdlib compatible slog
  adapter, rather than creating a custom io.Writer for use as the
membership `logOutput` config

Signed-off-by: TJ Hoplock <[email protected]>

* chore: address PR feedback

Signed-off-by: TJ Hoplock <[email protected]>

---------

Signed-off-by: TJ Hoplock <[email protected]>
  • Loading branch information
tjhop authored Nov 6, 2024
1 parent 82e804f commit f6b942c
Show file tree
Hide file tree
Showing 73 changed files with 612 additions and 646 deletions.
13 changes: 5 additions & 8 deletions .golangci.yml
Original file line number Diff line number Diff line change
@@ -1,9 +1,3 @@
run:
skip-files:
# Skip autogenerated files.
- ^.*\.(pb|y)\.go$
timeout: 5m

output:
sort-results: true

Expand All @@ -17,6 +11,7 @@ linters:
- misspell
- revive
- testifylint
- sloglint

issues:
max-issues-per-linter: 0
Expand All @@ -25,6 +20,10 @@ issues:
- path: _test.go
linters:
- errcheck
exclude-files:
# Skip autogenerated files.
- ^.*\.(pb|y)\.go$
timeout: 5m

linters-settings:
depguard:
Expand All @@ -48,8 +47,6 @@ linters-settings:
- (net/http.ResponseWriter).Write
# No need to check for errors on server's shutdown.
- (*net/http.Server).Shutdown
# Never check for logger errors.
- (github.com/go-kit/log.Logger).Log
# Never check for rollback errors as Rollback() is called when a previous error was detected.
- (github.com/prometheus/prometheus/storage.Appender).Rollback
godot:
Expand Down
11 changes: 6 additions & 5 deletions api/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,14 @@ package api
import (
"errors"
"fmt"
"log/slog"
"net/http"
"runtime"
"time"

"github.com/go-kit/log"
"github.com/prometheus/client_golang/prometheus"
"github.com/prometheus/common/model"
"github.com/prometheus/common/promslog"
"github.com/prometheus/common/route"

apiv2 "github.com/prometheus/alertmanager/api/v2"
Expand Down Expand Up @@ -70,7 +71,7 @@ type Options struct {
// the concurrency limit.
Concurrency int
// Logger is used for logging, if nil, no logging will happen.
Logger log.Logger
Logger *slog.Logger
// Registry is used to register Prometheus metrics. If nil, no metrics
// registration will happen.
Registry prometheus.Registerer
Expand Down Expand Up @@ -107,7 +108,7 @@ func New(opts Options) (*API, error) {
}
l := opts.Logger
if l == nil {
l = log.NewNopLogger()
l = promslog.NewNopLogger()
}
concurrency := opts.Concurrency
if concurrency < 1 {
Expand All @@ -124,7 +125,7 @@ func New(opts Options) (*API, error) {
opts.GroupMutedFunc,
opts.Silences,
opts.Peer,
log.With(l, "version", "v2"),
l.With("version", "v2"),
opts.Registry,
)
if err != nil {
Expand Down Expand Up @@ -153,7 +154,7 @@ func New(opts Options) (*API, error) {
}

return &API{
deprecationRouter: NewV1DeprecationRouter(log.With(l, "version", "v1")),
deprecationRouter: NewV1DeprecationRouter(l.With("version", "v1")),
v2: v2,
requestsInFlight: requestsInFlight,
concurrencyLimitExceeded: concurrencyLimitExceeded,
Expand Down
11 changes: 5 additions & 6 deletions api/v1_deprecation_router.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,20 +13,19 @@ package api

import (
"encoding/json"
"log/slog"
"net/http"

"github.com/go-kit/log"
"github.com/go-kit/log/level"
"github.com/prometheus/common/route"
)

// V1DeprecationRouter is the router to signal v1 users that the API v1 is now removed.
type V1DeprecationRouter struct {
logger log.Logger
logger *slog.Logger
}

// NewV1DeprecationRouter returns a new V1DeprecationRouter.
func NewV1DeprecationRouter(l log.Logger) *V1DeprecationRouter {
func NewV1DeprecationRouter(l *slog.Logger) *V1DeprecationRouter {
return &V1DeprecationRouter{
logger: l,
}
Expand All @@ -47,7 +46,7 @@ func (dr *V1DeprecationRouter) Register(r *route.Router) {
}

func (dr *V1DeprecationRouter) deprecationHandler(w http.ResponseWriter, req *http.Request) {
level.Warn(dr.logger).Log("msg", "v1 API received a request on a removed endpoint", "path", req.URL.Path, "method", req.Method)
dr.logger.Warn("v1 API received a request on a removed endpoint", "path", req.URL.Path, "method", req.Method)

resp := struct {
Status string `json:"status"`
Expand All @@ -61,6 +60,6 @@ func (dr *V1DeprecationRouter) deprecationHandler(w http.ResponseWriter, req *ht
w.WriteHeader(410)

if err := json.NewEncoder(w).Encode(resp); err != nil {
level.Error(dr.logger).Log("msg", "failed to write response", "err", err)
dr.logger.Error("failed to write response", "err", err)
}
}
47 changes: 23 additions & 24 deletions api/v2/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,13 @@ package v2
import (
"errors"
"fmt"
"log/slog"
"net/http"
"regexp"
"sort"
"sync"
"time"

"github.com/go-kit/log"
"github.com/go-kit/log/level"
"github.com/go-openapi/analysis"
"github.com/go-openapi/loads"
"github.com/go-openapi/runtime/middleware"
Expand Down Expand Up @@ -71,7 +70,7 @@ type API struct {
route *dispatch.Route
setAlertStatus setAlertStatusFn

logger log.Logger
logger *slog.Logger
m *metrics.Alerts

Handler http.Handler
Expand All @@ -92,7 +91,7 @@ func NewAPI(
gmf groupMutedFunc,
silences *silence.Silences,
peer cluster.ClusterPeer,
l log.Logger,
l *slog.Logger,
r prometheus.Registerer,
) (*API, error) {
api := API{
Expand Down Expand Up @@ -154,8 +153,8 @@ func setResponseHeaders(h http.Handler) http.Handler {
})
}

func (api *API) requestLogger(req *http.Request) log.Logger {
return log.With(api.logger, "path", req.URL.Path, "method", req.Method)
func (api *API) requestLogger(req *http.Request) *slog.Logger {
return api.logger.With("path", req.URL.Path, "method", req.Method)
}

// Update sets the API struct members that may change between reloads of alertmanager.
Expand Down Expand Up @@ -249,14 +248,14 @@ func (api *API) getAlertsHandler(params alert_ops.GetAlertsParams) middleware.Re

matchers, err := parseFilter(params.Filter)
if err != nil {
level.Debug(logger).Log("msg", "Failed to parse matchers", "err", err)
logger.Debug("Failed to parse matchers", "err", err)
return alertgroup_ops.NewGetAlertGroupsBadRequest().WithPayload(err.Error())
}

if params.Receiver != nil {
receiverFilter, err = regexp.Compile("^(?:" + *params.Receiver + ")$")
if err != nil {
level.Debug(logger).Log("msg", "Failed to compile receiver regex", "err", err)
logger.Debug("Failed to compile receiver regex", "err", err)
return alert_ops.
NewGetAlertsBadRequest().
WithPayload(
Expand Down Expand Up @@ -301,7 +300,7 @@ func (api *API) getAlertsHandler(params alert_ops.GetAlertsParams) middleware.Re
api.mtx.RUnlock()

if err != nil {
level.Error(logger).Log("msg", "Failed to get alerts", "err", err)
logger.Error("Failed to get alerts", "err", err)
return alert_ops.NewGetAlertsInternalServerError().WithPayload(err.Error())
}
sort.Slice(res, func(i, j int) bool {
Expand Down Expand Up @@ -361,12 +360,12 @@ func (api *API) postAlertsHandler(params alert_ops.PostAlertsParams) middleware.
validAlerts = append(validAlerts, a)
}
if err := api.alerts.Put(validAlerts...); err != nil {
level.Error(logger).Log("msg", "Failed to create alerts", "err", err)
logger.Error("Failed to create alerts", "err", err)
return alert_ops.NewPostAlertsInternalServerError().WithPayload(err.Error())
}

if validationErrs.Len() > 0 {
level.Error(logger).Log("msg", "Failed to validate alerts", "err", validationErrs.Error())
logger.Error("Failed to validate alerts", "err", validationErrs.Error())
return alert_ops.NewPostAlertsBadRequest().WithPayload(validationErrs.Error())
}

Expand All @@ -378,15 +377,15 @@ func (api *API) getAlertGroupsHandler(params alertgroup_ops.GetAlertGroupsParams

matchers, err := parseFilter(params.Filter)
if err != nil {
level.Debug(logger).Log("msg", "Failed to parse matchers", "err", err)
logger.Debug("Failed to parse matchers", "err", err)
return alertgroup_ops.NewGetAlertGroupsBadRequest().WithPayload(err.Error())
}

var receiverFilter *regexp.Regexp
if params.Receiver != nil {
receiverFilter, err = regexp.Compile("^(?:" + *params.Receiver + ")$")
if err != nil {
level.Error(logger).Log("msg", "Failed to compile receiver regex", "err", err)
logger.Error("Failed to compile receiver regex", "err", err)
return alertgroup_ops.
NewGetAlertGroupsBadRequest().
WithPayload(
Expand Down Expand Up @@ -518,13 +517,13 @@ func (api *API) getSilencesHandler(params silence_ops.GetSilencesParams) middlew

matchers, err := parseFilter(params.Filter)
if err != nil {
level.Debug(logger).Log("msg", "Failed to parse matchers", "err", err)
logger.Debug("Failed to parse matchers", "err", err)
return silence_ops.NewGetSilencesBadRequest().WithPayload(err.Error())
}

psils, _, err := api.silences.Query()
if err != nil {
level.Error(logger).Log("msg", "Failed to get silences", "err", err)
logger.Error("Failed to get silences", "err", err)
return silence_ops.NewGetSilencesInternalServerError().WithPayload(err.Error())
}

Expand All @@ -535,7 +534,7 @@ func (api *API) getSilencesHandler(params silence_ops.GetSilencesParams) middlew
}
silence, err := GettableSilenceFromProto(ps)
if err != nil {
level.Error(logger).Log("msg", "Failed to unmarshal silence from proto", "err", err)
logger.Error("Failed to unmarshal silence from proto", "err", err)
return silence_ops.NewGetSilencesInternalServerError().WithPayload(err.Error())
}
sils = append(sils, &silence)
Expand Down Expand Up @@ -614,18 +613,18 @@ func (api *API) getSilenceHandler(params silence_ops.GetSilenceParams) middlewar

sils, _, err := api.silences.Query(silence.QIDs(params.SilenceID.String()))
if err != nil {
level.Error(logger).Log("msg", "Failed to get silence by id", "err", err, "id", params.SilenceID.String())
logger.Error("Failed to get silence by id", "err", err, "id", params.SilenceID.String())
return silence_ops.NewGetSilenceInternalServerError().WithPayload(err.Error())
}

if len(sils) == 0 {
level.Error(logger).Log("msg", "Failed to find silence", "err", err, "id", params.SilenceID.String())
logger.Error("Failed to find silence", "err", err, "id", params.SilenceID.String())
return silence_ops.NewGetSilenceNotFound()
}

sil, err := GettableSilenceFromProto(sils[0])
if err != nil {
level.Error(logger).Log("msg", "Failed to convert unmarshal from proto", "err", err)
logger.Error("Failed to convert unmarshal from proto", "err", err)
return silence_ops.NewGetSilenceInternalServerError().WithPayload(err.Error())
}

Expand All @@ -637,7 +636,7 @@ func (api *API) deleteSilenceHandler(params silence_ops.DeleteSilenceParams) mid

sid := params.SilenceID.String()
if err := api.silences.Expire(sid); err != nil {
level.Error(logger).Log("msg", "Failed to expire silence", "err", err)
logger.Error("Failed to expire silence", "err", err)
if errors.Is(err, silence.ErrNotFound) {
return silence_ops.NewDeleteSilenceNotFound()
}
Expand All @@ -651,26 +650,26 @@ func (api *API) postSilencesHandler(params silence_ops.PostSilencesParams) middl

sil, err := PostableSilenceToProto(params.Silence)
if err != nil {
level.Error(logger).Log("msg", "Failed to marshal silence to proto", "err", err)
logger.Error("Failed to marshal silence to proto", "err", err)
return silence_ops.NewPostSilencesBadRequest().WithPayload(
fmt.Sprintf("failed to convert API silence to internal silence: %v", err.Error()),
)
}

if sil.StartsAt.After(sil.EndsAt) || sil.StartsAt.Equal(sil.EndsAt) {
msg := "Failed to create silence: start time must be before end time"
level.Error(logger).Log("msg", msg, "starts_at", sil.StartsAt, "ends_at", sil.EndsAt)
logger.Error(msg, "starts_at", sil.StartsAt, "ends_at", sil.EndsAt)
return silence_ops.NewPostSilencesBadRequest().WithPayload(msg)
}

if sil.EndsAt.Before(time.Now()) {
msg := "Failed to create silence: end time can't be in the past"
level.Error(logger).Log("msg", msg, "ends_at", sil.EndsAt)
logger.Error(msg, "ends_at", sil.EndsAt)
return silence_ops.NewPostSilencesBadRequest().WithPayload(msg)
}

if err = api.silences.Set(sil); err != nil {
level.Error(logger).Log("msg", "Failed to create silence", "err", err)
logger.Error("Failed to create silence", "err", err)
if errors.Is(err, silence.ErrNotFound) {
return silence_ops.NewPostSilencesNotFound().WithPayload(err.Error())
}
Expand Down
11 changes: 5 additions & 6 deletions api/v2/api_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import (
"github.com/go-openapi/runtime/middleware"
"github.com/go-openapi/strfmt"
"github.com/prometheus/common/model"
"github.com/prometheus/common/promslog"
"github.com/stretchr/testify/require"

open_api_models "github.com/prometheus/alertmanager/api/v2/models"
Expand All @@ -39,8 +40,6 @@ import (
"github.com/prometheus/alertmanager/silence"
"github.com/prometheus/alertmanager/silence/silencepb"
"github.com/prometheus/alertmanager/types"

"github.com/go-kit/log"
)

// If api.peers == nil, Alertmanager cluster feature is disabled. Make sure to
Expand Down Expand Up @@ -192,7 +191,7 @@ func TestDeleteSilenceHandler(t *testing.T) {
api := API{
uptime: time.Now(),
silences: silences,
logger: log.NewNopLogger(),
logger: promslog.NewNopLogger(),
}

r, err := http.NewRequest("DELETE", "/api/v2/silence/${tc.sid}", nil)
Expand Down Expand Up @@ -274,7 +273,7 @@ func TestPostSilencesHandler(t *testing.T) {
api := API{
uptime: time.Now(),
silences: silences,
logger: log.NewNopLogger(),
logger: promslog.NewNopLogger(),
}

sil := createSilence(t, tc.sid, "silenceCreator", tc.start, tc.end)
Expand All @@ -293,7 +292,7 @@ func TestPostSilencesHandlerMissingIdCreatesSilence(t *testing.T) {
api := API{
uptime: time.Now(),
silences: silences,
logger: log.NewNopLogger(),
logger: promslog.NewNopLogger(),
}

// Create a new silence. It should be assigned a random UUID.
Expand Down Expand Up @@ -557,7 +556,7 @@ receivers:
cfg, _ := config.Load(in)
api := API{
uptime: time.Now(),
logger: log.NewNopLogger(),
logger: promslog.NewNopLogger(),
alertmanagerConfig: cfg,
}

Expand Down
Loading

0 comments on commit f6b942c

Please sign in to comment.