Skip to content

Commit

Permalink
scopescope byebye
Browse files Browse the repository at this point in the history
  • Loading branch information
ie-pham committed Aug 21, 2024
1 parent 992ed0f commit 01da898
Show file tree
Hide file tree
Showing 9 changed files with 59 additions and 62 deletions.
2 changes: 1 addition & 1 deletion integration/e2e/multi_tenant_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ func testSearch(t *testing.T, tenant string, tenantSize int) {

tagsV2Resp, err := apiClient.SearchTagsV2()
require.NoError(t, err)
require.Equal(t, 4, len(tagsV2Resp.GetScopes())) // resource, span, event, link, scope intrinsics
require.Equal(t, 4, len(tagsV2Resp.GetScopes())) // resource, span, event, link, instrumentation intrinsics
for _, s := range tagsV2Resp.Scopes {
require.NotEmpty(t, s.Tags)
}
Expand Down
2 changes: 1 addition & 1 deletion modules/ingester/instance_search_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -408,7 +408,7 @@ func testSearchTagsAndValuesV2(
assert.Contains(t, tagsResp.TagNames, tagName)
assert.Equal(t, expectedLinkTagValues, tagValues)

// scope scope attr
// instrumentation scope attr

tagValuesResp, err = i.SearchTagValuesV2(ctx, &tempopb.SearchTagValuesRequest{
TagName: fmt.Sprintf("instrumentation.%s", tagName),
Expand Down
2 changes: 1 addition & 1 deletion pkg/traceql/expr.y
Original file line number Diff line number Diff line change
Expand Up @@ -412,7 +412,7 @@ scopedIntrinsicField:
// link:
| LINK_COLON TRACE_ID { $$ = NewIntrinsic(IntrinsicLinkTraceID) }
| LINK_COLON SPAN_ID { $$ = NewIntrinsic(IntrinsicLinkSpanID) }
// scope:
// instrumentation:
| INSTRUMENTATION_COLON NAME { $$ = NewIntrinsic(IntrinsicInstrumentationName) }
| INSTRUMENTATION_COLON VERSION { $$ = NewIntrinsic(IntrinsicInstrumentationVersion) }
;
Expand Down
12 changes: 6 additions & 6 deletions tempodb/encoding/vparquet4/block_autocomplete.go
Original file line number Diff line number Diff line change
Expand Up @@ -233,8 +233,8 @@ func autocompleteIter(ctx context.Context, tr tagRequest, pf *parquet.File, opts
}
}

if len(catConditions.scope) > 0 || tr.keysRequested(traceql.AttributeScopeInstrumentation) {
currentIter, err = createDistinctScopeIterator(makeIter, tr, currentIter, catConditions.scope)
if len(catConditions.instrumentation) > 0 || tr.keysRequested(traceql.AttributeScopeInstrumentation) {
currentIter, err = createDistinctScopeIterator(makeIter, tr, currentIter, catConditions.instrumentation)
if err != nil {
return nil, errors.Wrap(err, "creating instrumentation iterator")
}
Expand Down Expand Up @@ -767,7 +767,7 @@ func createDistinctScopeIterator(
attrIter, err := createDistinctAttributeIterator(makeIter, tr, genericConditions, DefinitionLevelInstrumentationScopeAttrs,
columnPathInstrumentationAttrKey, columnPathInstrumentationAttrString, columnPathInstrumentationAttrInt, columnPathInstrumentationAttrDouble, columnPathInstrumentationAttrBool)
if err != nil {
return nil, errors.Wrap(err, "creating scope attribute iterator")
return nil, errors.Wrap(err, "creating instrumentation attribute iterator")
}

// if no intrinsics and no events then we can just return the attribute iterator
Expand All @@ -783,9 +783,9 @@ func createDistinctScopeIterator(
iters = append(iters, primaryIter)
}

scopeCol := newDistinctValueCollector(mapScopeAttr, "scope")
instrumentationCol := newDistinctValueCollector(mapInstrumentationAttr, "instrumentation")

return parquetquery.NewJoinIterator(DefinitionLevelInstrumentationScope, iters, scopeCol), nil
return parquetquery.NewJoinIterator(DefinitionLevelInstrumentationScope, iters, instrumentationCol), nil
}

func createDistinctResourceIterator(
Expand Down Expand Up @@ -1133,7 +1133,7 @@ func mapSpanAttr(e entry) traceql.Static {
return traceql.NewStaticNil()
}

func mapScopeAttr(e entry) traceql.Static {
func mapInstrumentationAttr(e entry) traceql.Static {
switch e.Key {
case columnPathInstrumentationName, columnPathInstrumentationVersion:
return traceql.NewStaticString(unsafeToString(e.Value.ByteArray()))
Expand Down
2 changes: 1 addition & 1 deletion tempodb/encoding/vparquet4/block_search_tags.go
Original file line number Diff line number Diff line change
Expand Up @@ -346,7 +346,7 @@ func searchStandardTagValues(ctx context.Context, tag traceql.Attribute, pf *par
columnPathInstrumentationAttrBool,
makeIter, keyPred, cb)
if err != nil {
return fmt.Errorf("search scope key values: %w", err)
return fmt.Errorf("search instrumentation key values: %w", err)
}
}

Expand Down
75 changes: 36 additions & 39 deletions tempodb/encoding/vparquet4/block_traceql.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,13 @@ import (
)

var (
pqSpanPool = parquetquery.NewResultPool(1)
pqSpansetPool = parquetquery.NewResultPool(1)
pqTracePool = parquetquery.NewResultPool(1)
pqAttrPool = parquetquery.NewResultPool(1)
pqEventPool = parquetquery.NewResultPool(1)
pqLinkPool = parquetquery.NewResultPool(1)
pqScopePool = parquetquery.NewResultPool(1)
pqSpanPool = parquetquery.NewResultPool(1)
pqSpansetPool = parquetquery.NewResultPool(1)
pqTracePool = parquetquery.NewResultPool(1)
pqAttrPool = parquetquery.NewResultPool(1)
pqEventPool = parquetquery.NewResultPool(1)
pqLinkPool = parquetquery.NewResultPool(1)
pqInstrumentationPool = parquetquery.NewResultPool(1)
)

type attrVal struct {
Expand Down Expand Up @@ -937,18 +937,18 @@ const (
columnPathLinkAttrDouble = "rs.list.element.ss.list.element.Spans.list.element.Links.list.element.Attrs.list.element.ValueDouble.list.element"
columnPathLinkAttrBool = "rs.list.element.ss.list.element.Spans.list.element.Links.list.element.Attrs.list.element.ValueBool.list.element"

otherEntrySpansetKey = "spanset"
otherEntrySpanKey = "span"
otherEntryEventKey = "event"
otherEntryLinkKey = "link"
otherEntryScopeKey = "scope"
otherEntrySpansetKey = "spanset"
otherEntrySpanKey = "span"
otherEntryEventKey = "event"
otherEntryLinkKey = "link"
otherEntryInstrumentationKey = "instrumentation"

// a fake intrinsic scope at the trace lvl
intrinsicScopeTrace = -1
intrinsicScopeSpan = -2
intrinsicScopeEvent = -3
intrinsicScopeLink = -4
intrinsicScopeScope = -5
intrinsicScopeTrace = -1
intrinsicScopeSpan = -2
intrinsicScopeEvent = -3
intrinsicScopeLink = -4
intrinsicScopeInstrumentation = -5
)

// todo: scope is the only field used here. either remove the other fields or use them.
Expand Down Expand Up @@ -982,8 +982,8 @@ var intrinsicColumnLookups = map[traceql.Intrinsic]struct {
traceql.IntrinsicLinkTraceID: {intrinsicScopeLink, traceql.TypeString, columnPathLinkTraceID},
traceql.IntrinsicLinkSpanID: {intrinsicScopeLink, traceql.TypeString, columnPathLinkSpanID},

traceql.IntrinsicInstrumentationName: {intrinsicScopeScope, traceql.TypeString, columnPathInstrumentationName},
traceql.IntrinsicInstrumentationVersion: {intrinsicScopeScope, traceql.TypeString, columnPathInstrumentationVersion},
traceql.IntrinsicInstrumentationName: {intrinsicScopeInstrumentation, traceql.TypeString, columnPathInstrumentationName},
traceql.IntrinsicInstrumentationVersion: {intrinsicScopeInstrumentation, traceql.TypeString, columnPathInstrumentationVersion},

traceql.IntrinsicServiceStats: {intrinsicScopeTrace, traceql.TypeNil, ""}, // Not a real column, this entry is only used to assign default scope.
}
Expand Down Expand Up @@ -1548,12 +1548,12 @@ func fetch(ctx context.Context, req traceql.FetchSpansRequest, pf *parquet.File,
}

type categorizedConditions struct {
span []traceql.Condition
scope []traceql.Condition
resource []traceql.Condition
trace []traceql.Condition
event []traceql.Condition
link []traceql.Condition
span []traceql.Condition
instrumentation []traceql.Condition
resource []traceql.Condition
trace []traceql.Condition
event []traceql.Condition
link []traceql.Condition
}

// categorizeConditions categorizes conditions by scope
Expand Down Expand Up @@ -1592,8 +1592,8 @@ func categorizeConditions(conditions []traceql.Condition) (*categorizedCondition
case intrinsicScopeTrace:
categorizedCond.trace = append(categorizedCond.trace, cond)

case traceql.AttributeScopeInstrumentation, intrinsicScopeScope:
categorizedCond.scope = append(categorizedCond.scope, cond)
case traceql.AttributeScopeInstrumentation, intrinsicScopeInstrumentation:
categorizedCond.instrumentation = append(categorizedCond.instrumentation, cond)

default:
return nil, false, fmt.Errorf("unsupported traceql scope: %s", cond.Attribute)
Expand Down Expand Up @@ -1657,12 +1657,12 @@ func createAllIterator(ctx context.Context, primaryIter parquetquery.Iterator, c
return nil, fmt.Errorf("creating span iterator: %w", err)
}

scopeIter, err := createScopeIterator(makeIter, spanIter, catConditions.scope, allConditions, selectAll)
instrumentationIter, err := createInstrumentationIterator(makeIter, spanIter, catConditions.instrumentation, allConditions, selectAll)
if err != nil {
return nil, fmt.Errorf("creating scope iterator: %w", err)
}

resourceIter, err := createResourceIterator(makeIter, scopeIter, catConditions.resource, batchRequireAtLeastOneMatchOverall, allConditions, dc, selectAll)
resourceIter, err := createResourceIterator(makeIter, instrumentationIter, catConditions.resource, batchRequireAtLeastOneMatchOverall, allConditions, dc, selectAll)
if err != nil {
return nil, fmt.Errorf("creating resource iterator: %w", err)
}
Expand Down Expand Up @@ -2102,10 +2102,7 @@ func createSpanIterator(makeIter makeIterFn, innerIterators []parquetquery.Itera
return parquetquery.NewLeftJoinIterator(DefinitionLevelResourceSpansILSSpan, required, iters, spanCol, parquetquery.WithPool(pqSpanPool))
}

// createResourceIterator iterates through all resourcespans-level (batch-level) columns, groups them into rows representing
// one batch each. It builds on top of the span iterator, and turns the groups of spans and resource-level values into
// spansets. Spansets are returned that match any of the given conditions.
func createScopeIterator(makeIter makeIterFn, spanIterator parquetquery.Iterator, conditions []traceql.Condition, allConditions, selectAll bool) (parquetquery.Iterator, error) {
func createInstrumentationIterator(makeIter makeIterFn, spanIterator parquetquery.Iterator, conditions []traceql.Condition, allConditions, selectAll bool) (parquetquery.Iterator, error) {
var (
iters = []parquetquery.Iterator{}
genericConditions []traceql.Condition
Expand Down Expand Up @@ -2138,7 +2135,7 @@ func createScopeIterator(makeIter makeIterFn, spanIterator parquetquery.Iterator

if selectAll {
for _, entry := range intrinsicColumnLookups {
if entry.scope != intrinsicScopeScope {
if entry.scope != intrinsicScopeInstrumentation {
continue
}
iters = append(iters, makeIter(entry.columnPath, nil, entry.columnPath))
Expand All @@ -2148,7 +2145,7 @@ func createScopeIterator(makeIter makeIterFn, spanIterator parquetquery.Iterator
attrIter, err := createAttributeIterator(makeIter, genericConditions, DefinitionLevelInstrumentationScopeAttrs,
columnPathInstrumentationAttrKey, columnPathInstrumentationAttrString, columnPathInstrumentationAttrInt, columnPathInstrumentationAttrDouble, columnPathInstrumentationAttrBool, allConditions, selectAll)
if err != nil {
return nil, fmt.Errorf("creating scope attribute iterator: %w", err)
return nil, fmt.Errorf("creating instrumentation attribute iterator: %w", err)
}
if attrIter != nil {
iters = append(iters, attrIter)
Expand All @@ -2163,7 +2160,7 @@ func createScopeIterator(makeIter makeIterFn, spanIterator parquetquery.Iterator
}
minCount = len(distinct)
}
scopeCol := newInstrumentationCollector(minCount)
instrumentationCol := newInstrumentationCollector(minCount)

var required []parquetquery.Iterator

Expand All @@ -2181,13 +2178,13 @@ func createScopeIterator(makeIter makeIterFn, spanIterator parquetquery.Iterator
// Left join here means the span iterator + 1 are required,
// and all other resource conditions are optional. Whatever matches
// is returned.
return parquetquery.NewLeftJoinIterator(DefinitionLevelInstrumentationScope, required, iters, scopeCol, parquetquery.WithPool(pqScopePool))
return parquetquery.NewLeftJoinIterator(DefinitionLevelInstrumentationScope, required, iters, instrumentationCol, parquetquery.WithPool(pqInstrumentationPool))
}

// createResourceIterator iterates through all resourcespans-level (batch-level) columns, groups them into rows representing
// one batch each. It builds on top of the span iterator, and turns the groups of spans and resource-level values into
// spansets. Spansets are returned that match any of the given conditions.
func createResourceIterator(makeIter makeIterFn, scopeIterator parquetquery.Iterator, conditions []traceql.Condition, requireAtLeastOneMatchOverall, allConditions bool, dedicatedColumns backend.DedicatedColumns, selectAll bool) (parquetquery.Iterator, error) {
func createResourceIterator(makeIter makeIterFn, instrumentationIterator parquetquery.Iterator, conditions []traceql.Condition, requireAtLeastOneMatchOverall, allConditions bool, dedicatedColumns backend.DedicatedColumns, selectAll bool) (parquetquery.Iterator, error) {
var (
columnSelectAs = map[string]string{}
columnPredicates = map[string][]parquetquery.Predicate{}
Expand Down Expand Up @@ -2298,7 +2295,7 @@ func createResourceIterator(makeIter makeIterFn, scopeIterator parquetquery.Iter

// Put span iterator last so it is only read when
// the resource conditions are met.
required = append(required, scopeIterator)
required = append(required, instrumentationIterator)

// Left join here means the span iterator + 1 are required,
// and all other resource conditions are optional. Whatever matches
Expand Down
6 changes: 3 additions & 3 deletions tempodb/encoding/vparquet4/block_traceql_meta_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -710,7 +710,7 @@ func TestBackendBlockSearchFetchMetaData(t *testing.T) {
),
},
{
"Intrinsic scope name lookup",
"Intrinsic instrumentation name lookup",
makeReq(
parse(t, `{instrumentation:name = "scope-1"}`), //
),
Expand Down Expand Up @@ -743,7 +743,7 @@ func TestBackendBlockSearchFetchMetaData(t *testing.T) {
),
},
{
"Intrinsic scope version lookup",
"Intrinsic instrumentation version lookup",
makeReq(
parse(t, `{instrumentation:version = "version-1"}`), //
),
Expand Down Expand Up @@ -776,7 +776,7 @@ func TestBackendBlockSearchFetchMetaData(t *testing.T) {
),
},
{
"Scope attribute lookup",
"Instrumentation attribute lookup",
makeReq(
parse(t, `{instrumentation.scope-attr-int = 101}`), //
),
Expand Down
14 changes: 7 additions & 7 deletions tempodb/encoding/vparquet4/block_traceql_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -841,19 +841,19 @@ func flattenForSelectAll(tr *Trace, dcm dedicatedColumnMapping) *traceql.Spanset
sortAttrs(rsAttrs)

for _, ss := range rs.ScopeSpans {
var scopeAttrs []attrVal
scopeAttrs = append(scopeAttrs, attrVal{traceql.IntrinsicInstrumentationNameAttribute, traceql.NewStaticString(ss.Scope.Name)})
scopeAttrs = append(scopeAttrs, attrVal{traceql.IntrinsicInstrumentationVersionAttribute, traceql.NewStaticString(ss.Scope.Version)})
var instrumentationAttrs []attrVal
instrumentationAttrs = append(instrumentationAttrs, attrVal{traceql.IntrinsicInstrumentationNameAttribute, traceql.NewStaticString(ss.Scope.Name)})
instrumentationAttrs = append(instrumentationAttrs, attrVal{traceql.IntrinsicInstrumentationVersionAttribute, traceql.NewStaticString(ss.Scope.Version)})
for _, a := range parquetToProtoAttrs(ss.Scope.Attrs) {
if arr := a.Value.GetArrayValue(); arr != nil {
for _, v := range arr.Values {
scopeAttrs = append(scopeAttrs, attrVal{traceql.NewScopedAttribute(traceql.AttributeScopeInstrumentation, false, a.Key), traceql.StaticFromAnyValue(v)})
instrumentationAttrs = append(instrumentationAttrs, attrVal{traceql.NewScopedAttribute(traceql.AttributeScopeInstrumentation, false, a.Key), traceql.StaticFromAnyValue(v)})
}
continue
}
scopeAttrs = append(scopeAttrs, attrVal{traceql.NewScopedAttribute(traceql.AttributeScopeInstrumentation, false, a.Key), traceql.StaticFromAnyValue(a.Value)})
instrumentationAttrs = append(instrumentationAttrs, attrVal{traceql.NewScopedAttribute(traceql.AttributeScopeInstrumentation, false, a.Key), traceql.StaticFromAnyValue(a.Value)})
}
sortAttrs(scopeAttrs)
sortAttrs(instrumentationAttrs)

for _, s := range ss.Spans {

Expand All @@ -863,7 +863,7 @@ func flattenForSelectAll(tr *Trace, dcm dedicatedColumnMapping) *traceql.Spanset
newS.durationNanos = s.DurationNano
newS.setTraceAttrs(traceAttrs)
newS.setResourceAttrs(rsAttrs)
newS.setInstrumentationAttrs(scopeAttrs)
newS.setInstrumentationAttrs(instrumentationAttrs)
newS.addSpanAttr(traceql.IntrinsicDurationAttribute, traceql.NewStaticDuration(time.Duration(s.DurationNano)))
newS.addSpanAttr(traceql.IntrinsicKindAttribute, traceql.NewStaticKind(otlpKindToTraceqlKind(uint64(s.Kind))))
newS.addSpanAttr(traceql.IntrinsicNameAttribute, traceql.NewStaticString(s.Name))
Expand Down
6 changes: 3 additions & 3 deletions tempodb/tempodb_search_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,8 @@ func TestSearchCompleteBlock(t *testing.T) {
)
})
if vers == vparquet4.VersionString {
t.Run("event/link/scope query", func(t *testing.T) {
runEventLinkScopeSearchTest(t, vers)
t.Run("event/link/instrumentation query", func(t *testing.T) {
runEventLinkInstrumentationSearchTest(t, vers)
})
}
}
Expand Down Expand Up @@ -1658,7 +1658,7 @@ func runCompleteBlockSearchTest(t *testing.T, blockVersion string, runners ...ru
// todo: do some compaction and then call runner again
}

func runEventLinkScopeSearchTest(t *testing.T, blockVersion string) {
func runEventLinkInstrumentationSearchTest(t *testing.T, blockVersion string) {
// only run this test for vparquet4
if blockVersion != vparquet4.VersionString {
return
Expand Down

0 comments on commit 01da898

Please sign in to comment.