Skip to content

Commit

Permalink
[k187] Spans: Stop emitting GetChunks and IndexClient.GetChunkRefs (
Browse files Browse the repository at this point in the history
#11830)

Backport 3b7ac35 from #11825

---

**What this PR does / why we need it**:
Modify our code to not emit the `GetChunks` and the
`IndexClient.GetChunkRefs` spans.
We don't worse our observability by getting rid of them because their
data is redundant with parent spans. Example:
- If an specific index-gateway or ingester struggles, the parent span
`/logproto.Querier/GetChunkIDs` will report it just fine
- If an specific query causes trouble the `query.Exec` span will show
the query name, the `start` and `end`, etc. Also, its children will show
the slower parts of the query (in case we have a weak link)

**Which issue(s) this PR fixes**:
N/A

Co-authored-by: Dylan Guedes <[email protected]>
  • Loading branch information
grafanabot and DylanGuedes authored Jan 30, 2024
1 parent 50522b2 commit b1e7221
Show file tree
Hide file tree
Showing 2 changed files with 0 additions and 29 deletions.
10 changes: 0 additions & 10 deletions pkg/storage/stores/composite_store_entry.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,18 +46,8 @@ func (c *storeEntry) GetChunks(ctx context.Context, userID string, from, through
if ctx.Err() != nil {
return nil, nil, ctx.Err()
}
sp, ctx := opentracing.StartSpanFromContext(ctx, "GetChunks")
defer sp.Finish()
log := spanlogger.FromContext(ctx)
defer log.Span.Finish()

shortcut, err := c.validateQueryTimeRange(ctx, userID, &from, &through)
level.Debug(log).Log(
"shortcut", shortcut,
"from", from.Time(),
"through", through.Time(),
"err", err,
)
if err != nil {
return nil, nil, err
} else if shortcut {
Expand Down
19 changes: 0 additions & 19 deletions pkg/storage/stores/shipper/indexshipper/tsdb/index_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -105,32 +105,13 @@ func cleanMatchers(matchers ...*labels.Matcher) ([]*labels.Matcher, *index.Shard
// They share almost the same fields, so we can add the missing `KB` field to the proto and then
// use that within the tsdb package.
func (c *IndexClient) GetChunkRefs(ctx context.Context, userID string, from, through model.Time, predicate chunk.Predicate) ([]logproto.ChunkRef, error) {
sp, ctx := opentracing.StartSpanFromContext(ctx, "IndexClient.GetChunkRefs")
defer sp.Finish()

var kvps []interface{}
defer func() {
sp.LogKV(kvps...)
}()

matchers, shard, err := cleanMatchers(predicate.Matchers...)
kvps = append(kvps,
"from", from.Time(),
"through", through.Time(),
"matchers", syntax.MatchersString(matchers),
"shard", shard,
"cleanMatcherErr", err,
)
if err != nil {
return nil, err
}

// TODO(owen-d): use a pool to reduce allocs here
chks, err := c.idx.GetChunkRefs(ctx, userID, from, through, nil, shard, matchers...)
kvps = append(kvps,
"chunks", len(chks),
"indexErr", err,
)
if err != nil {
return nil, err
}
Expand Down

0 comments on commit b1e7221

Please sign in to comment.