cleanup

Signed-off-by: Joe Elliott <[email protected]>

pkg/traceql/ast_execute.go

@@ -468,30 +468,6 @@ func (a Attribute) execute(span Span) (Static, error) {
 		return static, nil
 	}
 
-	// atts := span.Attributes()
-	// static, ok := atts[a]
-	// if ok {
-	// 	return static, nil
-	// }
-
-	// if the requested attribute has a scope none then we will check first for span attributes matching
-	// then any attributes matching. we don't need to both if this is an intrinsic b/c those will always
-	// be caught above if they exist
-
-	// jpe - replicate this in the implementation?
-	// if a.Scope == AttributeScopeNone && a.Intrinsic == IntrinsicNone {
-	// 	for attribute, static := range atts {
-	// 		if a.Name == attribute.Name && attribute.Scope == AttributeScopeSpan {
-	// 			return static, nil
-	// 		}
-	// 	}
-	// 	for attribute, static := range atts {
-	// 		if a.Name == attribute.Name {
-	// 			return static, nil
-	// 		}
-	// 	}
-	// }
-
 	return NewStaticNil(), nil
 }
 

pkg/traceql/ast_test.go

@@ -372,7 +372,17 @@ func (m *mockSpan) WithAttrBool(key string, value bool) *mockSpan {
 }
 
 func (m *mockSpan) AttributeFor(a Attribute) (Static, bool) {
-	s, ok := m.attributes[a] // jpe - need fancy resource/span logic?
+	s, ok := m.attributes[a]
+	// if not found explicitly, check if it's a span attribute
+	if !ok {
+		a.Scope = AttributeScopeSpan
+		s, ok = m.attributes[a]
+	}
+	// if not found explicitly, check if it's a resource attribute
+	if !ok {
+		a.Scope = AttributeScopeResource
+		s, ok = m.attributes[a]
+	}
 	return s, ok
 }
 

pkg/traceql/engine.go

