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.

Sign up for GitHub

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

Switch to slog #497

Merged
merged 5 commits into from
Dec 3, 2024
Merged

Switch to slog #497

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
17c68e3
update Prometheus dependency to use slog
yeya24 Dec 1, 2024
2719186
sort import
yeya24 Dec 1, 2024
f641707
fix engine test
yeya24 Dec 1, 2024
d024746
fix lint
yeya24 Dec 3, 2024
0838d6b
ignore functions in goleak
yeya24 Dec 3, 2024
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
18 changes: 9 additions & 9 deletions engine/engine.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,16 @@ package engine

import (
"context"
"log/slog"
"math"
"runtime"
"sort"
"time"

"github.com/efficientgo/core/errors"
"github.com/go-kit/log"
"github.com/go-kit/log/level"
"github.com/prometheus/client_golang/prometheus"
"github.com/prometheus/client_golang/prometheus/promauto"
"github.com/prometheus/common/promslog"
"github.com/prometheus/prometheus/promql"
"github.com/prometheus/prometheus/promql/parser"
"github.com/prometheus/prometheus/storage"
Expand Down Expand Up @@ -109,15 +109,15 @@ func New(opts Opts) *Engine {
// This method is useful when the data being queried does not easily fit into the Prometheus storage model.
func NewWithScanners(opts Opts, scanners engstorage.Scanners) *Engine {
if opts.Logger == nil {
opts.Logger = log.NewNopLogger()
opts.Logger = promslog.NewNopLogger()
}
if opts.LookbackDelta == 0 {
opts.LookbackDelta = 5 * time.Minute
level.Debug(opts.Logger).Log("msg", "lookback delta is zero, setting to default value", "value", 5*time.Minute)
opts.Logger.Debug("lookback delta is zero, setting to default value", "value", 5*time.Minute)
}
if opts.ExtLookbackDelta == 0 {
opts.ExtLookbackDelta = 1 * time.Hour
level.Debug(opts.Logger).Log("msg", "externallookback delta is zero, setting to default value", "value", 1*24*time.Hour)
opts.Logger.Debug("external lookback delta is zero, setting to default value", "value", 1*24*time.Hour)
}
if opts.SelectorBatchSize != 0 {
opts.LogicalOptimizers = append(
Expand Down Expand Up @@ -215,7 +215,7 @@ type Engine struct {
disableDuplicateLabelChecks bool
disableFallback bool

logger log.Logger
logger *slog.Logger
lookbackDelta time.Duration
enablePerStepStats bool
logicalOptimizers []logicalplan.Optimizer
Expand Down Expand Up @@ -700,7 +700,7 @@ func (q *compatibilityQuery) Stats() *stats.Statistics {

func (q *compatibilityQuery) Close() {
if err := q.scanners.Close(); err != nil {
level.Warn(q.engine.logger).Log("msg", "error closing storage scanners, some memory might have leaked", "err", err)
q.engine.logger.Warn("error closing storage scanners, some memory might have leaked", "err", err)
}
}

Expand All @@ -720,7 +720,7 @@ func (n nopQueryTracker) Insert(ctx context.Context, query string) (int, error)
func (n nopQueryTracker) Delete(insertIndex int) {}
func (n nopQueryTracker) Close() error { return nil }

func recoverEngine(logger log.Logger, plan logicalplan.Plan, errp *error) {
func recoverEngine(logger *slog.Logger, plan logicalplan.Plan, errp *error) {
e := recover()
if e == nil {
return
Expand All @@ -732,7 +732,7 @@ func recoverEngine(logger log.Logger, plan logicalplan.Plan, errp *error) {
buf := make([]byte, 64<<10)
buf = buf[:runtime.Stack(buf, false)]

level.Error(logger).Log("msg", "runtime panic in engine", "expr", plan.Root().String(), "err", e, "stacktrace", string(buf))
logger.Error("runtime panic in engine", "expr", plan.Root().String(), "err", e, "stacktrace", string(buf))
*errp = errors.Wrap(err, "unexpected error")
}
}
23 changes: 13 additions & 10 deletions engine/engine_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,20 +22,13 @@ import (

"github.com/efficientgo/core/errors"
"github.com/efficientgo/core/testutil"
"github.com/go-kit/log"
"github.com/google/go-cmp/cmp"
"github.com/google/go-cmp/cmp/cmpopts"
"github.com/stretchr/testify/require"
"go.uber.org/goleak"
"golang.org/x/exp/maps"

"github.com/thanos-io/promql-engine/engine"
"github.com/thanos-io/promql-engine/execution/model"
"github.com/thanos-io/promql-engine/execution/warnings"
"github.com/thanos-io/promql-engine/logicalplan"
"github.com/thanos-io/promql-engine/query"
"github.com/thanos-io/promql-engine/storage/prometheus"

"github.com/prometheus/common/promslog"
"github.com/prometheus/prometheus/model/histogram"
"github.com/prometheus/prometheus/model/labels"
"github.com/prometheus/prometheus/model/timestamp"
Expand All @@ -48,11 +41,21 @@ import (
"github.com/prometheus/prometheus/util/annotations"
"github.com/prometheus/prometheus/util/stats"
"github.com/prometheus/prometheus/util/teststorage"

"github.com/thanos-io/promql-engine/engine"
"github.com/thanos-io/promql-engine/execution/model"
"github.com/thanos-io/promql-engine/execution/warnings"
"github.com/thanos-io/promql-engine/logicalplan"
"github.com/thanos-io/promql-engine/query"
"github.com/thanos-io/promql-engine/storage/prometheus"
)

func TestMain(m *testing.M) {
parser.EnableExperimentalFunctions = true
goleak.VerifyTestMain(m)
goleak.VerifyTestMain(m,
Copy link
Contributor

Choose a reason for hiding this comment

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

Would updating the opencensus-go package to latest help?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I checked their master branch and the goroutine is still there https://github.com/census-instrumentation/opencensus-go/blob/master/stats/view/worker.go#L34.

Thanos main repo also ignores this function in the goleak test. But I am not sure why the problem only occur now. It should be always there

// https://github.com/census-instrumentation/opencensus-go/blob/d7677d6af5953e0506ac4c08f349c62b917a443a/stats/view/worker.go#L34
goleak.IgnoreTopFunction("go.opencensus.io/stats/view.(*worker).start"),
)
}

func TestPromqlAcceptance(t *testing.T) {
Expand Down Expand Up @@ -4265,7 +4268,7 @@ func TestQueryConcurrency(t *testing.T) {

var (
ctx = context.Background()
logger = log.NewLogfmtLogger(os.Stdout)
logger = promslog.New(&promslog.Config{Writer: os.Stdout})
concurrency = 2
maxQueries = 4
responseChan = make(chan struct{}, maxQueries)
Expand Down
4 changes: 1 addition & 3 deletions go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,10 @@ require (
github.com/cortexproject/promqlsmith v0.0.0-20240506042652-6cfdd9739a5e
github.com/efficientgo/core v1.0.0-rc.2
github.com/facette/natsort v0.0.0-20181210072756-2cd4dd1e2dcb
github.com/go-kit/log v0.2.1
github.com/google/go-cmp v0.6.0
github.com/prometheus/client_golang v1.20.5
github.com/prometheus/common v0.60.1
github.com/prometheus/prometheus v0.300.0-beta.0.0.20241007135006-65f610353919
github.com/prometheus/prometheus v0.300.0-beta.0.0.20241007202516-805954d8f8ba
github.com/stretchr/testify v1.9.0
github.com/zhangyunhao116/umap v0.0.0-20221211160557-cb7705fafa39
go.uber.org/goleak v1.3.0
Expand All @@ -38,7 +37,6 @@ require (
github.com/dennwc/varint v1.0.0 // indirect
github.com/edsrzf/mmap-go v1.2.0 // indirect
github.com/felixge/httpsnoop v1.0.4 // indirect
github.com/go-logfmt/logfmt v0.6.0 // indirect
github.com/go-logr/logr v1.4.2 // indirect
github.com/go-logr/stdr v1.2.2 // indirect
github.com/gogo/protobuf v1.3.2 // indirect
Expand Down
8 changes: 2 additions & 6 deletions go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,13 +99,9 @@ github.com/fxamacker/cbor/v2 v2.7.0/go.mod h1:pxXPTn3joSm21Gbwsv0w9OSA2y1HFR9qXE
github.com/go-kit/kit v0.8.0/go.mod h1:xBxKIO96dXMWWy0MnWVtmwkA9/13aqxPnvrjFYMA2as=
github.com/go-kit/kit v0.9.0/go.mod h1:xBxKIO96dXMWWy0MnWVtmwkA9/13aqxPnvrjFYMA2as=
github.com/go-kit/log v0.1.0/go.mod h1:zbhenjAZHb184qTLMA9ZjW7ThYL0H2mk7Q6pNt4vbaY=
github.com/go-kit/log v0.2.1 h1:MRVx0/zhvdseW+Gza6N9rVzU/IVzaeE1SFI4raAhmBU=
github.com/go-kit/log v0.2.1/go.mod h1:NwTd00d/i8cPZ3xOwwiv2PO5MOcx78fFErGNcVmBjv0=
github.com/go-logfmt/logfmt v0.3.0/go.mod h1:Qt1PoO58o5twSAckw1HlFXLmHsOX5/0LbT9GBnD5lWE=
github.com/go-logfmt/logfmt v0.4.0/go.mod h1:3RMwSq7FuexP4Kalkev3ejPJsZTpXXBr9+V4qmtdjCk=
github.com/go-logfmt/logfmt v0.5.0/go.mod h1:wCYkCAKZfumFQihp8CzCvQ3paCTfi41vtzG1KdI/P7A=
github.com/go-logfmt/logfmt v0.6.0 h1:wGYYu3uicYdqXVgoYbvnkrPVXkuLM1p1ifugDMEdRi4=
github.com/go-logfmt/logfmt v0.6.0/go.mod h1:WYhtIu8zTZfxdn5+rREduYbwxfcBr/Vr6KEVveWlfTs=
github.com/go-logr/logr v1.2.2/go.mod h1:jdQByPbusPIv2/zmleS9BjJVeZ6kBagPoEUsqbVz/1A=
github.com/go-logr/logr v1.4.2 h1:6pFjapn8bFcIbiKo3XT4j/BhANplGihG6tvd+8rYgrY=
github.com/go-logr/logr v1.4.2/go.mod h1:9T104GzyrTigFIr8wt5mBrctHMim0Nb2HLGrmQ40KvY=
Expand Down Expand Up @@ -316,8 +312,8 @@ github.com/prometheus/procfs v0.1.3/go.mod h1:lV6e/gmhEcM9IjHGsFOCxxuZ+z1YqCvr4O
github.com/prometheus/procfs v0.6.0/go.mod h1:cz+aTbrPOrUb4q7XlbU9ygM+/jj0fzG6c1xBZuNvfVA=
github.com/prometheus/procfs v0.15.1 h1:YagwOFzUgYfKKHX6Dr+sHT7km/hxC76UB0learggepc=
github.com/prometheus/procfs v0.15.1/go.mod h1:fB45yRUv8NstnjriLhBQLuOUt+WW4BsoGhij/e3PBqk=
github.com/prometheus/prometheus v0.300.0-beta.0.0.20241007135006-65f610353919 h1:5sFkY/h4vW7n7hR047vw27EQ0cosFkxdYrbhdXuTuXE=
github.com/prometheus/prometheus v0.300.0-beta.0.0.20241007135006-65f610353919/go.mod h1:T9MXHLcLt4uvXTL0fziwJDCj6w6W07odsQ29inDUZyk=
github.com/prometheus/prometheus v0.300.0-beta.0.0.20241007202516-805954d8f8ba h1:5ma/GhzY2KnNgeFrADOtzJ0UGcoKa7H9mhnfiEhUOu4=
github.com/prometheus/prometheus v0.300.0-beta.0.0.20241007202516-805954d8f8ba/go.mod h1:suua3XMOzCiQC0hE9ADwJF8QbNvBSz3vOhdJRKx8vas=
github.com/redis/go-redis/v9 v9.6.1 h1:HHDteefn6ZkTtY5fGUE8tj8uy85AHk6zP7CpzIAM0y4=
github.com/redis/go-redis/v9 v9.6.1/go.mod h1:0C0c6ycQsdpVNQpxb1njEQIqkx5UcsM8FJCQLgE9+RA=
github.com/rogpeppe/go-internal v1.12.0 h1:exVL4IDcn6na9z1rAb56Vxr+CgyK3nn3O+epU5NdKM8=
Expand Down
Loading