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

chore!: adopt log/slog, drop go-kit/log #183

Merged
merged 2 commits into from
Oct 21, 2024
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
4 changes: 2 additions & 2 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,14 @@ orbs:
executors:
golang:
docker:
- image: cimg/go:1.22
- image: cimg/go:1.23
- image: quay.io/prometheus/node-exporter:latest

jobs:
test:
executor: golang
environment:
prom_ver: 2.51.0
prom_ver: 2.54.1
steps:
- prometheus/setup_environment
- run: wget https://github.com/prometheus/prometheus/releases/download/v${prom_ver}/prometheus-${prom_ver}.linux-amd64.tar.gz
Expand Down
13 changes: 7 additions & 6 deletions .github/workflows/golangci-lint.yml
Original file line number Diff line number Diff line change
Expand Up @@ -24,15 +24,16 @@ jobs:
runs-on: ubuntu-latest
steps:
- name: Checkout repository
uses: actions/checkout@9bb56186c3b09b4f86b1c65136769dd318469633 # v4.1.2
- name: install Go
uses: actions/setup-go@0c52d547c9bc32b1aa3301fd7a9cb496313a4491 # v5.0.0
uses: actions/checkout@d632683dd7b4114ad314bca15554477dd762a938 # v4.2.0
- name: Install Go
uses: actions/setup-go@0a12ed9d6a96ab950c8f026ed9f722fe0da7ef32 # v5.0.2
with:
go-version: 1.22.x
go-version: 1.23.x
- name: Install snmp_exporter/generator dependencies
run: sudo apt-get update && sudo apt-get -y install libsnmp-dev
if: github.repository == 'prometheus/snmp_exporter'
- name: Lint
uses: golangci/golangci-lint-action@3cfe3a4abbb849e10058ce4af15d205b6da42804 # v4.0.0
uses: golangci/golangci-lint-action@aaa42aa0628b4ae2578232a66b541047968fac86 # v6.1.0
with:
version: v1.56.2
args: --verbose
version: v1.60.2
3 changes: 1 addition & 2 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ linters:
enable:
- misspell
- revive
- sloglint

linters-settings:
errcheck:
Expand All @@ -11,8 +12,6 @@ linters-settings:
- io.Copy
# Used in HTTP handlers, any error is handled by the server itself.
- (net/http.ResponseWriter).Write
# Never check for logger errors.
- (github.com/go-kit/log.Logger).Log
revive:
rules:
# https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#unused-parameter
Expand Down
2 changes: 1 addition & 1 deletion .promu.yml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
go:
# This must match .circle/config.yml.
version: 1.22
version: 1.23
repository:
path: github.com/prometheus-community/pushprox
build:
Expand Down
10 changes: 8 additions & 2 deletions Makefile.common
Original file line number Diff line number Diff line change
Expand Up @@ -55,13 +55,13 @@ ifneq ($(shell command -v gotestsum 2> /dev/null),)
endif
endif

PROMU_VERSION ?= 0.15.0
PROMU_VERSION ?= 0.17.0
PROMU_URL := https://github.com/prometheus/promu/releases/download/v$(PROMU_VERSION)/promu-$(PROMU_VERSION).$(GO_BUILD_PLATFORM).tar.gz

SKIP_GOLANGCI_LINT :=
GOLANGCI_LINT :=
GOLANGCI_LINT_OPTS ?=
GOLANGCI_LINT_VERSION ?= v1.56.2
GOLANGCI_LINT_VERSION ?= v1.60.2
# golangci-lint only supports linux, darwin and windows platforms on i386/amd64/arm64.
# windows isn't included here because of the path separator being different.
ifeq ($(GOHOSTOS),$(filter $(GOHOSTOS),linux darwin))
Expand Down Expand Up @@ -275,3 +275,9 @@ $(1)_precheck:
exit 1; \
fi
endef

govulncheck: install-govulncheck
govulncheck ./...

