Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[refactor] Return start and end timestamps from FindTraceIDs in v2 api #6770

Merged
merged 10 commits into from
Feb 24, 2025
3 changes: 1 addition & 2 deletions cmd/jaeger/internal/integration/trace_reader.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import (
"math"
"strings"

"go.opentelemetry.io/collector/pdata/pcommon"
"go.opentelemetry.io/collector/pdata/ptrace"
"go.uber.org/zap"
"google.golang.org/grpc"
Expand Down Expand Up @@ -138,7 +137,7 @@ func (r *traceReader) FindTraces(
func (*traceReader) FindTraceIDs(
_ context.Context,
_ tracestore.TraceQueryParams,
) iter.Seq2[[]pcommon.TraceID, error] {
) iter.Seq2[[]tracestore.FoundTraceID, error] {
panic("not implemented")
}

Expand Down
10 changes: 4 additions & 6 deletions internal/storage/v2/api/tracestore/mocks/Reader.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

15 changes: 14 additions & 1 deletion internal/storage/v2/api/tracestore/reader.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ type Reader interface {
// of matching trace IDs. This is useful in some contexts, such as batch jobs, where a
// large list of trace IDs may be queried first and then the full traces are loaded
// in batches.
FindTraceIDs(ctx context.Context, query TraceQueryParams) iter.Seq2[[]pcommon.TraceID, error]
FindTraceIDs(ctx context.Context, query TraceQueryParams) iter.Seq2[[]FoundTraceID, error]
}

// GetTraceParams contains single-trace parameters for a GetTraces request.
Expand All @@ -88,6 +88,19 @@ type TraceQueryParams struct {
NumTraces int
}

// FoundTraceID is a wrapper around trace ID returned from FindTraceIDs
// with an optional time range that may be used in GetTraces calls.
//
// The time range is provided as an optimization hint for some storage backends
// that can perform more efficient queries when they know the approximate time range.
// The value should not be used for precise time-based filtering or assumptions.
// It is meant as a rough boundary and may not be populated in all cases.
type FoundTraceID struct {
TraceID pcommon.TraceID
Start time.Time
End time.Time
}

func (t *TraceQueryParams) ToSpanStoreQueryParameters() *spanstore.TraceQueryParameters {
return &spanstore.TraceQueryParameters{
ServiceName: t.ServiceName,
Expand Down
16 changes: 10 additions & 6 deletions internal/storage/v2/v1adapter/spanreader_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -336,7 +336,7 @@ func TestSpanReader_FindTraceIDs(t *testing.T) {
name string
query *spanstore.TraceQueryParameters
expectedQuery tracestore.TraceQueryParams
traceIDs []pcommon.TraceID
traceIDs []tracestore.FoundTraceID
expectedTraceIDs []model.TraceID
err error
expectedErr error
Expand All @@ -360,7 +360,7 @@ func TestSpanReader_FindTraceIDs(t *testing.T) {
expectedQuery: tracestore.TraceQueryParams{
ServiceName: "service1",
},
traceIDs: []pcommon.TraceID{},
traceIDs: []tracestore.FoundTraceID{},
expectedTraceIDs: nil,
},
{
Expand All @@ -371,9 +371,13 @@ func TestSpanReader_FindTraceIDs(t *testing.T) {
expectedQuery: tracestore.TraceQueryParams{
ServiceName: "service1",
},
traceIDs: []pcommon.TraceID{
pcommon.TraceID([16]byte{0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 2}),
pcommon.TraceID([16]byte{0, 0, 0, 0, 0, 0, 0, 3, 0, 0, 0, 0, 0, 0, 0, 4}),
traceIDs: []tracestore.FoundTraceID{
{
TraceID: [16]byte{0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 2},
},
{
TraceID: [16]byte{0, 0, 0, 0, 0, 0, 0, 3, 0, 0, 0, 0, 0, 0, 0, 4},
},
},
expectedTraceIDs: []model.TraceID{
model.NewTraceID(1, 2),
Expand All @@ -385,7 +389,7 @@ func TestSpanReader_FindTraceIDs(t *testing.T) {
for _, test := range tests {
tr := tracestoremocks.Reader{}
tr.On("FindTraceIDs", mock.Anything, test.expectedQuery).
Return(iter.Seq2[[]pcommon.TraceID, error](func(yield func([]pcommon.TraceID, error) bool) {
Return(iter.Seq2[[]tracestore.FoundTraceID, error](func(yield func([]tracestore.FoundTraceID, error) bool) {
yield(test.traceIDs, test.err)
})).Once()

Expand Down
11 changes: 6 additions & 5 deletions internal/storage/v2/v1adapter/tracereader.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import (
"errors"
"iter"

"go.opentelemetry.io/collector/pdata/pcommon"
"go.opentelemetry.io/collector/pdata/ptrace"

"github.com/jaegertracing/jaeger-idl/model/v1"
Expand Down Expand Up @@ -113,16 +112,18 @@ func (tr *TraceReader) FindTraces(
func (tr *TraceReader) FindTraceIDs(
ctx context.Context,
query tracestore.TraceQueryParams,
) iter.Seq2[[]pcommon.TraceID, error] {
return func(yield func([]pcommon.TraceID, error) bool) {
) iter.Seq2[[]tracestore.FoundTraceID, error] {
return func(yield func([]tracestore.FoundTraceID, error) bool) {
traceIDs, err := tr.spanReader.FindTraceIDs(ctx, query.ToSpanStoreQueryParameters())
if err != nil {
yield(nil, err)
return
}
otelIDs := make([]pcommon.TraceID, 0, len(traceIDs))
otelIDs := make([]tracestore.FoundTraceID, 0, len(traceIDs))
for _, traceID := range traceIDs {
otelIDs = append(otelIDs, FromV1TraceID(traceID))
otelIDs = append(otelIDs, tracestore.FoundTraceID{
TraceID: FromV1TraceID(traceID),
})
}
yield(otelIDs, nil)
}
Expand Down
12 changes: 8 additions & 4 deletions internal/storage/v2/v1adapter/tracereader_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -404,7 +404,7 @@ func TestTraceReader_FindTraceIDsDelegatesResponse(t *testing.T) {
tests := []struct {
name string
modelTraceIDs []model.TraceID
expectedTraceIDs []pcommon.TraceID
expectedTraceIDs []tracestore.FoundTraceID
err error
}{
{
Expand All @@ -413,9 +413,13 @@ func TestTraceReader_FindTraceIDsDelegatesResponse(t *testing.T) {
{Low: 3, High: 2},
{Low: 4, High: 3},
},
expectedTraceIDs: []pcommon.TraceID{
pcommon.TraceID([]byte{0, 0, 0, 0, 0, 0, 0, 2, 0, 0, 0, 0, 0, 0, 0, 3}),
pcommon.TraceID([]byte{0, 0, 0, 0, 0, 0, 0, 3, 0, 0, 0, 0, 0, 0, 0, 4}),
expectedTraceIDs: []tracestore.FoundTraceID{
{
TraceID: pcommon.TraceID([]byte{0, 0, 0, 0, 0, 0, 0, 2, 0, 0, 0, 0, 0, 0, 0, 3}),
},
{
TraceID: pcommon.TraceID([]byte{0, 0, 0, 0, 0, 0, 0, 3, 0, 0, 0, 0, 0, 0, 0, 4}),
},
},
},
{
Expand Down
7 changes: 4 additions & 3 deletions internal/storage/v2/v1adapter/translator.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (

"github.com/jaegertracing/jaeger-idl/model/v1"
"github.com/jaegertracing/jaeger/internal/jptrace"
"github.com/jaegertracing/jaeger/internal/storage/v2/api/tracestore"
)

// V1BatchesFromTraces converts OpenTelemetry traces (ptrace.Traces)
Expand Down Expand Up @@ -62,18 +63,18 @@ func V1TracesFromSeq2(otelSeq iter.Seq2[[]ptrace.Traces, error]) ([]*model.Trace
return jaegerTraces, nil
}

func V1TraceIDsFromSeq2(traceIDsIter iter.Seq2[[]pcommon.TraceID, error]) ([]model.TraceID, error) {
func V1TraceIDsFromSeq2(traceIDsIter iter.Seq2[[]tracestore.FoundTraceID, error]) ([]model.TraceID, error) {
var (
iterErr error
modelTraceIDs []model.TraceID
)
traceIDsIter(func(traceIDs []pcommon.TraceID, err error) bool {
traceIDsIter(func(traceIDs []tracestore.FoundTraceID, err error) bool {
if err != nil {
iterErr = err
return false
}
for _, traceID := range traceIDs {
modelTraceIDs = append(modelTraceIDs, ToV1TraceID(traceID))
modelTraceIDs = append(modelTraceIDs, ToV1TraceID(traceID.TraceID))
}
return true
})
Expand Down
26 changes: 16 additions & 10 deletions internal/storage/v2/v1adapter/translator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import (

"github.com/jaegertracing/jaeger-idl/model/v1"
"github.com/jaegertracing/jaeger/internal/jptrace"
"github.com/jaegertracing/jaeger/internal/storage/v2/api/tracestore"
)

func TestProtoFromTraces_AddsWarnings(t *testing.T) {
Expand Down Expand Up @@ -287,30 +288,35 @@ func TestV1TraceToOtelTrace_ReturnEmptyOtelTrace(t *testing.T) {
func TestV1TraceIDsFromSeq2(t *testing.T) {
testCases := []struct {
name string
seqTraceIDs iter.Seq2[[]pcommon.TraceID, error]
seqTraceIDs iter.Seq2[[]tracestore.FoundTraceID, error]
expectedIDs []model.TraceID
expectedError error
}{
{
name: "empty sequence",
seqTraceIDs: func(func([]pcommon.TraceID, error) bool) {},
seqTraceIDs: func(func([]tracestore.FoundTraceID, error) bool) {},
expectedIDs: nil,
expectedError: nil,
},
{
name: "sequence with error",
seqTraceIDs: func(yield func([]pcommon.TraceID, error) bool) {
seqTraceIDs: func(yield func([]tracestore.FoundTraceID, error) bool) {
yield(nil, assert.AnError)
},
expectedIDs: nil,
expectedError: assert.AnError,
},
{
name: "sequence with one chunk of trace IDs",
seqTraceIDs: func(yield func([]pcommon.TraceID, error) bool) {
traceID1 := pcommon.TraceID([16]byte{0, 0, 0, 0, 0, 0, 0, 2, 0, 0, 0, 0, 0, 0, 0, 3})
traceID2 := pcommon.TraceID([16]byte{0, 0, 0, 0, 0, 0, 0, 4, 0, 0, 0, 0, 0, 0, 0, 5})
yield([]pcommon.TraceID{traceID1, traceID2}, nil)
seqTraceIDs: func(yield func([]tracestore.FoundTraceID, error) bool) {
yield([]tracestore.FoundTraceID{
{
TraceID: pcommon.TraceID([16]byte{0, 0, 0, 0, 0, 0, 0, 2, 0, 0, 0, 0, 0, 0, 0, 3}),
},
{
TraceID: pcommon.TraceID([16]byte{0, 0, 0, 0, 0, 0, 0, 4, 0, 0, 0, 0, 0, 0, 0, 5}),
},
}, nil)
},
expectedIDs: []model.TraceID{
model.NewTraceID(2, 3),
Expand All @@ -320,12 +326,12 @@ func TestV1TraceIDsFromSeq2(t *testing.T) {
},
{
name: "sequence with multiple chunks of trace IDs",
seqTraceIDs: func(yield func([]pcommon.TraceID, error) bool) {
seqTraceIDs: func(yield func([]tracestore.FoundTraceID, error) bool) {
traceID1 := pcommon.TraceID([16]byte{0, 0, 0, 0, 0, 0, 0, 2, 0, 0, 0, 0, 0, 0, 0, 3})
traceID2 := pcommon.TraceID([16]byte{0, 0, 0, 0, 0, 0, 0, 4, 0, 0, 0, 0, 0, 0, 0, 5})
traceID3 := pcommon.TraceID([16]byte{0, 0, 0, 0, 0, 0, 0, 6, 0, 0, 0, 0, 0, 0, 0, 7})
yield([]pcommon.TraceID{traceID1}, nil)
yield([]pcommon.TraceID{traceID2, traceID3}, nil)
yield([]tracestore.FoundTraceID{{TraceID: traceID1}}, nil)
yield([]tracestore.FoundTraceID{{TraceID: traceID2}, {TraceID: traceID3}}, nil)
},
expectedIDs: []model.TraceID{
model.NewTraceID(2, 3),
Expand Down
Loading