@@ -179,7 +179,7 @@ func (e *Engine) ExecuteTagValues(
 	case AttributeScopeResource,
 		AttributeScopeSpan: // If tag is scoped, we can check the map directly
 		collectAttributeValue = func(s Span) bool {
-			if v, ok := s.AttributeFor(tag); ok { // jpe - does this work?
+			if v, ok := s.AttributeFor(tag); ok { // jpe - more permissive - note in the PR
 				return cb(v)
 			}
 			return false

pkg/traceql/storage.go

@@ -80,7 +80,9 @@ type Span interface {
 	// AttributeFor returns the attribute for the given key. If the attribute is not found then
 	// the second return value will be false.
 	AttributeFor(Attribute) (Static, bool)
-	AllAttributes() map[Attribute]Static // jpe - comment. don't call this. do i need this for metadata?
+	// AllAttributes returns a map of all attributes for this span. AllAttributes should be used sparingly
+	// and is expected to be significantly slower than AttributeFor.
+	AllAttributes() map[Attribute]Static
 
 	ID() []byte
 	StartTimeUnixNanos() uint64

pkg/traceqlmetrics/metrics.go

@@ -309,7 +309,7 @@ func GetMetrics(ctx context.Context, query, groupBy string, spanLimit int, start
 
 func lookup(needles []traceql.Attribute, span traceql.Span) traceql.Static {
 	for _, n := range needles {
-		if v, ok := span.AttributeFor(n); ok { // jpe is this equivalent?
+		if v, ok := span.AttributeFor(n); ok { // jpe is this equivalent? no - more permissive. note in the PR
 			return v
 		}
 	}

tempodb/encoding/versioned.go

@@ -84,9 +84,9 @@ func LatestEncoding() VersionedEncoding {
 // AllEncodings returns all encodings
 func AllEncodings() []VersionedEncoding {
 	return []VersionedEncoding{
-		// v2.Encoding{},
-		// vparquet.Encoding{},
-		// vparquet2.Encoding{},
+		v2.Encoding{},
+		vparquet.Encoding{},
+		vparquet2.Encoding{},
 		vparquet3.Encoding{},
 	}
 }

tempodb/encoding/vparquet/block_traceql.go

@@ -50,7 +50,7 @@ func (s *span) AttributeFor(a traceql.Attribute) (traceql.Static, bool) {
 	// then any attributes matching. we don't need to both if this is an intrinsic b/c those will always
 	// be caught above if they exist
 	if a.Scope == traceql.AttributeScopeNone && a.Intrinsic == traceql.IntrinsicNone {
-		for attribute, static := range atts { // jpe - map iteration is slow
+		for attribute, static := range atts {
 			if a.Name == attribute.Name && attribute.Scope == traceql.AttributeScopeSpan {
 				return static, true
 			}

tempodb/encoding/vparquet2/block_traceql.go

@@ -52,7 +52,7 @@ func (s *span) AttributeFor(a traceql.Attribute) (traceql.Static, bool) {
 	// then any attributes matching. we don't need to both if this is an intrinsic b/c those will always
 	// be caught above if they exist
 	if a.Scope == traceql.AttributeScopeNone && a.Intrinsic == traceql.IntrinsicNone {
-		for attribute, static := range atts { // jpe - map iteration is slow
+		for attribute, static := range atts {
 			if a.Name == attribute.Name && attribute.Scope == traceql.AttributeScopeSpan {
 				return static, true
 			}

tempodb/encoding/vparquet3/block_traceql.go

@@ -30,7 +30,7 @@ type stattr struct { // jpe - static attr, get it ?
 
 // span implements traceql.Span
 type span struct {
-	spanAttrs     []stattr // jpe add intrinsic attrs? this might be even faster with a slice. the hard part is on insertion we need to overwrite
+	spanAttrs     []stattr
 	resourceAttrs []stattr
 	traceAttrs    []stattr
 
@@ -50,12 +50,21 @@ type span struct {
 func (s *span) AllAttributes() map[traceql.Attribute]traceql.Static {
 	atts := make(map[traceql.Attribute]traceql.Static, len(s.spanAttrs)+len(s.resourceAttrs)+len(s.traceAttrs))
 	for _, st := range s.traceAttrs {
+		if st.s.Type == traceql.TypeNil {
+			continue
+		}
 		atts[st.a] = st.s
 	}
 	for _, st := range s.resourceAttrs {
+		if st.s.Type == traceql.TypeNil {
+			continue
+		}
 		atts[st.a] = st.s
 	}
 	for _, st := range s.spanAttrs {
+		if st.s.Type == traceql.TypeNil {
+			continue
+		}
 		atts[st.a] = st.s
 	}
 	return atts
@@ -307,16 +316,15 @@ func (s *span) ChildOf(lhs []traceql.Span, rhs []traceql.Span, falseForAll bool,
 }
 
 func (s *span) addSpanAttr(a traceql.Attribute, st traceql.Static) {
-	s.spanAttrs = append(s.spanAttrs, stattr{a: a, s: st}) // jpex
+	s.spanAttrs = append(s.spanAttrs, stattr{a: a, s: st})
 }
 
 func (s *span) setResourceAttrs(attrs []stattr) {
-	// jpe - i assume this honors cap and such
 	s.resourceAttrs = append(s.resourceAttrs, attrs...)
 }
 
 func (s *span) setTraceAttrs(attrs []stattr) {
-	s.traceAttrs = append(s.resourceAttrs, attrs...)
+	s.traceAttrs = append(s.traceAttrs, attrs...)
 }
 
 // attributesMatched counts all attributes in the map as well as metadata fields like start/end/id
@@ -363,11 +371,7 @@ func (s *span) attributesMatched() int {
 // can return a slice of dropped and kept spansets?
 var spanPool = sync.Pool{
 	New: func() interface{} {
-		return &span{
-			//spanAttrs: make(map[traceql.Attribute]traceql.Static),
-			//resourceAttrs: make(map[traceql.Attribute]traceql.Static), jpe - don't make trace or resource attrs
-			//traceAttrs:    make(map[traceql.Attribute]traceql.Static),
-		}
+		return &span{}
 	},
 }
 
@@ -384,8 +388,6 @@ func putSpan(s *span) {
 	s.spanAttrs = s.spanAttrs[:0]
 	s.resourceAttrs = s.resourceAttrs[:0]
 	s.traceAttrs = s.traceAttrs[:0]
-	// clear(s.resourceAttrs) jpe - can't clear trace and resource attrs b/c they are shared
-	// clear(s.traceAttrs)
 
 	spanPool.Put(s)
 }
@@ -1949,7 +1951,7 @@ func (c *batchCollector) String() string {
 func (c *batchCollector) KeepGroup(res *parquetquery.IteratorResult) bool {
 	// First pass over spans and attributes from the AttributeCollector
 	spans := res.OtherEntries[:0]
-	c.resAttrs = c.resAttrs[:0] // jpe - cheaper to keep a buffer on batch collector and copy?
+	c.resAttrs = c.resAttrs[:0]
 
 	for _, kv := range res.OtherEntries {
 		switch v := kv.Value.(type) {