pkg/util/tracing: Add hidden tag group, make server responsible for s…

…orting.

Release note: none

This moves all tags marked as "hidden" into a single tag group at the UI layer.
This declutters the trace page a little bit and makes it easier to pick out
more important information.

pkg/kv/test_utils.go

@@ -27,7 +27,7 @@ func OnlyFollowerReads(rec tracingpb.Recording) bool {
 		if sp.Operation != "/cockroach.roachpb.Internal/Batch" {
 			continue
 		}
-		anonTagGroup := sp.FindTagGroup("")
+		anonTagGroup := sp.FindTagGroup(tracingpb.AnonymousTagGroupName)
 		if anonTagGroup == nil {
 			continue
 		}

pkg/sql/exec_util.go

@@ -2627,7 +2627,7 @@ func getMessagesForSubtrace(
 
 	for _, tg := range span.TagGroups {
 		var prefix string
-		if tg.Name != "" {
+		if tg.Name != tracingpb.AnonymousTagGroupName {
 			prefix = fmt.Sprintf("%s-", tg.Name)
 		}
 		for _, tag := range tg.Tags {

pkg/sql/trace_test.go

@@ -638,7 +638,7 @@ func TestTraceDistSQL(t *testing.T) {
 	require.True(t, ok, "table reader span not found")
 	require.Empty(t, rec.OrphanSpans())
 	// Check that the table reader indeed came from a remote note.
-	anonTagGroup := sp.FindTagGroup("")
+	anonTagGroup := sp.FindTagGroup(tracingpb.AnonymousTagGroupName)
 	require.NotNil(t, anonTagGroup)
 	val, ok := anonTagGroup.FindTag("node")
 	require.True(t, ok)

pkg/ui/workspaces/db-console/src/views/tracez/tracez.tsx

@@ -157,12 +157,12 @@ const TagBadge = ({
   let badgeStatus: BadgeStatus;
   if (status) {
     badgeStatus = status;
+  } else if (t.hidden) {
+    badgeStatus = "default";
   } else if (isExpandable) {
     badgeStatus = "warning";
-  } else if (!t.hidden) {
-    badgeStatus = "info";
   } else {
-    badgeStatus = "default";
+    badgeStatus = "info";
   }
   return (
     <Button
@@ -216,7 +216,6 @@ const TagCell = (props: {
 }) => {
   const [expandedTagIndex, setExpandedTagIndex] = useState<number>(-1);
   const processedTags = props.sr.span.processed_tags;
-  const orderedTags = _.sortBy(processedTags, t => t.hidden);
 
   // Pad 8px on top and bottom.
   //
@@ -230,7 +229,7 @@ const TagCell = (props: {
   return (
     <div className={"outer-row"}>
       <div className={"inner-row"}>
-        {orderedTags.map((t, i) => (
+        {processedTags.map((t, i) => (
           <TagBadge
             t={t}
             setSearch={props.setSearch}
@@ -248,11 +247,13 @@ const TagCell = (props: {
       </div>
       {expandedTagIndex != -1 && (
         <div className={"inner-row"}>
-          {orderedTags[expandedTagIndex].children.map((t, i) => (
+          {processedTags[expandedTagIndex].children.map((t, i) => (
             <TagBadge
               t={t}
               key={i}
-              status="warning"
+              status={
+                processedTags[expandedTagIndex].hidden ? "default" : "warning"
+              }
               setSearch={props.setSearch}
               isExpanded={false}
             />
@@ -454,7 +455,6 @@ export const Tracez = () => {
       });
     });
   };
-
   return (
     <div>
       {showTrace && currentTrace ? (

pkg/util/tracing/crdbspan.go

@@ -870,17 +870,11 @@ func (s *crdbSpan) getRecordingNoChildrenLocked(
 		}
 		rs.Tags[k] = v
 
-		var tagGroup *tracingpb.TagGroup
-		for i, tg := range rs.TagGroups {
-			if tg.Name == "" {
-				tagGroup = &rs.TagGroups[i]
-				break
-			}
-		}
-
+		tagGroup := rs.FindTagGroup(tracingpb.AnonymousTagGroupName)
 		if tagGroup == nil {
-			tagGroup = addTagGroup("")
+			tagGroup = addTagGroup(tracingpb.AnonymousTagGroupName)
 		}
+
 		tagGroup.Tags = append(tagGroup.Tags, tracingpb.Tag{
 			Key:   k,
 			Value: v,
@@ -952,6 +946,13 @@ func (s *crdbSpan) getRecordingNoChildrenLocked(
 				addTag(kv.Key, fmt.Sprintf("<can't render %T>", kv.Value))
 			}
 		}
+
+		// Pull anonymous group to the front.
+		for i := range rs.TagGroups {
+			if rs.TagGroups[i].Name == tracingpb.AnonymousTagGroupName {
+				rs.TagGroups[0], rs.TagGroups[i] = rs.TagGroups[i], rs.TagGroups[0]
+			}
+		}
 	}
 
 	if numLogs := s.mu.recording.logs.Len(); numLogs != 0 {

pkg/util/tracing/grpcinterceptor/grpc_interceptor_test.go

@@ -266,22 +266,22 @@ func TestGRPCInterceptors(t *testing.T) {
 				// it's not worth having to have a separate expectation for each.
 				// Note that we check that we're not leaking spans at the end of
 				// the test.
-				anonymousTagGroup := rec.FindTagGroup("")
+				anonymousTagGroup := rec.FindTagGroup(tracingpb.AnonymousTagGroupName)
 				if anonymousTagGroup == nil {
 					continue
 				}
 
-				filteredAnonymousTags := make([]tracingpb.Tag, 0)
+				filteredAnonymousTagGroup := make([]tracingpb.Tag, 0)
 				for _, tag := range anonymousTagGroup.Tags {
 					if tag.Key == "_unfinished" {
 						continue
 					}
 					if tag.Key == "_verbose" {
 						continue
 					}
-					filteredAnonymousTags = append(filteredAnonymousTags, tag)
+					filteredAnonymousTagGroup = append(filteredAnonymousTagGroup, tag)
 				}
-				anonymousTagGroup.Tags = filteredAnonymousTags
+				anonymousTagGroup.Tags = filteredAnonymousTagGroup
 			}
 			require.Equal(t, 1, n)
 

pkg/util/tracing/span_test.go

@@ -301,7 +301,7 @@ func TestSpanRecordStructuredLimit(t *testing.T) {
 	rec := sp.GetRecording(tracingpb.RecordingVerbose)
 	require.Len(t, rec, 1)
 	require.Len(t, rec[0].StructuredRecords, numStructuredRecordings)
-	val, ok := rec[0].FindTagGroup("").FindTag("_dropped")
+	val, ok := rec[0].FindTagGroup(tracingpb.AnonymousTagGroupName).FindTag("_dropped")
 	require.True(t, ok)
 	require.Equal(t, "1", val)
 
@@ -352,7 +352,7 @@ func TestSpanRecordLimit(t *testing.T) {
 	rec := sp.GetRecording(tracingpb.RecordingVerbose)
 	require.Len(t, rec, 1)
 	require.Len(t, rec[0].Logs, numLogs)
-	val, ok := rec[0].FindTagGroup("").FindTag("_dropped")
+	val, ok := rec[0].FindTagGroup(tracingpb.AnonymousTagGroupName).FindTag("_dropped")
 	require.True(t, ok)
 	require.Equal(t, val, "1")
 
@@ -612,7 +612,7 @@ func TestSpanTags(t *testing.T) {
 
 	rec := sp.GetRecording(tracingpb.RecordingVerbose)
 
-	anonTagGroup := rec[0].FindTagGroup("")
+	anonTagGroup := rec[0].FindTagGroup(tracingpb.AnonymousTagGroupName)
 	_, ok = anonTagGroup.FindTag("tag")
 	require.True(t, ok)
 
@@ -657,8 +657,8 @@ func TestSpanTagsInRecordings(t *testing.T) {
 	require.Len(t, rec, 1)
 
 	require.Len(t, rec[0].TagGroups, 1)
-	anonTagGroup := rec[0].FindTagGroup("")
-	require.Len(t, anonTagGroup.Tags, 5) // _unfinished:1 _verbose:1 foo:tagbar foo1:1 foor2:bar2
+	anonTagGroup := rec[0].FindTagGroup(tracingpb.AnonymousTagGroupName)
+	require.Len(t, anonTagGroup.Tags, 5) // foo:tagbar foo1:1 foor2:bar2 _unfinished:1 _verbose:1
 
 	_, ok := anonTagGroup.FindTag("foo")
 	require.True(t, ok)
@@ -674,7 +674,7 @@ func TestSpanTagsInRecordings(t *testing.T) {
 	require.Len(t, rec, 1)
 
 	require.Len(t, rec[0].TagGroups, 1)
-	anonTagGroup = rec[0].FindTagGroup("")
+	anonTagGroup = rec[0].FindTagGroup(tracingpb.AnonymousTagGroupName)
 	require.Len(t, anonTagGroup.Tags, 6)
 
 	_, ok = anonTagGroup.FindTag("foo3")
@@ -692,27 +692,27 @@ func TestVerboseTag(t *testing.T) {
 
 	sp.SetRecordingType(tracingpb.RecordingStructured)
 	rec := sp.GetRecording(tracingpb.RecordingVerbose)
-	anonymousTagGroup := rec[0].FindTagGroup("")
-	ok := anonymousTagGroup != nil
+	anonTagGroup := rec[0].FindTagGroup(tracingpb.AnonymousTagGroupName)
+	ok := anonTagGroup != nil
 	if ok {
-		_, ok = anonymousTagGroup.FindTag("_verbose")
+		_, ok = anonTagGroup.FindTag("_verbose")
 	}
 	require.False(t, ok)
 
 	// The tag is present while the span is recording verbosely.
 	sp.SetRecordingType(tracingpb.RecordingVerbose)
 	rec = sp.GetRecording(tracingpb.RecordingVerbose)
 
-	_, ok = rec[0].FindTagGroup("").FindTag("_verbose")
+	_, ok = rec[0].FindTagGroup(tracingpb.AnonymousTagGroupName).FindTag("_verbose")
 	require.True(t, ok)
 
 	// After we stop recording, the tag goes away.
 	sp.SetRecordingType(tracingpb.RecordingStructured)
 	rec = sp.GetRecording(tracingpb.RecordingVerbose)
-	anonymousTagGroup = rec[0].FindTagGroup("")
-	ok = anonymousTagGroup != nil
+	anonTagGroup = rec[0].FindTagGroup(tracingpb.AnonymousTagGroupName)
+	ok = anonTagGroup != nil
 	if ok {
-		_, ok = anonymousTagGroup.FindTag("_verbose")
+		_, ok = anonTagGroup.FindTag("_verbose")
 	}
 	require.False(t, ok)
 }

pkg/util/tracing/tracingpb/recorded_span.go

@@ -20,6 +20,11 @@ import (
 	types "github.com/gogo/protobuf/types"
 )
 
+const (
+	// AnonymousTagGroupName is the name of the anonymous tag group.
+	AnonymousTagGroupName = ""
+)
+
 // TraceID is a probabilistically-unique id, shared by all spans in a trace.
 type TraceID uint64
 

pkg/util/tracing/tracingpb/recording.go

@@ -262,7 +262,7 @@ func (r Recording) visitSpan(sp RecordedSpan, depth int) []traceLogData {
 
 	for _, tg := range sp.TagGroups {
 		var prefix string
-		if tg.Name != "" {
+		if tg.Name != AnonymousTagGroupName {
 			prefix = fmt.Sprintf("%s-", tg.Name)
 		}
 		for _, tag := range tg.Tags {
@@ -457,7 +457,7 @@ func (r Recording) ToJaegerJSON(stmt, comment, nodeStr string) (string, error) {
 		for _, tagGroup := range sp.TagGroups {
 			for _, tag := range tagGroup.Tags {
 				var prefix string
-				if tagGroup.Name != "" {
+				if tagGroup.Name != AnonymousTagGroupName {
 					prefix = fmt.Sprintf("%s-", tagGroup.Name)
 				}
 				s.Tags = append(s.Tags, jaegerjson.KeyValue{

pkg/util/tracing/tracingui/span_registry_ui.go

@@ -98,6 +98,10 @@ type ProcessedSnapshot struct {
 	Stacks map[int]string // GoroutineID to stack trace
 }
 
+const (
+	hiddenTagGroupName = "..."
+)
+
 var hiddenTags = map[string]struct{}{
 	"_unfinished": {},
 	"_verbose":    {},
@@ -148,6 +152,15 @@ type ProcessedChildTag struct {
 	Key, Val string
 }
 
+func appendRespectingHiddenGroup(tags []ProcessedTag, tag ProcessedTag) []ProcessedTag {
+	tags = append(tags, tag)
+	endIndex := len(tags) - 1
+	if tags[endIndex-1].Key == hiddenTagGroupName {
+		tags[endIndex], tags[endIndex-1] = tags[endIndex-1], tags[endIndex]
+	}
+	return tags
+}
+
 // propagateTagUpwards copies tag from sp to all of sp's ancestors.
 func propagateTagUpwards(tag ProcessedTag, sp *processedSpan, spans map[uint64]*processedSpan) {
 	tag.CopiedFromChild = true
@@ -158,7 +171,7 @@ func propagateTagUpwards(tag ProcessedTag, sp *processedSpan, spans map[uint64]*
 		if !ok {
 			return
 		}
-		p.Tags = append(p.Tags, tag)
+		p.Tags = appendRespectingHiddenGroup(p.Tags, tag)
 		parentID = p.ParentSpanID
 	}
 }
@@ -170,7 +183,7 @@ func propagateInheritTagDownwards(
 	tag.Inherited = true
 	tag.Hidden = true
 	for _, child := range children[sp.SpanID] {
-		child.Tags = append(child.Tags, tag)
+		child.Tags = appendRespectingHiddenGroup(child.Tags, tag)
 		propagateInheritTagDownwards(tag, child, children)
 	}
 }
@@ -188,21 +201,39 @@ func processSpan(s tracingpb.RecordedSpan, snap tracing.SpansSnapshot) processed
 	}
 
 	p.Tags = make([]ProcessedTag, 0)
+
+	// Added to p.Tags if iteration populates the Children.
+	hiddenTag := ProcessedTag{
+		Key:      hiddenTagGroupName,
+		Hidden:   true,
+		Children: make([]ProcessedChildTag, 0),
+	}
+
 	for _, tagGroup := range s.TagGroups {
 		key := tagGroup.Name
 
-		if key == "" {
-			// The anonymous tag group. Each tag should be treated as top-level.
+		if key == tracingpb.AnonymousTagGroupName {
+			// The anonymous tag group.
+			// Non-hidden should be treated as top-level.
+			// Hidden tags should be moved to the hidden tag group.
 			for _, tagKV := range tagGroup.Tags {
-				p.Tags = append(p.Tags, processTag(tagKV.Key, tagKV.Value, snap))
+				if _, hidden := hiddenTags[tagKV.Key]; hidden {
+					hiddenTag.Children = append(hiddenTag.Children, ProcessedChildTag{
+						Key: tagKV.Key,
+						Val: tagKV.Value,
+					})
+				} else {
+					p.Tags = append(p.Tags, processTag(tagKV.Key, tagKV.Value, snap))
+				}
 			}
 		} else {
 			// A named tag group. Each tag should be treated as a child of this parent.
 			processedParentTag := ProcessedTag{
 				// Don't actually need to call processTag() here, none of the checks
 				// will apply to a tag group.
-				Key: key,
-				Val: "",
+				Key:    key,
+				Val:    "",
+				Hidden: false,
 			}
 			processedParentTag.Children = make([]ProcessedChildTag, len(tagGroup.Tags))
 			for i, tag := range tagGroup.Tags {
@@ -213,22 +244,21 @@ func processSpan(s tracingpb.RecordedSpan, snap tracing.SpansSnapshot) processed
 			}
 			p.Tags = append(p.Tags, processedParentTag)
 		}
-
 	}
-
+	if len(hiddenTag.Children) != 0 {
+		p.Tags = append(p.Tags, hiddenTag)
+	}
 	return p
 }
 
-// processTag massages span tags for presentation in the UI. It marks some tags
-// as hidden, it marks some tags to be inherited by child spans, and it expands
-// lock contention tags with information about the lock holder txn.
+// processTag massages span tags for presentation in the UI. It marks some
+// tags to be inherited by child spans, and it expands lock contention tags
+// with information about the lock holder txn.
 func processTag(k, v string, snap tracing.SpansSnapshot) ProcessedTag {
 	p := ProcessedTag{
 		Key: k,
 		Val: v,
 	}
-	_, hidden := hiddenTags[k]
-	p.Hidden = hidden
 
 	switch k {
 	case "lock_holder_txn":