Skip to content

Commit

Permalink
Change model.ToOTELxxxID() to accept ID argument (#6589)
Browse files Browse the repository at this point in the history
## Which problem is this PR solving?
- Part of #6494

## Description of the changes
- If we move TraceID/SpanID types to jaeger-idl we will not be able to
define methods on them, so change to plain functions.

## How was this change tested?
- CI

Signed-off-by: Yuri Shkuro <[email protected]>
  • Loading branch information
yurishkuro authored Jan 22, 2025
1 parent 08af186 commit cbad04b
Show file tree
Hide file tree
Showing 9 changed files with 23 additions and 23 deletions.
2 changes: 1 addition & 1 deletion cmd/query/app/apiv3/grpc_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ func (h *Handler) GetTrace(request *api_v3.GetTraceRequest, stream api_v3.QueryS
query := querysvc.GetTraceParams{
TraceIDs: []tracestore.GetTraceParams{
{
TraceID: traceID.ToOTELTraceID(),
TraceID: model.ToOTELTraceID(traceID),
Start: request.GetStartTime(),
End: request.GetEndTime(),
},
Expand Down
2 changes: 1 addition & 1 deletion cmd/query/app/apiv3/http_gateway.go
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ func (h *HTTPGateway) getTrace(w http.ResponseWriter, r *http.Request) {
request := querysvc.GetTraceParams{
TraceIDs: []tracestore.GetTraceParams{
{
TraceID: traceID.ToOTELTraceID(),
TraceID: model.ToOTELTraceID(traceID),
},
},
}
Expand Down
4 changes: 2 additions & 2 deletions model/otelids.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import (
// ToOTELTraceID converts the TraceID to OTEL's representation of a trace identitfier.
// This was taken from
// https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/main/internal/coreinternal/idutils/big_endian_converter.go.
func (t *TraceID) ToOTELTraceID() pcommon.TraceID {
func ToOTELTraceID(t TraceID) pcommon.TraceID {
traceID := [16]byte{}
binary.BigEndian.PutUint64(traceID[:8], t.High)
binary.BigEndian.PutUint64(traceID[8:], t.Low)
Expand All @@ -30,7 +30,7 @@ func TraceIDFromOTEL(traceID pcommon.TraceID) TraceID {
// ToOTELSpanID converts the SpanID to OTEL's representation of a span identitfier.
// This was taken from
// https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/main/internal/coreinternal/idutils/big_endian_converter.go.
func (s SpanID) ToOTELSpanID() pcommon.SpanID {
func ToOTELSpanID(s SpanID) pcommon.SpanID {
spanID := [8]byte{}
binary.BigEndian.PutUint64(spanID[:], uint64(s))
return pcommon.SpanID(spanID)
Expand Down
4 changes: 2 additions & 2 deletions model/otelids_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ func TestToOTELTraceID(t *testing.T) {
Low: 3,
High: 2,
}
otelTraceID := modelTraceID.ToOTELTraceID()
otelTraceID := model.ToOTELTraceID(modelTraceID)
expected := []byte{0, 0, 0, 0, 0, 0, 0, 2, 0, 0, 0, 0, 0, 0, 0, 3}
require.Equal(t, pcommon.TraceID(expected), otelTraceID)
}
Expand Down Expand Up @@ -53,7 +53,7 @@ func TestToOTELSpanID(t *testing.T) {

for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
actual := test.spanID.ToOTELSpanID()
actual := model.ToOTELSpanID(test.spanID)
assert.Equal(t, test.expected, actual)
})
}
Expand Down
12 changes: 6 additions & 6 deletions plugin/storage/integration/integration.go
Original file line number Diff line number Diff line change
Expand Up @@ -217,11 +217,11 @@ func (s *StorageIntegration) testGetLargeSpan(t *testing.T) {
t.Log("Testing Large Trace over 10K with duplicate IDs...")

expected := s.writeLargeTraceWithDuplicateSpanIds(t)
expectedTraceID := expected.Spans[0].TraceID
expectedTraceID := model.ToOTELTraceID(expected.Spans[0].TraceID)

actual := &model.Trace{} // no spans
found := s.waitForCondition(t, func(_ *testing.T) bool {
iterTraces := s.TraceReader.GetTraces(context.Background(), tracestore.GetTraceParams{TraceID: expectedTraceID.ToOTELTraceID()})
iterTraces := s.TraceReader.GetTraces(context.Background(), tracestore.GetTraceParams{TraceID: expectedTraceID})
traces, err := v1adapter.V1TracesFromSeq2(iterTraces)
if len(traces) > 0 {
actual = traces[0]
Expand Down Expand Up @@ -293,11 +293,11 @@ func (s *StorageIntegration) testGetTrace(t *testing.T) {
defer s.cleanUp(t)

expected := s.loadParseAndWriteExampleTrace(t)
expectedTraceID := expected.Spans[0].TraceID
expectedTraceID := model.ToOTELTraceID(expected.Spans[0].TraceID)

actual := &model.Trace{} // no spans
found := s.waitForCondition(t, func(t *testing.T) bool {
iterTraces := s.TraceReader.GetTraces(context.Background(), tracestore.GetTraceParams{TraceID: expectedTraceID.ToOTELTraceID()})
iterTraces := s.TraceReader.GetTraces(context.Background(), tracestore.GetTraceParams{TraceID: expectedTraceID})
traces, err := v1adapter.V1TracesFromSeq2(iterTraces)
if err != nil {
t.Log(err)
Expand All @@ -313,8 +313,8 @@ func (s *StorageIntegration) testGetTrace(t *testing.T) {
}

t.Run("NotFound error", func(t *testing.T) {
fakeTraceID := model.TraceID{High: 0, Low: 1}
iterTraces := s.TraceReader.GetTraces(context.Background(), tracestore.GetTraceParams{TraceID: fakeTraceID.ToOTELTraceID()})
fakeTraceID := model.ToOTELTraceID(model.TraceID{High: 0, Low: 1})
iterTraces := s.TraceReader.GetTraces(context.Background(), tracestore.GetTraceParams{TraceID: fakeTraceID})
traces, err := v1adapter.V1TracesFromSeq2(iterTraces)
require.NoError(t, err) // v2 TraceReader no longer returns an error for not found
assert.Empty(t, traces)
Expand Down
2 changes: 1 addition & 1 deletion storage_v2/v1adapter/spanreader.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ func NewSpanReader(traceReader tracestore.Reader) *SpanReader {

func (sr *SpanReader) GetTrace(ctx context.Context, query spanstore.GetTraceParameters) (*model.Trace, error) {
getTracesIter := sr.traceReader.GetTraces(ctx, tracestore.GetTraceParams{
TraceID: query.TraceID.ToOTELTraceID(),
TraceID: model.ToOTELTraceID(query.TraceID),
Start: query.StartTime,
End: query.EndTime,
})
Expand Down
2 changes: 1 addition & 1 deletion storage_v2/v1adapter/tracereader.go
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ func (tr *TraceReader) FindTraceIDs(
}
otelIDs := make([]pcommon.TraceID, 0, len(traceIDs))
for _, traceID := range traceIDs {
otelIDs = append(otelIDs, traceID.ToOTELTraceID())
otelIDs = append(otelIDs, model.ToOTELTraceID(traceID))
}
yield(otelIDs, nil)
}
Expand Down
2 changes: 1 addition & 1 deletion storage_v2/v1adapter/translator.go
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ func transferWarningsToOTLPSpans(batches []*model.Batch, spanMap map[pcommon.Spa
if len(span.Warnings) == 0 {
continue
}
if otelSpan, ok := spanMap[span.SpanID.ToOTELSpanID()]; ok {
if otelSpan, ok := spanMap[model.ToOTELSpanID(span.SpanID)]; ok {
jptrace.AddWarnings(otelSpan, span.Warnings...)
}
}
Expand Down
16 changes: 8 additions & 8 deletions storage_v2/v1adapter/translator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -146,9 +146,9 @@ func TestV1TracesFromSeq2(t *testing.T) {
// Add a new span and set attributes
modelTraceID := model.NewTraceID(2, 3)
span1 := spans.AppendEmpty()
span1.SetTraceID(modelTraceID.ToOTELTraceID())
span1.SetTraceID(model.ToOTELTraceID(modelTraceID))
span1.SetName("op-success-a")
span1.SetSpanID(model.NewSpanID(1).ToOTELSpanID())
span1.SetSpanID(model.ToOTELSpanID(model.NewSpanID(1)))

// Yield the test trace
yield([]ptrace.Traces{testTrace}, nil)
Expand Down Expand Up @@ -183,18 +183,18 @@ func TestV1TracesFromSeq2(t *testing.T) {
spans1 := sSpans1.Spans()
modelTraceID := model.NewTraceID(2, 3)
span1 := spans1.AppendEmpty()
span1.SetTraceID(modelTraceID.ToOTELTraceID())
span1.SetTraceID(model.ToOTELTraceID(modelTraceID))
span1.SetName("op-two-chunks-a")
span1.SetSpanID(model.NewSpanID(1).ToOTELSpanID())
span1.SetSpanID(model.ToOTELSpanID(model.NewSpanID(1)))

traceChunk2 := ptrace.NewTraces()
rSpans2 := traceChunk2.ResourceSpans().AppendEmpty()
sSpans2 := rSpans2.ScopeSpans().AppendEmpty()
spans2 := sSpans2.Spans()
span2 := spans2.AppendEmpty()
span2.SetTraceID(modelTraceID.ToOTELTraceID())
span2.SetTraceID(model.ToOTELTraceID(modelTraceID))
span2.SetName("op-two-chunks-b")
span2.SetSpanID(model.NewSpanID(2).ToOTELSpanID())
span2.SetSpanID(model.ToOTELSpanID(model.NewSpanID(2)))
// Yield the test trace
yield([]ptrace.Traces{traceChunk1, traceChunk2}, nil)
},
Expand All @@ -218,9 +218,9 @@ func TestV1TracesFromSeq2(t *testing.T) {

modelTraceID := model.NewTraceID(2, 3)
span1 := spans.AppendEmpty()
span1.SetTraceID(modelTraceID.ToOTELTraceID())
span1.SetTraceID(model.ToOTELTraceID(modelTraceID))
span1.SetName("op-error-a")
span1.SetSpanID(model.NewSpanID(1).ToOTELSpanID())
span1.SetSpanID(model.ToOTELSpanID(model.NewSpanID(1)))

// Yield the test trace
if !yield([]ptrace.Traces{testTrace}, nil) {
Expand Down

0 comments on commit cbad04b

Please sign in to comment.