Skip to content

Commit

Permalink
fix some nits
Browse files Browse the repository at this point in the history
  • Loading branch information
yeya24 committed Apr 15, 2019
1 parent 24c5d58 commit 5828e4b
Show file tree
Hide file tree
Showing 4 changed files with 15 additions and 32 deletions.
17 changes: 4 additions & 13 deletions cmd/thanos/sidecar.go
Expand Up @@ -11,7 +11,6 @@ import (

"github.com/hashicorp/go-version"
"github.com/prometheus/common/model"

"github.com/go-kit/kit/log"
"github.com/go-kit/kit/log/level"
"github.com/improbable-eng/thanos/pkg/block/metadata"
Expand Down Expand Up @@ -116,7 +115,7 @@ func runSidecar(

confContentYaml, err := objStoreConfig.Content()
if err != nil {
return err
return errors.Wrap(err, "")
}

var uploads = true
Expand All @@ -143,7 +142,7 @@ func runSidecar(
if uploads {
// Retry infinitely until we get Prometheus version.
if err := runutil.Retry(2*time.Second, ctx.Done(), func() error {
if err := m.FetchPromVersion(logger); err != nil {
if m.version, err = promclient.PromVersion(logger, m.promURL); err != nil {
level.Warn(logger).Log(
"msg", "failed to get Prometheus version. Is Prometheus running? Retrying",
"err", err,
Expand All @@ -153,7 +152,7 @@ func runSidecar(

return nil
}); err != nil {
return err
return errors.Wrap(err, "fetch Prometheus version")
}

// Check prometheus's flags to ensure sane sidecar flags.
Expand Down Expand Up @@ -293,10 +292,7 @@ func runSidecar(

var s *shipper.Shipper
if uploadCompacted {
s, err = shipper.NewWithCompacted(ctx, logger, reg, dataDir, bkt, m.Labels, metadata.SidecarSource)
if err != nil {
return errors.Wrap(err, "create shipper")
}
s = shipper.NewWithCompacted(logger, reg, dataDir, bkt, m.Labels, metadata.SidecarSource)
} else {
s = shipper.New(logger, reg, dataDir, bkt, m.Labels, metadata.SidecarSource)
}
Expand Down Expand Up @@ -409,8 +405,3 @@ func (s *promMetadata) Timestamps() (mint int64, maxt int64) {

return s.mint, s.maxt
}

func (s *promMetadata) FetchPromVersion(logger log.Logger) (err error) {
s.version, err = promclient.PromVersion(logger, s.promURL)
return err
}
2 changes: 1 addition & 1 deletion pkg/promclient/promclient.go
Expand Up @@ -379,7 +379,7 @@ func PromqlQueryInstant(ctx context.Context, logger log.Logger, base *url.URL, q
return vec, warnings, nil
}

// GetPromVersion will return the version of Prometheus by querying /version Prometheus endpoint.
// PromVersion will return the version of Prometheus by querying /version Prometheus endpoint.
func PromVersion(logger log.Logger, base *url.URL) (*version.Version, error) {
if logger == nil {
logger = log.NewNopLogger()
Expand Down
22 changes: 8 additions & 14 deletions pkg/shipper/shipper.go
Expand Up @@ -5,14 +5,6 @@ package shipper
import (
"context"
"encoding/json"
"io/ioutil"
"math"
"os"
"path"
"path/filepath"
"sort"
"time"

"github.com/go-kit/kit/log"
"github.com/go-kit/kit/log/level"
"github.com/improbable-eng/thanos/pkg/block"
Expand All @@ -25,6 +17,12 @@ import (
"github.com/prometheus/tsdb"
"github.com/prometheus/tsdb/fileutil"
"github.com/prometheus/tsdb/labels"
"io/ioutil"
"math"
"os"
"path"
"path/filepath"
"sort"
)

type metrics struct {
Expand Down Expand Up @@ -117,24 +115,20 @@ func New(
// to remote if necessary, including compacted blocks which are already in filesystem.
// It attaches the Thanos metadata section in each meta JSON file.
func NewWithCompacted(
ctx context.Context,
logger log.Logger,
r prometheus.Registerer,
dir string,
bucket objstore.Bucket,
lbls func() labels.Labels,
source metadata.SourceType,
) (*Shipper, error) {
) *Shipper {
if logger == nil {
logger = log.NewNopLogger()
}
if lbls == nil {
lbls = func() labels.Labels { return nil }
}

ctx, cancel := context.WithTimeout(ctx, 30*time.Second)
defer cancel()

return &Shipper{
logger: logger,
dir: dir,
Expand All @@ -143,7 +137,7 @@ func NewWithCompacted(
metrics: newMetrics(r, true),
source: source,
uploadCompacted: true,
}, nil
}
}

// Timestamps returns the minimum timestamp for which data is available and the highest timestamp
Expand Down
6 changes: 2 additions & 4 deletions pkg/shipper/shipper_e2e_test.go
Expand Up @@ -190,8 +190,7 @@ func TestShipper_SyncBlocksWithMigrating_e2e(t *testing.T) {
defer upcancel()
testutil.Ok(t, p.WaitPrometheusUp(upctx))

shipper, err := NewWithCompacted(ctx, log.NewLogfmtLogger(os.Stderr), nil, dir, bkt, func() labels.Labels { return extLset }, metadata.TestSource)
testutil.NotOk(t, err) // Compaction not disabled!
shipper := NewWithCompacted(log.NewLogfmtLogger(os.Stderr), nil, dir, bkt, func() labels.Labels { return extLset }, metadata.TestSource)

p.DisableCompaction()
testutil.Ok(t, p.Restart())
Expand All @@ -200,8 +199,7 @@ func TestShipper_SyncBlocksWithMigrating_e2e(t *testing.T) {
defer upcancel2()
testutil.Ok(t, p.WaitPrometheusUp(upctx2))

shipper, err = NewWithCompacted(ctx, log.NewLogfmtLogger(os.Stderr), nil, dir, bkt, func() labels.Labels { return extLset }, metadata.TestSource)
testutil.Ok(t, err)
shipper = NewWithCompacted(log.NewLogfmtLogger(os.Stderr), nil, dir, bkt, func() labels.Labels { return extLset }, metadata.TestSource)

// Create 10 new blocks. 9 of them (non compacted) should be actually uploaded.
var (
Expand Down

0 comments on commit 5828e4b

Please sign in to comment.