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

Compactor: Add tracing support #4903

Merged
merged 23 commits into from
Dec 1, 2021
Merged
Changes from 1 commit
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
9d3b32f
first draft for tracing
metonymic-smokey Nov 18, 2021
aa0ede1
add tracer to context
metonymic-smokey Nov 19, 2021
2f342ac
minor fixes
metonymic-smokey Nov 19, 2021
47c3171
create common block spans; log block errors
metonymic-smokey Nov 20, 2021
e1dd35a
reverted to generic block spans
metonymic-smokey Nov 22, 2021
031aeaf
Merge branch 'thanos-io:main' into tracing
metonymic-smokey Nov 22, 2021
8104e73
changed block ID tag; add span for block delete
metonymic-smokey Nov 22, 2021
e329f85
minor fix
metonymic-smokey Nov 22, 2021
ffd55fd
removed extra spans
metonymic-smokey Nov 22, 2021
480ffe7
Merge branch 'main' into tracing
metonymic-smokey Nov 24, 2021
a3a8f6d
addressed some review comments
metonymic-smokey Nov 24, 2021
9305a89
removed block spans
metonymic-smokey Nov 24, 2021
2f50d22
changed group key
metonymic-smokey Nov 25, 2021
6d4f0de
removed extra var declarations
metonymic-smokey Nov 25, 2021
7a5795e
Merge branch 'thanos-io:main' into tracing
metonymic-smokey Nov 25, 2021
ae6b6d5
removed comments
metonymic-smokey Nov 25, 2021
3783abd
added changelog entry
metonymic-smokey Nov 26, 2021
f12a18e
update healthcheck to healthstats.
metonymic-smokey Nov 27, 2021
7620389
Merge branch 'thanos-io:main' into tracing
metonymic-smokey Nov 29, 2021
a5ef615
draft: logging errors
metonymic-smokey Nov 29, 2021
ced45c4
removed extra line
metonymic-smokey Nov 30, 2021
9296528
used alternate function to log errors
metonymic-smokey Nov 30, 2021
01812fc
minor nits
metonymic-smokey Dec 1, 2021
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
Prev Previous commit
Next Next commit
minor fixes
Signed-off-by: metonymic-smokey <ahuja.aditi@gmail.com>
  • Loading branch information
metonymic-smokey committed Nov 19, 2021

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
commit 2f342ac7245aae6149c9a5d955fa927dad21305f
9 changes: 2 additions & 7 deletions cmd/thanos/compact.go
Original file line number Diff line number Diff line change
@@ -42,9 +42,8 @@ import (
"github.com/thanos-io/thanos/pkg/prober"
"github.com/thanos-io/thanos/pkg/runutil"
httpserver "github.com/thanos-io/thanos/pkg/server/http"
"github.com/thanos-io/thanos/pkg/ui"

"github.com/thanos-io/thanos/pkg/tracing"
"github.com/thanos-io/thanos/pkg/ui"
)

var (
@@ -161,10 +160,6 @@ func newCompactMetrics(reg *prometheus.Registry, deleteDelay time.Duration) *com
return m
}

type contextKey struct{}

var tracerKey = contextKey{}

