Skip to content

Commit

Permalink
[refactor] Return start and end timestamps from FindTraceIDs in v2 …
Browse files Browse the repository at this point in the history
…api (#6770)

## Which problem is this PR solving?
- Towards #6765

## Description of the changes
- This PR changes the API of `FindTraceIDs` from returning
`[]pcommon.TraceID` to returning a `FindTraceIDsChunk` that contains the
trace IDs along with `Start` and `End` fields that are meant to serve as
hints for some storage backends (e.g Tempo) that can optimize the
queries when given a time range.

## How was this change tested?
- CI

## Checklist
- [x] I have read
https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md
- [x] I have signed all commits
- [x] I have added unit tests for the new functionality
- [x] I have run lint and test steps successfully
  - for `jaeger`: `make lint test`
  - for `jaeger-ui`: `npm run lint` and `npm run test`

---------

Signed-off-by: Mahad Zaryab <[email protected]>
  • Loading branch information
mahadzaryab1 authored Feb 24, 2025
1 parent f956540 commit b207889
Show file tree
Hide file tree
Showing 8 changed files with 63 additions and 37 deletions.
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

0 comments on commit b207889

Please sign in to comment.