From b02c55943bdd5bfde5f5e9e1a0ae1db2424d6f2b Mon Sep 17 00:00:00 2001 From: Mahad Zaryab <43658574+mahadzaryab1@users.noreply.github.com> Date: Wed, 25 Dec 2024 17:45:40 -0500 Subject: [PATCH] [refactor] Move model<->otlp translation from `jptrace` to `v1adapter` (#6414) ## Description of the changes - Move model<->otlp translation from `jptrace` to `v1adapter` to keep `jptrace` specific to helper around the `ptrace` domain objects. ## 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 --- .golangci.yml | 4 ++-- cmd/collector/app/handler/otlp_receiver.go | 4 ++-- .../internal/integration/span_writer.go | 4 ++-- .../processors/adaptivesampling/processor.go | 4 ++-- cmd/query/app/apiv3/otlp_translator.go | 4 ++-- cmd/query/app/otlp_translator.go | 4 ++-- internal/jptrace/warning.go | 8 ++++---- storage_v2/v1adapter/reader.go | 5 ++--- .../v1adapter}/translator.go | 15 ++++++++------- .../v1adapter}/translator_test.go | 19 ++++++++++--------- storage_v2/v1adapter/writer.go | 3 +-- 11 files changed, 37 insertions(+), 37 deletions(-) rename {internal/jptrace => storage_v2/v1adapter}/translator.go (83%) rename {internal/jptrace => storage_v2/v1adapter}/translator_test.go (85%) diff --git a/.golangci.yml b/.golangci.yml index 97170f1338a..bac7db011bd 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -142,9 +142,9 @@ linters-settings: disallow-otel-contrib-translator: deny: - pkg: github.com/open-telemetry/opentelemetry-collector-contrib/pkg/translator/jaeger - desc: "Use jptrace package instead of opentelemetry-collector-contrib/pkg/translator/jaeger" + desc: "Use v1adapter package instead of opentelemetry-collector-contrib/pkg/translator/jaeger" files: - - "!**/jptrace/**" + - "!**/v1adapter/**" # TODO: remove once we have upgraded to Go 1.23 disallow-iter: diff --git a/cmd/collector/app/handler/otlp_receiver.go b/cmd/collector/app/handler/otlp_receiver.go index ebde8d2ba38..228ad80b666 100644 --- a/cmd/collector/app/handler/otlp_receiver.go +++ b/cmd/collector/app/handler/otlp_receiver.go @@ -22,8 +22,8 @@ import ( "github.com/jaegertracing/jaeger/cmd/collector/app/flags" "github.com/jaegertracing/jaeger/cmd/collector/app/processor" - "github.com/jaegertracing/jaeger/internal/jptrace" "github.com/jaegertracing/jaeger/pkg/tenancy" + "github.com/jaegertracing/jaeger/storage_v2/v1adapter" ) var _ component.Host = (*otelHost)(nil) // API check @@ -108,7 +108,7 @@ type consumerDelegate struct { } func (c *consumerDelegate) consume(ctx context.Context, td ptrace.Traces) error { - batches := jptrace.ProtoFromTraces(td) + batches := v1adapter.ProtoFromTraces(td) for _, batch := range batches { err := c.batchConsumer.consume(ctx, batch) if err != nil { diff --git a/cmd/jaeger/internal/integration/span_writer.go b/cmd/jaeger/internal/integration/span_writer.go index 2f718fb5022..073eaec65ee 100644 --- a/cmd/jaeger/internal/integration/span_writer.go +++ b/cmd/jaeger/internal/integration/span_writer.go @@ -16,9 +16,9 @@ import ( "go.opentelemetry.io/collector/exporter/otlpexporter" "go.uber.org/zap" - "github.com/jaegertracing/jaeger/internal/jptrace" "github.com/jaegertracing/jaeger/model" "github.com/jaegertracing/jaeger/storage/spanstore" + "github.com/jaegertracing/jaeger/storage_v2/v1adapter" ) var ( @@ -68,7 +68,7 @@ func (w *spanWriter) Close() error { } func (w *spanWriter) WriteSpan(ctx context.Context, span *model.Span) error { - td := jptrace.ProtoToTraces([]*model.Batch{ + td := v1adapter.V1BatchesToTraces([]*model.Batch{ { Spans: []*model.Span{span}, Process: span.Process, diff --git a/cmd/jaeger/internal/processors/adaptivesampling/processor.go b/cmd/jaeger/internal/processors/adaptivesampling/processor.go index 7a0ca2a0641..6a42151b1da 100644 --- a/cmd/jaeger/internal/processors/adaptivesampling/processor.go +++ b/cmd/jaeger/internal/processors/adaptivesampling/processor.go @@ -12,9 +12,9 @@ import ( "github.com/jaegertracing/jaeger/cmd/collector/app/sampling/samplingstrategy" "github.com/jaegertracing/jaeger/cmd/jaeger/internal/extension/remotesampling" - "github.com/jaegertracing/jaeger/internal/jptrace" "github.com/jaegertracing/jaeger/internal/metrics/otelmetrics" "github.com/jaegertracing/jaeger/plugin/sampling/strategyprovider/adaptive" + "github.com/jaegertracing/jaeger/storage_v2/v1adapter" ) type traceProcessor struct { @@ -65,7 +65,7 @@ func (tp *traceProcessor) close(context.Context) error { } func (tp *traceProcessor) processTraces(_ context.Context, td ptrace.Traces) (ptrace.Traces, error) { - batches := jptrace.ProtoFromTraces(td) + batches := v1adapter.ProtoFromTraces(td) for _, batch := range batches { for _, span := range batch.Spans { if span.Process == nil { diff --git a/cmd/query/app/apiv3/otlp_translator.go b/cmd/query/app/apiv3/otlp_translator.go index e9767720903..1d1bba2f4cc 100644 --- a/cmd/query/app/apiv3/otlp_translator.go +++ b/cmd/query/app/apiv3/otlp_translator.go @@ -6,12 +6,12 @@ package apiv3 import ( "go.opentelemetry.io/collector/pdata/ptrace" - "github.com/jaegertracing/jaeger/internal/jptrace" "github.com/jaegertracing/jaeger/model" + "github.com/jaegertracing/jaeger/storage_v2/v1adapter" ) func modelToOTLP(spans []*model.Span) ptrace.Traces { batch := &model.Batch{Spans: spans} - tr := jptrace.ProtoToTraces([]*model.Batch{batch}) + tr := v1adapter.V1BatchesToTraces([]*model.Batch{batch}) return tr } diff --git a/cmd/query/app/otlp_translator.go b/cmd/query/app/otlp_translator.go index daaaabf2aeb..82ed887d322 100644 --- a/cmd/query/app/otlp_translator.go +++ b/cmd/query/app/otlp_translator.go @@ -8,8 +8,8 @@ import ( "go.opentelemetry.io/collector/pdata/ptrace" - "github.com/jaegertracing/jaeger/internal/jptrace" "github.com/jaegertracing/jaeger/model" + "github.com/jaegertracing/jaeger/storage_v2/v1adapter" ) func otlp2traces(otlpSpans []byte) ([]*model.Trace, error) { @@ -18,7 +18,7 @@ func otlp2traces(otlpSpans []byte) ([]*model.Trace, error) { if err != nil { return nil, fmt.Errorf("cannot unmarshal OTLP : %w", err) } - jaegerBatches := jptrace.ProtoFromTraces(otlpTraces) + jaegerBatches := v1adapter.ProtoFromTraces(otlpTraces) var traces []*model.Trace traceMap := make(map[model.TraceID]*model.Trace) for _, batch := range jaegerBatches { diff --git a/internal/jptrace/warning.go b/internal/jptrace/warning.go index 5247930fc25..e8c7c69311f 100644 --- a/internal/jptrace/warning.go +++ b/internal/jptrace/warning.go @@ -13,15 +13,15 @@ const ( // store various warnings produced from transformations, // such as inbound sanitizers and outbound adjusters. // The value type of the attribute is a string slice. - warningsAttribute = "jaeger.internal.warnings" + WarningsAttribute = "jaeger.internal.warnings" ) func AddWarnings(span ptrace.Span, warnings ...string) { var w pcommon.Slice - if currWarnings, ok := span.Attributes().Get(warningsAttribute); ok { + if currWarnings, ok := span.Attributes().Get(WarningsAttribute); ok { w = currWarnings.Slice() } else { - w = span.Attributes().PutEmptySlice(warningsAttribute) + w = span.Attributes().PutEmptySlice(WarningsAttribute) } for _, warning := range warnings { w.AppendEmpty().SetStr(warning) @@ -29,7 +29,7 @@ func AddWarnings(span ptrace.Span, warnings ...string) { } func GetWarnings(span ptrace.Span) []string { - if w, ok := span.Attributes().Get(warningsAttribute); ok { + if w, ok := span.Attributes().Get(WarningsAttribute); ok { warnings := []string{} ws := w.Slice() for i := 0; i < ws.Len(); i++ { diff --git a/storage_v2/v1adapter/reader.go b/storage_v2/v1adapter/reader.go index 04508a1247c..225e9267d89 100644 --- a/storage_v2/v1adapter/reader.go +++ b/storage_v2/v1adapter/reader.go @@ -10,7 +10,6 @@ import ( "go.opentelemetry.io/collector/pdata/pcommon" "go.opentelemetry.io/collector/pdata/ptrace" - "github.com/jaegertracing/jaeger/internal/jptrace" "github.com/jaegertracing/jaeger/model" "github.com/jaegertracing/jaeger/pkg/iter" "github.com/jaegertracing/jaeger/storage/dependencystore" @@ -60,7 +59,7 @@ func (tr *TraceReader) GetTraces( return } batch := &model.Batch{Spans: t.GetSpans()} - tr := jptrace.ProtoToTraces([]*model.Batch{batch}) + tr := V1BatchesToTraces([]*model.Batch{batch}) if !yield([]ptrace.Traces{tr}, nil) { return } @@ -105,7 +104,7 @@ func (tr *TraceReader) FindTraces( } for _, trace := range traces { batch := &model.Batch{Spans: trace.GetSpans()} - otelTrace := jptrace.ProtoToTraces([]*model.Batch{batch}) + otelTrace := V1BatchesToTraces([]*model.Batch{batch}) if !yield([]ptrace.Traces{otelTrace}, nil) { return } diff --git a/internal/jptrace/translator.go b/storage_v2/v1adapter/translator.go similarity index 83% rename from internal/jptrace/translator.go rename to storage_v2/v1adapter/translator.go index a7249135812..dda96a5630a 100644 --- a/internal/jptrace/translator.go +++ b/storage_v2/v1adapter/translator.go @@ -1,13 +1,14 @@ // Copyright (c) 2024 The Jaeger Authors. // SPDX-License-Identifier: Apache-2.0 -package jptrace +package v1adapter import ( jaegerTranslator "github.com/open-telemetry/opentelemetry-collector-contrib/pkg/translator/jaeger" "go.opentelemetry.io/collector/pdata/pcommon" "go.opentelemetry.io/collector/pdata/ptrace" + "github.com/jaegertracing/jaeger/internal/jptrace" "github.com/jaegertracing/jaeger/model" ) @@ -20,11 +21,11 @@ func ProtoFromTraces(traces ptrace.Traces) []*model.Batch { return batches } -// ProtoToTraces converts Jaeger model batches ([]*model.Batch) +// V1BatchesToTraces converts Jaeger model batches ([]*model.Batch) // to OpenTelemetry traces (ptrace.Traces). -func ProtoToTraces(batches []*model.Batch) ptrace.Traces { +func V1BatchesToTraces(batches []*model.Batch) ptrace.Traces { traces, _ := jaegerTranslator.ProtoToTraces(batches) // never returns an error - spanMap := SpanMap(traces, func(s ptrace.Span) pcommon.SpanID { + spanMap := jptrace.SpanMap(traces, func(s ptrace.Span) pcommon.SpanID { return s.SpanID() }) transferWarningsToOTLPSpans(batches, spanMap) @@ -49,14 +50,14 @@ func transferWarningsToModelSpans(traces ptrace.Traces, spanMap map[model.SpanID spans := scopes.At(j).Spans() for k := 0; k < spans.Len(); k++ { otelSpan := spans.At(k) - warnings := GetWarnings(otelSpan) + warnings := jptrace.GetWarnings(otelSpan) if len(warnings) == 0 { continue } if span, ok := spanMap[model.SpanIDFromOTEL(otelSpan.SpanID())]; ok { span.Warnings = append(span.Warnings, warnings...) // filter out the warning tag - span.Tags = filterTags(span.Tags, warningsAttribute) + span.Tags = filterTags(span.Tags, jptrace.WarningsAttribute) } } } @@ -70,7 +71,7 @@ func transferWarningsToOTLPSpans(batches []*model.Batch, spanMap map[pcommon.Spa continue } if otelSpan, ok := spanMap[span.SpanID.ToOTELSpanID()]; ok { - AddWarnings(otelSpan, span.Warnings...) + jptrace.AddWarnings(otelSpan, span.Warnings...) } } } diff --git a/internal/jptrace/translator_test.go b/storage_v2/v1adapter/translator_test.go similarity index 85% rename from internal/jptrace/translator_test.go rename to storage_v2/v1adapter/translator_test.go index 70d7e70079c..e78a71f423f 100644 --- a/internal/jptrace/translator_test.go +++ b/storage_v2/v1adapter/translator_test.go @@ -1,7 +1,7 @@ // Copyright (c) 2024 The Jaeger Authors. // SPDX-License-Identifier: Apache-2.0 -package jptrace +package v1adapter import ( "testing" @@ -10,6 +10,7 @@ import ( "go.opentelemetry.io/collector/pdata/pcommon" "go.opentelemetry.io/collector/pdata/ptrace" + "github.com/jaegertracing/jaeger/internal/jptrace" "github.com/jaegertracing/jaeger/model" ) @@ -20,8 +21,8 @@ func TestProtoFromTraces_AddsWarnings(t *testing.T) { span1 := ss1.Spans().AppendEmpty() span1.SetName("test-span-1") span1.SetSpanID(pcommon.SpanID([8]byte{1, 2, 3, 4, 5, 6, 7, 8})) - AddWarnings(span1, "test-warning-1") - AddWarnings(span1, "test-warning-2") + jptrace.AddWarnings(span1, "test-warning-1") + jptrace.AddWarnings(span1, "test-warning-2") span1.Attributes().PutStr("key", "value") ss2 := rs1.ScopeSpans().AppendEmpty() @@ -34,7 +35,7 @@ func TestProtoFromTraces_AddsWarnings(t *testing.T) { span3 := ss3.Spans().AppendEmpty() span3.SetName("test-span-3") span3.SetSpanID(pcommon.SpanID([8]byte{17, 18, 19, 20, 21, 22, 23, 24})) - AddWarnings(span3, "test-warning-3") + jptrace.AddWarnings(span3, "test-warning-3") batches := ProtoFromTraces(traces) @@ -84,23 +85,23 @@ func TestProtoToTraces_AddsWarnings(t *testing.T) { }, } batches := []*model.Batch{batch1, batch2} - traces := ProtoToTraces(batches) + traces := V1BatchesToTraces(batches) assert.Equal(t, 2, traces.ResourceSpans().Len()) - spanMap := SpanMap(traces, func(s ptrace.Span) string { + spanMap := jptrace.SpanMap(traces, func(s ptrace.Span) string { return s.Name() }) span1 := spanMap["test-span-1"] assert.Equal(t, pcommon.SpanID([8]byte{0, 0, 0, 0, 0, 0, 0, 1}), span1.SpanID()) - assert.Equal(t, []string{"test-warning-1", "test-warning-2"}, GetWarnings(span1)) + assert.Equal(t, []string{"test-warning-1", "test-warning-2"}, jptrace.GetWarnings(span1)) span2 := spanMap["test-span-2"] assert.Equal(t, pcommon.SpanID([8]byte{0, 0, 0, 0, 0, 0, 0, 2}), span2.SpanID()) - assert.Empty(t, GetWarnings(span2)) + assert.Empty(t, jptrace.GetWarnings(span2)) span3 := spanMap["test-span-3"] assert.Equal(t, pcommon.SpanID([8]byte{0, 0, 0, 0, 0, 0, 0, 3}), span3.SpanID()) - assert.Equal(t, []string{"test-warning-3"}, GetWarnings(span3)) + assert.Equal(t, []string{"test-warning-3"}, jptrace.GetWarnings(span3)) } diff --git a/storage_v2/v1adapter/writer.go b/storage_v2/v1adapter/writer.go index b16b54ed43e..5602d1ae8f7 100644 --- a/storage_v2/v1adapter/writer.go +++ b/storage_v2/v1adapter/writer.go @@ -9,7 +9,6 @@ import ( "go.opentelemetry.io/collector/pdata/ptrace" - "github.com/jaegertracing/jaeger/internal/jptrace" "github.com/jaegertracing/jaeger/storage/spanstore" "github.com/jaegertracing/jaeger/storage_v2/tracestore" ) @@ -26,7 +25,7 @@ func NewTraceWriter(spanWriter spanstore.Writer) tracestore.Writer { // WriteTraces implements tracestore.Writer. func (t *TraceWriter) WriteTraces(ctx context.Context, td ptrace.Traces) error { - batches := jptrace.ProtoFromTraces(td) + batches := ProtoFromTraces(td) var errs []error for _, batch := range batches { for _, span := range batch.Spans {