Skip to content

Commit 5694c37

Browse files
Parship12Parship12
committed
1. dry the service tests
2. use range with .All() in copySpansUpToLimit 3. argument order to (dest, src, ...)
1 parent b10aa8b commit 5694c37

File tree

3 files changed

+123
-161
lines changed

3 files changed

+123
-161
lines changed

‎cmd/jaeger/internal/extension/jaegerquery/internal/querysvc/v2/querysvc/service_test.go‎

Lines changed: 112 additions & 148 deletions
Original file line numberDiff line numberDiff line change
@@ -624,161 +624,125 @@ func TestGetCapabilities(t *testing.T) {
624624
}
625625
}
626626

627-
func TestMaxTraceSize_UnderLimit(t *testing.T) {
628-
// 3 spans
629-
trace := ptrace.NewTraces()
630-
resources := trace.ResourceSpans().AppendEmpty()
631-
scopes := resources.ScopeSpans().AppendEmpty()
632-
for i := 0; i < 3; i++ {
633-
span := scopes.Spans().AppendEmpty()
634-
span.SetTraceID(testTraceID)
635-
span.SetSpanID(pcommon.SpanID([8]byte{byte(i + 1)}))
636-
span.SetName(fmt.Sprintf("span-%d", i))
637-
}
638-
639-
responseIter := iter.Seq2[[]ptrace.Traces, error](func(yield func([]ptrace.Traces, error) bool) {
640-
yield([]ptrace.Traces{trace}, nil)
641-
})
642-
643-
traceReader := &tracestoremocks.Reader{}
644-
dependencyStorage := &depstoremocks.Reader{}
645-
options := QueryServiceOptions{
646-
MaxTraceSize: 5, // limit is 5, but trace has only 3 spans
647-
}
648-
tqs := &testQueryService{}
649-
tqs.queryService = NewQueryService(traceReader, dependencyStorage, options)
650-
651-
params := GetTraceParams{
652-
TraceIDs: []tracestore.GetTraceParams{{TraceID: testTraceID}},
653-
}
654-
traceReader.On("GetTraces", mock.Anything, params.TraceIDs).
655-
Return(responseIter).Once()
656-
657-
getTracesIter := tqs.queryService.GetTraces(context.Background(), params)
658-
gotTraces, err := jiter.FlattenWithErrors(getTracesIter)
659-
require.NoError(t, err)
660-
require.Len(t, gotTraces, 1)
661-
662-
gotSpans := gotTraces[0].ResourceSpans().At(0).ScopeSpans().At(0).Spans()
663-
require.Equal(t, 3, gotSpans.Len())
664-
665-
// no warning should be present
666-
firstSpan := gotSpans.At(0)
667-
_, hasWarning := firstSpan.Attributes().Get("@jaeger@warnings")
668-
require.False(t, hasWarning)
669-
}
670-
671-
func TestMaxTraceSize_OverLimit(t *testing.T) {
672-
// 5 spans split across 2 batches
673-
trace1 := ptrace.NewTraces()
674-
resources1 := trace1.ResourceSpans().AppendEmpty()
675-
scopes1 := resources1.ScopeSpans().AppendEmpty()
676-
for i := 0; i < 3; i++ {
677-
span := scopes1.Spans().AppendEmpty()
678-
span.SetTraceID(testTraceID)
679-
span.SetSpanID(pcommon.SpanID([8]byte{byte(i + 1)}))
680-
span.SetName(fmt.Sprintf("span-%d", i))
681-
}
682-
683-
trace2 := ptrace.NewTraces()
684-
resources2 := trace2.ResourceSpans().AppendEmpty()
685-
scopes2 := resources2.ScopeSpans().AppendEmpty()
686-
for i := 3; i < 5; i++ {
687-
span := scopes2.Spans().AppendEmpty()
688-
span.SetTraceID(testTraceID)
689-
span.SetSpanID(pcommon.SpanID([8]byte{byte(i + 1)}))
690-
span.SetName(fmt.Sprintf("span-%d", i))
691-
}
692-
693-
responseIter := iter.Seq2[[]ptrace.Traces, error](func(yield func([]ptrace.Traces, error) bool) {
694-
if !yield([]ptrace.Traces{trace1}, nil) {
695-
return
696-
}
697-
yield([]ptrace.Traces{trace2}, nil)
698-
})
699-
700-
traceReader := &tracestoremocks.Reader{}
701-
dependencyStorage := &depstoremocks.Reader{}
702-
options := QueryServiceOptions{
703-
MaxTraceSize: 3, // Limit is 3, but trace has 5 spans total
704-
}
705-
tqs := &testQueryService{}
706-
tqs.queryService = NewQueryService(traceReader, dependencyStorage, options)
707-
708-
params := GetTraceParams{
709-
TraceIDs: []tracestore.GetTraceParams{{TraceID: testTraceID}},
627+
func TestMaxTraceSize(t *testing.T) {
628+
tests := []struct {
629+
name string
630+
maxTraceSize int
631+
createTraces func() []ptrace.Traces
632+
expectedSpans int
633+
expectWarning bool
634+
warningPattern string
635+
}{
636+
{
637+
name: "under_limit",
638+
maxTraceSize: 5,
639+
createTraces: func() []ptrace.Traces {
640+
trace := ptrace.NewTraces()
641+
scopes := trace.ResourceSpans().AppendEmpty().ScopeSpans().AppendEmpty()
642+
for i := 0; i < 3; i++ {
643+
span := scopes.Spans().AppendEmpty()
644+
span.SetTraceID(testTraceID)
645+
span.SetSpanID(pcommon.SpanID([8]byte{byte(i + 1)}))
646+
span.SetName(fmt.Sprintf("span-%d", i))
647+
}
648+
return []ptrace.Traces{trace}
649+
},
650+
expectedSpans: 3,
651+
expectWarning: false,
652+
},
653+
{
654+
name: "over_limit",
655+
maxTraceSize: 3,
656+
createTraces: func() []ptrace.Traces {
657+
trace1 := ptrace.NewTraces()
658+
scopes1 := trace1.ResourceSpans().AppendEmpty().ScopeSpans().AppendEmpty()
659+
for i := 0; i < 3; i++ {
660+
span := scopes1.Spans().AppendEmpty()
661+
span.SetTraceID(testTraceID)
662+
span.SetSpanID(pcommon.SpanID([8]byte{byte(i + 1)}))
663+
span.SetName(fmt.Sprintf("span-%d", i))
664+
}
665+
666+
trace2 := ptrace.NewTraces()
667+
scopes2 := trace2.ResourceSpans().AppendEmpty().ScopeSpans().AppendEmpty()
668+
for i := 3; i < 5; i++ {
669+
span := scopes2.Spans().AppendEmpty()
670+
span.SetTraceID(testTraceID)
671+
span.SetSpanID(pcommon.SpanID([8]byte{byte(i + 1)}))
672+
span.SetName(fmt.Sprintf("span-%d", i))
673+
}
674+
return []ptrace.Traces{trace1, trace2}
675+
},
676+
expectedSpans: 3,
677+
expectWarning: true,
678+
warningPattern: "trace has more than 3 spans",
679+
},
680+
{
681+
name: "exactly_at_limit",
682+
maxTraceSize: 3,
683+
createTraces: func() []ptrace.Traces {
684+
trace := ptrace.NewTraces()
685+
scopes := trace.ResourceSpans().AppendEmpty().ScopeSpans().AppendEmpty()
686+
for i := 0; i < 3; i++ {
687+
span := scopes.Spans().AppendEmpty()
688+
span.SetTraceID(testTraceID)
689+
span.SetSpanID(pcommon.SpanID([8]byte{byte(i + 1)}))
690+
span.SetName(fmt.Sprintf("span-%d", i))
691+
}
692+
return []ptrace.Traces{trace}
693+
},
694+
expectedSpans: 3,
695+
expectWarning: false,
696+
},
710697
}
711-
traceReader.On("GetTraces", mock.Anything, params.TraceIDs).
712-
Return(responseIter).Once()
713698

714-
getTracesIter := tqs.queryService.GetTraces(context.Background(), params)
715-
gotTraces, err := jiter.FlattenWithErrors(getTracesIter)
716-
require.NoError(t, err)
717-
require.Len(t, gotTraces, 1)
699+
for _, tt := range tests {
700+
t.Run(tt.name, func(t *testing.T) {
701+
traces := tt.createTraces()
702+
responseIter := iter.Seq2[[]ptrace.Traces, error](func(yield func([]ptrace.Traces, error) bool) {
703+
for _, trace := range traces {
704+
if !yield([]ptrace.Traces{trace}, nil) {
705+
return
706+
}
707+
}
708+
})
718709

719-
// count total spans in the result
720-
totalSpans := 0
721-
resources := gotTraces[0].ResourceSpans()
722-
for i := 0; i < resources.Len(); i++ {
723-
scopes := resources.At(i).ScopeSpans()
724-
for j := 0; j < scopes.Len(); j++ {
725-
totalSpans += scopes.At(j).Spans().Len()
726-
}
727-
}
710+
traceReader := &tracestoremocks.Reader{}
711+
dependencyStorage := &depstoremocks.Reader{}
712+
options := QueryServiceOptions{
713+
MaxTraceSize: tt.maxTraceSize,
714+
}
715+
tqs := &testQueryService{}
716+
tqs.queryService = NewQueryService(traceReader, dependencyStorage, options)
728717

729-
// Only 3 spans should be present(the limit)
730-
require.Equal(t, 3, totalSpans)
731-
732-
// there should be a warning for the first span
733-
firstSpan := resources.At(0).ScopeSpans().At(0).Spans().At(0)
734-
warningsAttr, hasWarning := firstSpan.Attributes().Get("@jaeger@warnings")
735-
require.True(t, hasWarning)
736-
require.Equal(t, pcommon.ValueTypeSlice, warningsAttr.Type())
737-
warnings := warningsAttr.Slice()
738-
require.Positive(t, warnings.Len())
739-
require.Contains(t, warnings.At(warnings.Len()-1).Str(), "trace has more than 3 spans")
740-
}
718+
params := GetTraceParams{
719+
TraceIDs: []tracestore.GetTraceParams{{TraceID: testTraceID}},
720+
}
721+
traceReader.On("GetTraces", mock.Anything, params.TraceIDs).
722+
Return(responseIter).Once()
741723

742-
func TestMaxTraceSize_ExactlyAtLimit(t *testing.T) {
743-
// 3 spans
744-
trace := ptrace.NewTraces()
745-
resources := trace.ResourceSpans().AppendEmpty()
746-
scopes := resources.ScopeSpans().AppendEmpty()
747-
for i := 0; i < 3; i++ {
748-
span := scopes.Spans().AppendEmpty()
749-
span.SetTraceID(testTraceID)
750-
span.SetSpanID(pcommon.SpanID([8]byte{byte(i + 1)}))
751-
span.SetName(fmt.Sprintf("span-%d", i))
752-
}
724+
getTracesIter := tqs.queryService.GetTraces(context.Background(), params)
725+
gotTraces, err := jiter.FlattenWithErrors(getTracesIter)
726+
require.NoError(t, err)
727+
require.Len(t, gotTraces, 1)
753728

754-
responseIter := iter.Seq2[[]ptrace.Traces, error](func(yield func([]ptrace.Traces, error) bool) {
755-
yield([]ptrace.Traces{trace}, nil)
756-
})
729+
// count total spans
730+
actualSpans := gotTraces[0].SpanCount()
731+
require.Equal(t, tt.expectedSpans, actualSpans)
757732

758-
traceReader := &tracestoremocks.Reader{}
759-
dependencyStorage := &depstoremocks.Reader{}
760-
options := QueryServiceOptions{
761-
MaxTraceSize: 3, // Limit is exactly 3, trace has 3 spans
762-
}
763-
tqs := &testQueryService{}
764-
tqs.queryService = NewQueryService(traceReader, dependencyStorage, options)
733+
// check warning
734+
firstSpan := gotTraces[0].ResourceSpans().At(0).ScopeSpans().At(0).Spans().At(0)
735+
warningsAttr, hasWarning := firstSpan.Attributes().Get("@jaeger@warnings")
765736

766-
params := GetTraceParams{
767-
TraceIDs: []tracestore.GetTraceParams{{TraceID: testTraceID}},
737+
if tt.expectWarning {
738+
require.True(t, hasWarning, "expected warning but none found")
739+
require.Equal(t, pcommon.ValueTypeSlice, warningsAttr.Type())
740+
warnings := warningsAttr.Slice()
741+
require.Positive(t, warnings.Len())
742+
require.Contains(t, warnings.At(warnings.Len()-1).Str(), tt.warningPattern)
743+
} else {
744+
require.False(t, hasWarning, "unexpected warning found")
745+
}
746+
})
768747
}
769-
traceReader.On("GetTraces", mock.Anything, params.TraceIDs).
770-
Return(responseIter).Once()
771-
772-
getTracesIter := tqs.queryService.GetTraces(context.Background(), params)
773-
gotTraces, err := jiter.FlattenWithErrors(getTracesIter)
774-
require.NoError(t, err)
775-
require.Len(t, gotTraces, 1)
776-
777-
gotSpans := gotTraces[0].ResourceSpans().At(0).ScopeSpans().At(0).Spans()
778-
// all 3 spans should be present
779-
require.Equal(t, 3, gotSpans.Len())
780-
781-
firstSpan := gotSpans.At(0)
782-
_, hasWarning := firstSpan.Attributes().Get("@jaeger@warnings")
783-
require.False(t, hasWarning)
784748
}

‎internal/jptrace/aggregator.go‎

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ func AggregateTracesWithLimit(tracesSeq iter.Seq2[[]ptrace.Traces, error], maxSi
4949
spanCount = trace.SpanCount()
5050
if maxSize > 0 && spanCount > maxSize {
5151
currentTrace = ptrace.NewTraces()
52-
copySpansUpToLimit(trace, currentTrace, maxSize)
52+
copySpansUpToLimit(currentTrace, trace, maxSize)
5353
spanCount = maxSize
5454
markTraceTruncated(currentTrace)
5555
}
@@ -84,32 +84,30 @@ func mergeTraces(src, dest ptrace.Traces, maxSize int, spanCount *int) bool {
8484
// partial copy
8585
remaining := maxSize - *spanCount
8686
if remaining > 0 {
87-
copySpansUpToLimit(src, dest, remaining)
87+
copySpansUpToLimit(dest, src, remaining)
8888
*spanCount = maxSize
8989
}
9090
return true
9191
}
9292

93-
func copySpansUpToLimit(src, dest ptrace.Traces, limit int) {
93+
func copySpansUpToLimit(dest, src ptrace.Traces, limit int) {
9494
copied := 0
95-
resources := src.ResourceSpans()
9695

97-
for i := 0; i < resources.Len() && copied < limit; i++ {
98-
srcResource := resources.At(i)
96+
for _, srcResource := range src.ResourceSpans().All() {
9997
destResource := dest.ResourceSpans().AppendEmpty()
10098
srcResource.Resource().CopyTo(destResource.Resource())
10199
destResource.SetSchemaUrl(srcResource.SchemaUrl())
102100

103-
scopes := srcResource.ScopeSpans()
104-
for j := 0; j < scopes.Len() && copied < limit; j++ {
105-
srcScope := scopes.At(j)
101+
for _, srcScope := range srcResource.ScopeSpans().All() {
106102
destScope := destResource.ScopeSpans().AppendEmpty()
107103
srcScope.Scope().CopyTo(destScope.Scope())
108104
destScope.SetSchemaUrl(srcScope.SchemaUrl())
109105

110-
spans := srcScope.Spans()
111-
for k := 0; k < spans.Len() && copied < limit; k++ {
112-
spans.At(k).CopyTo(destScope.Spans().AppendEmpty())
106+
for _, span := range srcScope.Spans().All() {
107+
if copied >= limit {
108+
return
109+
}
110+
span.CopyTo(destScope.Spans().AppendEmpty())
113111
copied++
114112
}
115113
}

‎internal/jptrace/aggregator_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -186,7 +186,7 @@ func TestCopySpansUpToLimit(t *testing.T) {
186186
}
187187

188188
dest := ptrace.NewTraces()
189-
copySpansUpToLimit(src, dest, 3)
189+
copySpansUpToLimit(dest, src, 3)
190190

191191
assert.Equal(t, 3, dest.SpanCount())
192192
}

0 commit comments

Comments
 (0)