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.

Sign up for GitHub

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] Move model<->otlp translation from jptrace to v1adapter #6414

Merged
merged 3 commits into from
Dec 25, 2024
Merged

[refactor] Move model<->otlp translation from jptrace to v1adapter #6414

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
52939c0
Move Translator From jptrace To v1adapter
mahadzaryab1 Dec 25, 2024
7e24629
Fix Linter
mahadzaryab1 Dec 25, 2024
bba15ea
Rename Function
mahadzaryab1 Dec 25, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .golangci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
4 changes: 2 additions & 2 deletions cmd/collector/app/handler/otlp_receiver.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand Down
4 changes: 2 additions & 2 deletions cmd/jaeger/internal/integration/span_writer.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down Expand Up @@ -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.ProtoToTraces([]*model.Batch{
{
Spans: []*model.Span{span},
Process: span.Process,
Expand Down
4 changes: 2 additions & 2 deletions cmd/jaeger/internal/processors/adaptivesampling/processor.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down
4 changes: 2 additions & 2 deletions cmd/query/app/apiv3/otlp_translator.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.ProtoToTraces([]*model.Batch{batch})
return tr
}
4 changes: 2 additions & 2 deletions cmd/query/app/otlp_translator.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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 {
Expand Down
8 changes: 4 additions & 4 deletions internal/jptrace/warning.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,23 +13,23 @@ 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)
}
}

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++ {
Expand Down
5 changes: 2 additions & 3 deletions storage_v2/v1adapter/reader.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -60,7 +59,7 @@ func (tr *TraceReader) GetTraces(
return
}
batch := &model.Batch{Spans: t.GetSpans()}
tr := jptrace.ProtoToTraces([]*model.Batch{batch})
tr := ProtoToTraces([]*model.Batch{batch})
if !yield([]ptrace.Traces{tr}, nil) {
return
}
Expand Down Expand Up @@ -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 := ProtoToTraces([]*model.Batch{batch})
if !yield([]ptrace.Traces{otelTrace}, nil) {
return
}
Expand Down
11 changes: 6 additions & 5 deletions internal/jptrace/translator.go → storage_v2/v1adapter/translator.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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"
)

Expand All @@ -24,7 +25,7 @@ func ProtoFromTraces(traces ptrace.Traces) []*model.Batch {
// to OpenTelemetry traces (ptrace.Traces).
func ProtoToTraces(batches []*model.Batch) ptrace.Traces {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we change the name of the function? E.g. ModelBatchesToTraces. Because in some cases it's also useful to have model.Trace as input/output, not just model.Batch, and the naming does not allow that.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

or even better V1BatchesToTraces

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)
Expand All @@ -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)
}
}
}
Expand All @@ -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...)
}
}
}
Expand Down
17 changes: 9 additions & 8 deletions internal/jptrace/translator_test.go → storage_v2/v1adapter/translator_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// Copyright (c) 2024 The Jaeger Authors.
// SPDX-License-Identifier: Apache-2.0

package jptrace
package v1adapter

import (
"testing"
Expand All @@ -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"
)

Expand All @@ -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()
Expand All @@ -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)

Expand Down Expand Up @@ -88,19 +89,19 @@ func TestProtoToTraces_AddsWarnings(t *testing.T) {

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))
}
3 changes: 1 addition & 2 deletions storage_v2/v1adapter/writer.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand All @@ -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 {
Expand Down
Loading