Skip to content

Commit f675736

Browse files
vlad-diachenkovlad-diachenko
authored
#4147 added tenant_id tag to tracing spans (#4186)
* #4147 1. added logic to add tag `tenant_id` to tracing spans if tenant is defined in context. 2. added additional tracing spans to merge_querier's methods Signed-off-by: Vladyslav Diachenko <[email protected]> * #4147 fixed errors reported by linters Signed-off-by: Vladyslav Diachenko <[email protected]> * #4147 1.changed value of tenant_id tag in tracing span from string value to array 2. removed import alias for spanlogger Signed-off-by: Vladyslav Diachenko <[email protected]> * #4147 1. fixed tests 2. fixed name of the spans in mergeQuerier Signed-off-by: Vladyslav Diachenko <[email protected]> * #4147 updated CHANGELOG.md Signed-off-by: Vladyslav Diachenko <[email protected]> * #4147 passed method's span context to mergeDistinctStringSlice function to create inner spans under this context Signed-off-by: Vladyslav Diachenko <[email protected]> * #4147 added mocktracer package for testing purpose Signed-off-by: Vladyslav Diachenko <[email protected]> * #4147 fixed code style and removed redundant context param that is not used in callback function Signed-off-by: Vladyslav Diachenko <[email protected]> * #4147 fixed code style Signed-off-by: Vladyslav Diachenko <[email protected]>
1 parent da11bff commit f675736

File tree

10 files changed

+734
-2
lines changed

10 files changed

+734
-2
lines changed

‎CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
* `cortex_alertmanager_state_persist_failed_total`
2020
* [ENHANCEMENT] Blocks storage: support ingesting exemplars. Enabled by setting new CLI flag `-blocks-storage.tsdb.max-exemplars=<n>` or config option `blocks_storage.tsdb.max_exemplars` to positive value. #4124
2121
* [ENHANCEMENT] Distributor: Added distributors ring status section in the admin page. #4151
22+
* [ENHANCEMENT] Added `tenant_ids` tag to tracing spans #4147
2223
* [BUGFIX] Purger: fix `Invalid null value in condition for column range` caused by `nil` value in range for WriteBatch query. #4128
2324
* [BUGFIX] Ingester: fixed infrequent panic caused by a race condition between TSDB mmap-ed head chunks truncation and queries. #4176
2425

‎pkg/querier/tenantfederation/merge_queryable.go

+11-1
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import (
1515

1616
"github.com/cortexproject/cortex/pkg/tenant"
1717
"github.com/cortexproject/cortex/pkg/util/concurrency"
18+
"github.com/cortexproject/cortex/pkg/util/spanlogger"
1819
)
1920