func runCompact(
g *run.Group,
logger log.Logger,
@@ -422,7 +417,7 @@ func runCompact(
}

compactMainFn := func() error {
if err := compactor.Compact(ctx, tracer); err != nil {
if err := compactor.Compact(ctx); err != nil {
return errors.Wrap(err, "compaction")
}

34 changes: 13 additions & 21 deletions pkg/compact/compact.go
Original file line number Diff line number Diff line change
@@ -971,10 +971,7 @@ func (cg *Group) compact(ctx context.Context, dir string, planner Planner, comp
defer cg.mtx.Unlock()

// the ctx here comes from bucketCompactor.compact and hence has the groups parent span in it
// hence, no need to create a new context here
var span opentracing.Span
span, ctx = tracing.StartSpan(ctx, "compaction_group", opentracing.Tags{})
defer span.Finish()
// hence, no need to create a new context/span here

// Check for overlapped blocks.
overlappingBlocks := false
@@ -991,7 +988,7 @@ func (cg *Group) compact(ctx context.Context, dir string, planner Planner, comp
var toCompact []*metadata.Meta
tracing.DoInSpan(ctx, "compaction_planning", func(ctx context.Context) {
toCompact, err = planner.Plan(ctx, cg.metasByMinTime)
})
}, opentracing.Tags{"group key": cg.Key()})
if err != nil {
return false, ulid.ULID{}, errors.Wrap(err, "plan compaction")
}
@@ -1021,7 +1018,7 @@ func (cg *Group) compact(ctx context.Context, dir string, planner Planner, comp

tracing.DoInSpan(ctx, "compaction_block_download", func(ctx context.Context) {
err = block.Download(ctx, cg.logger, cg.bkt, meta.ULID, bdir)
})
}, opentracing.Tags{"block ID": meta.ULID})

Copy link
Member

Choose a reason for hiding this comment

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

tiny nit

Suggested change

if err != nil {
return false, ulid.ULID{}, retry(errors.Wrapf(err, "download block %s", meta.ULID))
@@ -1031,7 +1028,7 @@ func (cg *Group) compact(ctx context.Context, dir string, planner Planner, comp
var stats block.HealthStats
tracing.DoInSpan(ctx, "compaction_block_healthcheck", func(ctx context.Context) {
metonymic-smokey marked this conversation as resolved.
Show resolved Hide resolved
stats, err = block.GatherIndexHealthStats(cg.logger, filepath.Join(bdir, block.IndexFilename), meta.MinTime, meta.MaxTime)
})
}, opentracing.Tags{"block ID": meta.ULID})
if err != nil {
return false, ulid.ULID{}, errors.Wrapf(err, "gather index issues for block %s", bdir)
}
@@ -1057,10 +1054,10 @@ func (cg *Group) compact(ctx context.Context, dir string, planner Planner, comp
level.Info(cg.logger).Log("msg", "downloaded and verified blocks; compacting blocks", "plan", fmt.Sprintf("%v", toCompactDirs), "duration", time.Since(begin), "duration_ms", time.Since(begin).Milliseconds())

begin = time.Now()
// start and finish new span for compaction
compSpan, _ := tracing.StartSpan(ctx, "compaction")
compID, err = comp.Compact(dir, toCompactDirs, nil)
compSpan.Finish()
tracing.DoInSpan(ctx, "compaction", func(ctx context.Context) {
compID, err = comp.Compact(dir, toCompactDirs, nil)
})

if err != nil {
return false, ulid.ULID{}, halt(errors.Wrapf(err, "compact blocks %v", toCompactDirs))
}
@@ -1154,10 +1151,6 @@ func (cg *Group) deleteBlock(id ulid.ULID, bdir string) error {
return nil
}

type contextKey struct{}

var tracerKey = contextKey{}

// BucketCompactor compacts blocks in a bucket.
type BucketCompactor struct {
logger log.Logger
@@ -1200,9 +1193,7 @@ func NewBucketCompactor(
}

// Compact runs compaction over bucket.
func (c *BucketCompactor) Compact(ctx context.Context, tracer opentracing.Tracer) (rerr error) {
ctx = tracing.ContextWithTracer(ctx, tracer)

func (c *BucketCompactor) Compact(ctx context.Context) (rerr error) {
defer func() {
// Do not remove the compactDir if an error has occurred
// because potentially on the next run we would not have to download
@@ -1235,16 +1226,17 @@ func (c *BucketCompactor) Compact(ctx context.Context, tracer opentracing.Tracer
defer wg.Done()
for g := range groupChan {
workCtx = tracing.CopyTraceContext(workCtx, ctx)
workCtx = tracing.ContextWithTracer(workCtx, tracer)
// adding a parent span to ctx for overall compaction
// do not need to add a separate span since the parent span is not used separately here - can parent span be passed from original compaction function?
_, ctx = tracing.StartSpan(workCtx, "compaction", opentracing.Tags{})
var groupSpan tracing.Span
groupSpan, ctx = tracing.StartSpan(workCtx, "compaction", opentracing.Tags{"group key": g.Key()})
defer groupSpan.Finish() // added to avoid parent spans being finished before child
var err error
var shouldRerunGroup bool
// done in a child span derived from the parent span in ctx
tracing.DoInSpan(ctx, "group_compaction", func(ctx context.Context) {
shouldRerunGroup, _, err = g.Compact(ctx, c.compactDir, c.planner, c.comp)
})
}) //not adding group key for group_compaction since it is already added to "compaction" parent span
//DoInSpan internally runs startSpan
if err == nil {
if shouldRerunGroup {