install-govulncheck:
command -v govulncheck > /dev/null || go install golang.org/x/vuln/cmd/govulncheck@latest
53 changes: 26 additions & 27 deletions cmd/client/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"errors"
"fmt"
"io"
"log/slog"
"net"
"net/http"
"net/url"
Expand All @@ -32,13 +33,11 @@ import (
"github.com/Showmax/go-fqdn"
"github.com/alecthomas/kingpin/v2"
"github.com/cenkalti/backoff/v4"
"github.com/go-kit/log"
"github.com/go-kit/log/level"
"github.com/prometheus-community/pushprox/util"
"github.com/prometheus/client_golang/prometheus"
"github.com/prometheus/client_golang/prometheus/promhttp"
"github.com/prometheus/common/promlog"
"github.com/prometheus/common/promlog/flag"
"github.com/prometheus/common/promslog"
"github.com/prometheus/common/promslog/flag"
)

var (
Expand Down Expand Up @@ -89,11 +88,11 @@ func newBackOffFromFlags() backoff.BackOff {

// Coordinator for scrape requests and responses
type Coordinator struct {
logger log.Logger
logger *slog.Logger
}

func (c *Coordinator) handleErr(request *http.Request, client *http.Client, err error) {
level.Error(c.logger).Log("err", err)
c.logger.Error("Coordinator error", "error", err)
scrapeErrorCounter.Inc()
resp := &http.Response{
StatusCode: http.StatusInternalServerError,
Expand All @@ -102,14 +101,14 @@ func (c *Coordinator) handleErr(request *http.Request, client *http.Client, err
}
if err = c.doPush(resp, request, client); err != nil {
pushErrorCounter.Inc()
level.Warn(c.logger).Log("msg", "Failed to push failed scrape response:", "err", err)
c.logger.Warn("Failed to push failed scrape response:", "err", err)
return
}
level.Info(c.logger).Log("msg", "Pushed failed scrape response")
c.logger.Info("Pushed failed scrape response")
}

func (c *Coordinator) doScrape(request *http.Request, client *http.Client) {
logger := log.With(c.logger, "scrape_id", request.Header.Get("id"))
logger := c.logger.With("scrape_id", request.Header.Get("id"))
timeout, err := util.GetHeaderTimeout(request.Header)
if err != nil {
c.handleErr(request, client, err)
Expand Down Expand Up @@ -137,13 +136,13 @@ func (c *Coordinator) doScrape(request *http.Request, client *http.Client) {
c.handleErr(request, client, fmt.Errorf("failed to scrape %s: %w", request.URL.String(), err))
return
}
level.Info(logger).Log("msg", "Retrieved scrape response")
logger.Info("Retrieved scrape response")
if err = c.doPush(scrapeResp, request, client); err != nil {
pushErrorCounter.Inc()
level.Warn(logger).Log("msg", "Failed to push scrape response:", "err", err)
logger.Warn("Failed to push scrape response:", "err", err)
return
}
level.Info(logger).Log("msg", "Pushed scrape result")
logger.Info("Pushed scrape result")
}

// Report the result of the scrape back up to the proxy.
Expand Down Expand Up @@ -182,28 +181,28 @@ func (c *Coordinator) doPush(resp *http.Response, origRequest *http.Request, cli
func (c *Coordinator) doPoll(client *http.Client) error {
base, err := url.Parse(*proxyURL)
if err != nil {
level.Error(c.logger).Log("msg", "Error parsing url:", "err", err)
c.logger.Error("Error parsing url:", "err", err)
return fmt.Errorf("error parsing url: %w", err)
}
u, err := url.Parse("poll")
if err != nil {
level.Error(c.logger).Log("msg", "Error parsing url:", "err", err)
c.logger.Error("Error parsing url:", "err", err)
return fmt.Errorf("error parsing url poll: %w", err)
}
url := base.ResolveReference(u)
resp, err := client.Post(url.String(), "", strings.NewReader(*myFqdn))
if err != nil {
level.Error(c.logger).Log("msg", "Error polling:", "err", err)
c.logger.Error("Error polling:", "err", err)
return fmt.Errorf("error polling: %w", err)
}
defer resp.Body.Close()

request, err := http.ReadRequest(bufio.NewReader(resp.Body))
if err != nil {
level.Error(c.logger).Log("msg", "Error reading request:", "err", err)
c.logger.Error("Error reading request:", "err", err)
return fmt.Errorf("error reading request: %w", err)
}
level.Info(c.logger).Log("msg", "Got scrape request", "scrape_id", request.Header.Get("id"), "url", request.URL)
c.logger.Info("Got scrape request", "scrape_id", request.Header.Get("id"), "url", request.URL)

request.RequestURI = ""

Expand All @@ -221,32 +220,32 @@ func (c *Coordinator) loop(bo backoff.BackOff, client *http.Client) {
if err := backoff.RetryNotify(op, bo, func(err error, _ time.Duration) {
pollErrorCounter.Inc()
}); err != nil {
level.Error(c.logger).Log("err", err)
c.logger.Error("backoff returned error", "error", err)
}
}
}

func main() {
promlogConfig := promlog.Config{}
flag.AddFlags(kingpin.CommandLine, &promlogConfig)
promslogConfig := promslog.Config{}
flag.AddFlags(kingpin.CommandLine, &promslogConfig)
kingpin.HelpFlag.Short('h')
kingpin.Parse()
logger := promlog.New(&promlogConfig)
logger := promslog.New(&promslogConfig)
coordinator := Coordinator{logger: logger}

if *proxyURL == "" {
level.Error(coordinator.logger).Log("msg", "--proxy-url flag must be specified.")
coordinator.logger.Error("--proxy-url flag must be specified.")
os.Exit(1)
}
// Make sure proxyURL ends with a single '/'
*proxyURL = strings.TrimRight(*proxyURL, "/") + "/"
level.Info(coordinator.logger).Log("msg", "URL and FQDN info", "proxy_url", *proxyURL, "fqdn", *myFqdn)
coordinator.logger.Info("URL and FQDN info", "proxy_url", *proxyURL, "fqdn", *myFqdn)

tlsConfig := &tls.Config{}
if *tlsCert != "" {
cert, err := tls.LoadX509KeyPair(*tlsCert, *tlsKey)
if err != nil {
level.Error(coordinator.logger).Log("msg", "Certificate or Key is invalid", "err", err)
coordinator.logger.Error("Certificate or Key is invalid", "err", err)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
coordinator.logger.Error("Certificate or Key is invalid", "err", err)
coordinator.logger.Error("Certificate or key is invalid", "err", err)

os.Exit(1)
}

Expand All @@ -257,12 +256,12 @@ func main() {
if *caCertFile != "" {
caCert, err := os.ReadFile(*caCertFile)
if err != nil {
level.Error(coordinator.logger).Log("msg", "Not able to read cacert file", "err", err)
coordinator.logger.Error("Not able to read cacert file", "err", err)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
coordinator.logger.Error("Not able to read cacert file", "err", err)
coordinator.logger.Error("Unable to read cacert file", "err", err)

os.Exit(1)
}
caCertPool := x509.NewCertPool()
if ok := caCertPool.AppendCertsFromPEM(caCert); !ok {
level.Error(coordinator.logger).Log("msg", "Failed to use cacert file as ca certificate")
coordinator.logger.Error("Failed to use cacert file as ca certificate")
os.Exit(1)
}

Expand All @@ -272,7 +271,7 @@ func main() {
if *metricsAddr != "" {
go func() {
if err := http.ListenAndServe(*metricsAddr, promhttp.Handler()); err != nil {
level.Warn(coordinator.logger).Log("msg", "ListenAndServe", "err", err)
coordinator.logger.Warn("ListenAndServe", "err", err)
}
}()
}
Expand Down
11 changes: 3 additions & 8 deletions cmd/client/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,21 +19,16 @@ import (
"net/http"
"net/http/httptest"
"testing"
)

type TestLogger struct{}

func (tl *TestLogger) Log(vars ...interface{}) error {
fmt.Printf("%+v\n", vars)
return nil
}
"github.com/prometheus/common/promslog"
)

func prepareTest() (*httptest.Server, Coordinator) {
ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(http.StatusOK)
fmt.Fprintln(w, "GET /index.html HTTP/1.0\n\nOK")
}))
c := Coordinator{logger: &TestLogger{}}
c := Coordinator{logger: promslog.NewNopLogger()}
*proxyURL = ts.URL
return ts, c
}
Expand Down
15 changes: 7 additions & 8 deletions cmd/proxy/coordinator.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,12 @@ package main
import (
"context"
"fmt"
"log/slog"
"net/http"
"sync"
"time"

"github.com/alecthomas/kingpin/v2"
"github.com/go-kit/log"
"github.com/go-kit/log/level"
"github.com/google/uuid"
"github.com/prometheus-community/pushprox/util"
"github.com/prometheus/client_golang/prometheus"
Expand Down Expand Up @@ -55,11 +54,11 @@ type Coordinator struct {
// Clients we know about and when they last contacted us.
known map[string]time.Time

logger log.Logger
logger *slog.Logger
}

// NewCoordinator initiates the coordinator and starts the client cleanup routine
func NewCoordinator(logger log.Logger) (*Coordinator, error) {
func NewCoordinator(logger *slog.Logger) (*Coordinator, error) {
c := &Coordinator{
waiting: map[string]chan *http.Request{},
responses: map[string]chan *http.Response{},
Expand Down Expand Up @@ -112,7 +111,7 @@ func (c *Coordinator) DoScrape(ctx context.Context, r *http.Request) (*http.Resp
if err != nil {
return nil, err
}
level.Info(c.logger).Log("msg", "DoScrape", "scrape_id", id, "url", r.URL.String())
c.logger.Info("DoScrape", "scrape_id", id, "url", r.URL.String())
r.Header.Add("Id", id)
select {
case <-ctx.Done():
Expand All @@ -133,7 +132,7 @@ func (c *Coordinator) DoScrape(ctx context.Context, r *http.Request) (*http.Resp

// WaitForScrapeInstruction registers a client waiting for a scrape result
func (c *Coordinator) WaitForScrapeInstruction(fqdn string) (*http.Request, error) {
level.Info(c.logger).Log("msg", "WaitForScrapeInstruction", "fqdn", fqdn)
c.logger.Info("WaitForScrapeInstruction", "fqdn", fqdn)

c.addKnownClient(fqdn)
// TODO: What if the client times out?
Expand Down Expand Up @@ -165,7 +164,7 @@ func (c *Coordinator) WaitForScrapeInstruction(fqdn string) (*http.Request, erro
// ScrapeResult send by client
func (c *Coordinator) ScrapeResult(r *http.Response) error {
id := r.Header.Get("Id")
level.Info(c.logger).Log("msg", "ScrapeResult", "scrape_id", id)
c.logger.Info("ScrapeResult", "scrape_id", id)
ctx, cancel := context.WithTimeout(context.Background(), util.GetScrapeTimeout(maxScrapeTimeout, defaultScrapeTimeout, r.Header))
defer cancel()
// Don't expose internal headers.
Expand Down Expand Up @@ -217,7 +216,7 @@ func (c *Coordinator) gc() {
deleted++
}
}
level.Info(c.logger).Log("msg", "GC of clients completed", "deleted", deleted, "remaining", len(c.known))
c.logger.Info("GC of clients completed", "deleted", deleted, "remaining", len(c.known))
knownClients.Set(float64(len(c.known)))
}()
}
Expand Down
Loading