2021
const (
@@ -96,6 +97,9 @@ type mergeQueryable struct {
9697
// Querier returns a new mergeQuerier, which aggregates results from multiple
9798
// underlying queriers into a single result.
9899
func (m *mergeQueryable) Querier(ctx context.Context, mint int64, maxt int64) (storage.Querier, error) {
100+
// TODO: it's necessary to think how to override context inside querier
101+
// to mark spans created inside querier as child of a span created inside
102+
// methods of merged querier.
99103
ids, queriers, err := m.callback(ctx, mint, maxt)
100104
if err != nil {
101105
return nil, err
@@ -133,6 +137,8 @@ type mergeQuerier struct {
133137
// For the label "original_" + `idLabelName it will return all the values
134138
// of the underlying queriers for `idLabelName`.
135139
func (m *mergeQuerier) LabelValues(name string, matchers ...*labels.Matcher) ([]string, storage.Warnings, error) {
140+
log, _ := spanlogger.New(m.ctx, "mergeQuerier.LabelValues")
141+
defer log.Span.Finish()
136142
if name == m.idLabelName {
137143
return m.ids, nil, nil
138144
}
@@ -152,6 +158,8 @@ func (m *mergeQuerier) LabelValues(name string, matchers ...*labels.Matcher) ([]
152158
// queriers. It also adds the `idLabelName` and if present in the original
153159
// results the original `idLabelName`.
154160
func (m *mergeQuerier) LabelNames() ([]string, storage.Warnings, error) {
161+
log, _ := spanlogger.New(m.ctx, "mergeQuerier.LabelNames")
162+
defer log.Span.Finish()
155163
labelNames, warnings, err := m.mergeDistinctStringSlice(func(ctx context.Context, q storage.Querier) ([]string, storage.Warnings, error) {
156164
return q.LabelNames()
157165
})
@@ -272,6 +280,8 @@ type selectJob struct {
272280
// matching. The forwarded labelSelector is not containing those that operate
273281
// on `idLabelName`.
274282
func (m *mergeQuerier) Select(sortSeries bool, hints *storage.SelectHints, matchers ...*labels.Matcher) storage.SeriesSet {
283+
log, ctx := spanlogger.New(m.ctx, "mergeQuerier.Select")
284+
defer log.Span.Finish()
275285
matchedValues, filteredMatchers := filterValuesByMatchers(m.idLabelName, m.ids, matchers...)
276286
var jobs = make([]interface{}, len(matchedValues))
277287
var seriesSets = make([]storage.SeriesSet, len(matchedValues))
@@ -305,7 +315,7 @@ func (m *mergeQuerier) Select(sortSeries bool, hints *storage.SelectHints, match
305315
return nil
306316
}
307317

308-
err := concurrency.ForEach(m.ctx, jobs, maxConcurrency, run)
318+
err := concurrency.ForEach(ctx, jobs, maxConcurrency, run)
309319
if err != nil {
310320
return storage.ErrSeriesSet(err)
311321
}

‎pkg/querier/tenantfederation/merge_queryable_test.go

+75-1
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,14 @@ import (
44
"context"
55
"errors"
66
"fmt"
7+
"reflect"
78
"sort"
89
"strings"
910
"testing"
1011
"time"
1112

13+
"github.com/opentracing/opentracing-go"
14+
"github.com/opentracing/opentracing-go/mocktracer"
1215
"github.com/prometheus/common/model"
1316
"github.com/prometheus/prometheus/pkg/labels"
1417
"github.com/prometheus/prometheus/storage"
@@ -18,6 +21,7 @@ import (
1821

1922
"github.com/cortexproject/cortex/pkg/querier/series"
2023
"github.com/cortexproject/cortex/pkg/tenant"
24+
"github.com/cortexproject/cortex/pkg/util/spanlogger"
2125
)
2226

2327
const (
@@ -37,7 +41,11 @@ func (m *mockTenantQueryableWithFilter) Querier(ctx context.Context, _, _ int64)
3741
return nil, err
3842
}
3943

40-
q := mockTenantQuerier{tenant: tenantIDs[0], extraLabels: m.extraLabels}
44+
q := mockTenantQuerier{
45+
tenant: tenantIDs[0],
46+
extraLabels: m.extraLabels,
47+
ctx: ctx,
48+
}
4149

4250
// set warning if exists
4351
if m.warningsByTenant != nil {
@@ -66,6 +74,7 @@ type mockTenantQuerier struct {
6674

6775
warnings storage.Warnings
6876
queryErr error
77+
ctx context.Context
6978
}
7079

7180
func (m mockTenantQuerier) matrix() model.Matrix {
@@ -135,6 +144,8 @@ func (m *mockSeriesSet) Warnings() storage.Warnings {
135144
}
136145

137146
func (m mockTenantQuerier) Select(_ bool, sp *storage.SelectHints, matchers ...*labels.Matcher) storage.SeriesSet {
147+
log, _ := spanlogger.New(m.ctx, "mockTenantQuerier.select")
148+
defer log.Span.Finish()
138149
var matrix model.Matrix
139150

140151
for _, s := range m.matrix() {
@@ -478,3 +489,66 @@ func TestSetLabelsRetainExisting(t *testing.T) {
478489
assert.Equal(t, tc.expected, setLabelsRetainExisting(tc.labels, tc.additionalLabels...))
479490
}
480491
}
492+
493+
func TestTracingMergeQueryable(t *testing.T) {
494+
mockTracer := mocktracer.New()
495+
opentracing.SetGlobalTracer(mockTracer)
496+
ctx := user.InjectOrgID(context.Background(), "team-a|team-b")
497+
498+
// set a multi tenant resolver
499+
tenant.WithDefaultResolver(tenant.NewMultiResolver())
500+
filter := mockTenantQueryableWithFilter{}
501+
q := NewQueryable(&filter, false)
502+
// retrieve querier if set
503+
querier, err := q.Querier(ctx, mint, maxt)
504+
require.NoError(t, err)
505+
506+
seriesSet := querier.Select(true, &storage.SelectHints{Start: mint,
507+
End: maxt})
508+
509+
require.NoError(t, seriesSet.Err())
510+
spans := mockTracer.FinishedSpans()
511+
assertSpanExist(t, spans, "mergeQuerier.Select", expectedTag{spanlogger.TenantIDTagName,
512+
[]string{"team-a", "team-b"}})
513+
assertSpanExist(t, spans, "mockTenantQuerier.select", expectedTag{spanlogger.TenantIDTagName,
514+
[]string{"team-a"}})
515+
assertSpanExist(t, spans, "mockTenantQuerier.select", expectedTag{spanlogger.TenantIDTagName,
516+
[]string{"team-b"}})
517+
}
518+
519+
func assertSpanExist(t *testing.T,
520+
actualSpans []*mocktracer.MockSpan,
521+
name string,
522+
tag expectedTag) {
523+
for _, span := range actualSpans {
524+
if span.OperationName == name && containsTags(span, tag) {
525+
return
526+
}
527+
}
528+
require.FailNow(t, "can not find span matching params",
529+
"expected span with name `%v` and with "+
530+
"tags %v to be present but it was not. actual spans: %+v",
531+
name, tag, extractNameWithTags(actualSpans))
532+
}
533+
534+
func extractNameWithTags(actualSpans []*mocktracer.MockSpan) []spanWithTags {
535+
result := make([]spanWithTags, len(actualSpans))
536+
for i, span := range actualSpans {
537+
result[i] = spanWithTags{span.OperationName, span.Tags()}
538+
}
539+
return result
540+
}
541+
542+
func containsTags(span *mocktracer.MockSpan, expectedTag expectedTag) bool {
543+
return reflect.DeepEqual(span.Tag(expectedTag.key), expectedTag.values)
544+
}
545+
546+
type spanWithTags struct {
547+
name string
548+
tags map[string]interface{}
549+
}
550+
551+
type expectedTag struct {
552+
key string
553+
values []string
554+
}

‎pkg/util/spanlogger/spanlogger.go

+8
Original file line numberDiff line numberDiff line change
@@ -9,11 +9,16 @@ import (
99
"github.com/opentracing/opentracing-go/ext"
1010
otlog "github.com/opentracing/opentracing-go/log"
1111

12+
"github.com/cortexproject/cortex/pkg/tenant"
1213
util_log "github.com/cortexproject/cortex/pkg/util/log"
1314
)
1415

1516
type loggerCtxMarker struct{}
1617

18+
const (
19+
TenantIDTagName = "tenant_ids"
20+
)
21+
1722
var (
1823
loggerCtxKey = &loggerCtxMarker{}
1924
)
@@ -34,6 +39,9 @@ func New(ctx context.Context, method string, kvps ...interface{}) (*SpanLogger,
3439
// retrieved with FromContext or FromContextWithFallback.
3540
func NewWithLogger(ctx context.Context, l log.Logger, method string, kvps ...interface{}) (*SpanLogger, context.Context) {
3641
span, ctx := opentracing.StartSpanFromContext(ctx, method)
42+
if ids, _ := tenant.TenantIDs(ctx); len(ids) > 0 {
43+
span.SetTag(TenantIDTagName, ids)
44+
}
3745
logger := &SpanLogger{
3846
Logger: log.With(util_log.WithContext(ctx, l), "method", method),
3947
Span: span,

‎pkg/util/spanlogger/spanlogger_test.go

+24
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,11 @@ import (
55
"testing"
66

77
"github.com/go-kit/kit/log"
8+
"github.com/opentracing/opentracing-go"
9+
"github.com/opentracing/opentracing-go/mocktracer"
810
"github.com/pkg/errors"
911
"github.com/stretchr/testify/require"
12+
"github.com/weaveworks/common/user"
1013
)
1114

1215
func TestSpanLogger_Log(t *testing.T) {
@@ -44,6 +47,27 @@ func TestSpanLogger_CustomLogger(t *testing.T) {
4447
require.Equal(t, expect, logged)
4548
}
4649

50+
func TestSpanCreatedWithTenantTag(t *testing.T) {
51+
mockSpan := createSpan(user.InjectOrgID(context.Background(), "team-a"))
52+
53+
require.Equal(t, []string{"team-a"}, mockSpan.Tag(TenantIDTagName))
54+
}
55+
56+
func TestSpanCreatedWithoutTenantTag(t *testing.T) {
57+
mockSpan := createSpan(context.Background())
58+
59+
_, exist := mockSpan.Tags()[TenantIDTagName]
60+
require.False(t, exist)
61+
}
62+
63+
func createSpan(ctx context.Context) *mocktracer.MockSpan {
64+
mockTracer := mocktracer.New()
65+
opentracing.SetGlobalTracer(mockTracer)
66+
67+
logger, _ := New(ctx, "name")
68+
return logger.Span.(*mocktracer.MockSpan)
69+
}
70+
4771
type funcLogger func(keyvals ...interface{}) error
4872

4973
func (f funcLogger) Log(keyvals ...interface{}) error {

‎vendor/github.com/opentracing/opentracing-go/mocktracer/mocklogrecord.go

+105
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)