From 817db96c84ec7ba0da8f1631a1d9da40f4251f34 Mon Sep 17 00:00:00 2001 From: Navin Shrinivas Date: Tue, 23 Jan 2024 21:04:10 +0530 Subject: [PATCH 01/10] Add endpoints to allow uploading OTLP traces Signed-off-by: Navin Shrinivas partial work for issue #4949 --- cmd/query/app/apiv3/otlp_translator.go | 25 ++++++++++++++++++ cmd/query/app/http_handler.go | 36 ++++++++++++++++++++++++++ model/model.pb.go | 16 ++++++++++++ 3 files changed, 77 insertions(+) diff --git a/cmd/query/app/apiv3/otlp_translator.go b/cmd/query/app/apiv3/otlp_translator.go index b7daf1b5567..44bada7780f 100644 --- a/cmd/query/app/apiv3/otlp_translator.go +++ b/cmd/query/app/apiv3/otlp_translator.go @@ -17,8 +17,10 @@ package apiv3 import ( "fmt" + "github.com/gogo/protobuf/jsonpb" "github.com/gogo/protobuf/proto" model2otel "github.com/open-telemetry/opentelemetry-collector-contrib/pkg/translator/jaeger" + "go.opentelemetry.io/collector/pdata/ptrace" "go.opentelemetry.io/collector/pdata/ptrace/ptraceotlp" "github.com/jaegertracing/jaeger/model" @@ -45,3 +47,26 @@ func modelToOTLP(spans []*model.Span) ([]*tracev1.ResourceSpans, error) { } return chunk.ResourceSpans, nil } + +func OTLP2model(OTLPSpans []byte) ([]model.Trace, error) { + ptrace_unmarshaler := ptrace.JSONUnmarshaler{} + otlp_traces, err := ptrace_unmarshaler.UnmarshalTraces(OTLPSpans) + if err != nil { + fmt.Println(err) + return nil, fmt.Errorf("cannot marshal OTLP : %w", err) + } + batches, err := model2otel.ProtoFromTraces(otlp_traces) + fmt.Println(otlp_traces.ResourceSpans()) + if err != nil { + fmt.Println(err) + return nil, fmt.Errorf("cannot marshal OTLP : %w", err) + } + jaeger_traces := make([]model.Trace, len(batches) ) + for _, v := range batches { + mar := jsonpb.Marshaler{} + fmt.Println(mar.MarshalToString(v)) + jaeger_trace := v.ConvertToTraces() + jaeger_traces = append(jaeger_traces, *jaeger_trace) + } + return jaeger_traces, nil +} diff --git a/cmd/query/app/http_handler.go b/cmd/query/app/http_handler.go index 46f6589ea31..973d96026a8 100644 --- a/cmd/query/app/http_handler.go +++ b/cmd/query/app/http_handler.go @@ -20,6 +20,7 @@ import ( "encoding/json" "errors" "fmt" + "io/ioutil" "net/http" "net/url" "strconv" @@ -31,6 +32,7 @@ import ( "go.opentelemetry.io/otel/propagation" "go.uber.org/zap" + "github.com/jaegertracing/jaeger/cmd/query/app/apiv3" "github.com/jaegertracing/jaeger/cmd/query/app/querysvc" "github.com/jaegertracing/jaeger/model" uiconv "github.com/jaegertracing/jaeger/model/converter/json" @@ -121,6 +123,7 @@ func NewAPIHandler(queryService *querysvc.QueryService, tm *tenancy.Manager, opt // RegisterRoutes registers routes for this handler on the given router func (aH *APIHandler) RegisterRoutes(router *mux.Router) { aH.handleFunc(router, aH.getTrace, "/traces/{%s}", traceIDParam).Methods(http.MethodGet) + aH.handleFunc(router, aH.transformOTLP, "/transform").Methods(http.MethodPost) aH.handleFunc(router, aH.archiveTrace, "/archive/{%s}", traceIDParam).Methods(http.MethodPost) aH.handleFunc(router, aH.search, "/traces").Methods(http.MethodGet) aH.handleFunc(router, aH.getServices, "/services").Methods(http.MethodGet) @@ -158,6 +161,39 @@ func (aH *APIHandler) formatRoute(route string, args ...interface{}) string { return fmt.Sprintf("/%s"+route, args...) } +func (aH *APIHandler) transformOTLP(w http.ResponseWriter, r *http.Request) { + body, err := ioutil.ReadAll(r.Body) + if aH.handleError(w, err, http.StatusBadRequest) { + return + } + + if aH.handleError(w, err, http.StatusInternalServerError) { + return + } + var uiErrors []structuredError + traces, err := apiv3.OTLP2model(body) + + if aH.handleError(w, err, http.StatusInternalServerError) { + return + } + + uiTraces := make([]*ui.Trace, len(traces)) + for i, v := range traces { + uiTrace, uiErr := aH.convertModelToUI(&v, false) + if uiErr != nil { + uiErrors = append(uiErrors, *uiErr) + } + uiTraces[i] = uiTrace + } + + structuredRes := structuredResponse{ + Data: uiTraces, + Errors: uiErrors, + } + aH.writeJSON(w,r,structuredRes) + +} + func (aH *APIHandler) getServices(w http.ResponseWriter, r *http.Request) { services, err := aH.queryService.GetServices(r.Context()) if aH.handleError(w, err, http.StatusInternalServerError) { diff --git a/model/model.pb.go b/model/model.pb.go index 42290c9cce5..5d78c578d02 100644 --- a/model/model.pb.go +++ b/model/model.pb.go @@ -597,6 +597,22 @@ func (*Batch) ProtoMessage() {} func (*Batch) Descriptor() ([]byte, []int) { return fileDescriptor_4c16552f9fdb66d8, []int{6} } + +func (m *Batch)ConvertToTraces()(*Trace){ + ret_trace := Trace{} + ret_trace.Spans = m.Spans + for _,v := range ret_trace.Spans{ + v.Process = m.Process + } + ret_trace.ProcessMap = append(ret_trace.ProcessMap, Trace_ProcessMapping{ + ProcessID: m.Process.ServiceName, + Process: *m.Process, + }) + return &ret_trace + +} + + func (m *Batch) XXX_Unmarshal(b []byte) error { return m.Unmarshal(b) } From 18cd08941b1c359f3ca92bf26fe29aa33a8e209c Mon Sep 17 00:00:00 2001 From: Navin Shrinivas Date: Fri, 26 Jan 2024 11:09:02 +0530 Subject: [PATCH 02/10] Refactoring and fixes for endpoint in backend Signed-off-by: Navin Shrinivas --- cmd/query/app/apiv3/otlp_translator.go | 41 +++++++++------ cmd/query/app/apiv3/package_test.go | 58 +++++++++++++++++++++ cmd/query/app/http_handler.go | 72 ++++++++++++++------------ model/model.pb.go | 16 ------ model/trace.go | 18 +++++++ model/trace_test.go | 65 +++++++++++++++++++++++ 6 files changed, 205 insertions(+), 65 deletions(-) diff --git a/cmd/query/app/apiv3/otlp_translator.go b/cmd/query/app/apiv3/otlp_translator.go index 44bada7780f..7c8277479e8 100644 --- a/cmd/query/app/apiv3/otlp_translator.go +++ b/cmd/query/app/apiv3/otlp_translator.go @@ -17,7 +17,6 @@ package apiv3 import ( "fmt" - "github.com/gogo/protobuf/jsonpb" "github.com/gogo/protobuf/proto" model2otel "github.com/open-telemetry/opentelemetry-collector-contrib/pkg/translator/jaeger" "go.opentelemetry.io/collector/pdata/ptrace" @@ -48,25 +47,37 @@ func modelToOTLP(spans []*model.Span) ([]*tracev1.ResourceSpans, error) { return chunk.ResourceSpans, nil } -func OTLP2model(OTLPSpans []byte) ([]model.Trace, error) { - ptrace_unmarshaler := ptrace.JSONUnmarshaler{} - otlp_traces, err := ptrace_unmarshaler.UnmarshalTraces(OTLPSpans) +func OTLP2model(OTLPSpans []byte) ([]*model.Batch, error) { + ptraceUnmarshaler := ptrace.JSONUnmarshaler{} + otlpTraces, err := ptraceUnmarshaler.UnmarshalTraces(OTLPSpans) if err != nil { fmt.Println(err) - return nil, fmt.Errorf("cannot marshal OTLP : %w", err) + return nil, fmt.Errorf("cannot unmarshal OTLP : %w", err) } - batches, err := model2otel.ProtoFromTraces(otlp_traces) - fmt.Println(otlp_traces.ResourceSpans()) + jaegerBatches, err := model2otel.ProtoFromTraces(otlpTraces) if err != nil { fmt.Println(err) - return nil, fmt.Errorf("cannot marshal OTLP : %w", err) + return nil, fmt.Errorf("cannot transform OTLP to Jaeger: %w", err) } - jaeger_traces := make([]model.Trace, len(batches) ) - for _, v := range batches { - mar := jsonpb.Marshaler{} - fmt.Println(mar.MarshalToString(v)) - jaeger_trace := v.ConvertToTraces() - jaeger_traces = append(jaeger_traces, *jaeger_trace) + + return jaegerBatches, nil +} + +func BatchesToTraces(jaegerBatches []*model.Batch) ([]model.Trace, error) { + var jaegerTraces []model.Trace + spanMap := make(map[model.TraceID][]*model.Span) + for _, v := range jaegerBatches { + jaegerTrace := model.Trace{ + Spans: v.Spans, + } + jaegerTrace.DenormalizeProcess(v.Process) + jaegerTrace.FlattenToSpansMaps(spanMap) + } + for _, v := range spanMap { + jaegerTrace := model.Trace{ + Spans: v, + } + jaegerTraces = append(jaegerTraces, jaegerTrace) } - return jaeger_traces, nil + return jaegerTraces, nil } diff --git a/cmd/query/app/apiv3/package_test.go b/cmd/query/app/apiv3/package_test.go index c56173a61a1..61051eefc8b 100644 --- a/cmd/query/app/apiv3/package_test.go +++ b/cmd/query/app/apiv3/package_test.go @@ -6,9 +6,67 @@ package apiv3 import ( "testing" + "github.com/jaegertracing/jaeger/model" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "go.uber.org/goleak" ) func TestMain(m *testing.M) { goleak.VerifyTestMain(m) } + +func TestBatchesToTraces(t *testing.T) { + b1 := &model.Batch{ + Spans: []*model.Span{ + {TraceID: model.NewTraceID(1, 2), SpanID: model.NewSpanID(1), OperationName: "x"}, + {TraceID: model.NewTraceID(1, 3), SpanID: model.NewSpanID(2), OperationName: "y"}, + }, + Process: model.NewProcess("process1", model.KeyValues{}), + } + + b2 := &model.Batch{ + Spans: []*model.Span{ + {TraceID: model.NewTraceID(1, 2), SpanID: model.NewSpanID(2), OperationName: "z"}, + }, + Process: model.NewProcess("process2", model.KeyValues{}), + } + + mainBatch := []*model.Batch{b1, b2} + + traces, err := BatchesToTraces(mainBatch) + require.Nil(t, err) + + s1 := []*model.Span{ + { + TraceID: model.NewTraceID(1, 2), + SpanID: model.NewSpanID(1), + OperationName: "x", + Process: model.NewProcess("process1", model.KeyValues{}), + }, + { + TraceID: model.NewTraceID(1, 2), + SpanID: model.NewSpanID(2), + OperationName: "z", + Process: model.NewProcess("process2", model.KeyValues{}), + }, + } + + s2 := []*model.Span{ + { + TraceID: model.NewTraceID(1, 3), + SpanID: model.NewSpanID(2), + OperationName: "y", + Process: model.NewProcess("process1", model.KeyValues{}), + }, + } + + t1 := model.Trace{ + Spans: s1, + } + t2 := model.Trace{ + Spans: s2, + } + mainTrace := []model.Trace{t1, t2} + assert.Equal(t, mainTrace, traces) +} diff --git a/cmd/query/app/http_handler.go b/cmd/query/app/http_handler.go index 973d96026a8..bd8e9980bde 100644 --- a/cmd/query/app/http_handler.go +++ b/cmd/query/app/http_handler.go @@ -123,7 +123,6 @@ func NewAPIHandler(queryService *querysvc.QueryService, tm *tenancy.Manager, opt // RegisterRoutes registers routes for this handler on the given router func (aH *APIHandler) RegisterRoutes(router *mux.Router) { aH.handleFunc(router, aH.getTrace, "/traces/{%s}", traceIDParam).Methods(http.MethodGet) - aH.handleFunc(router, aH.transformOTLP, "/transform").Methods(http.MethodPost) aH.handleFunc(router, aH.archiveTrace, "/archive/{%s}", traceIDParam).Methods(http.MethodPost) aH.handleFunc(router, aH.search, "/traces").Methods(http.MethodGet) aH.handleFunc(router, aH.getServices, "/services").Methods(http.MethodGet) @@ -131,6 +130,7 @@ func (aH *APIHandler) RegisterRoutes(router *mux.Router) { aH.handleFunc(router, aH.getOperations, "/operations").Methods(http.MethodGet) // TODO - remove this when UI catches up aH.handleFunc(router, aH.getOperationsLegacy, "/services/{%s}/operations", serviceParam).Methods(http.MethodGet) + aH.handleFunc(router, aH.transformOTLP, "/transform").Methods(http.MethodPost) aH.handleFunc(router, aH.dependencies, "/dependencies").Methods(http.MethodGet) aH.handleFunc(router, aH.latencies, "/metrics/latencies").Methods(http.MethodGet) aH.handleFunc(router, aH.calls, "/metrics/calls").Methods(http.MethodGet) @@ -161,39 +161,6 @@ func (aH *APIHandler) formatRoute(route string, args ...interface{}) string { return fmt.Sprintf("/%s"+route, args...) } -func (aH *APIHandler) transformOTLP(w http.ResponseWriter, r *http.Request) { - body, err := ioutil.ReadAll(r.Body) - if aH.handleError(w, err, http.StatusBadRequest) { - return - } - - if aH.handleError(w, err, http.StatusInternalServerError) { - return - } - var uiErrors []structuredError - traces, err := apiv3.OTLP2model(body) - - if aH.handleError(w, err, http.StatusInternalServerError) { - return - } - - uiTraces := make([]*ui.Trace, len(traces)) - for i, v := range traces { - uiTrace, uiErr := aH.convertModelToUI(&v, false) - if uiErr != nil { - uiErrors = append(uiErrors, *uiErr) - } - uiTraces[i] = uiTrace - } - - structuredRes := structuredResponse{ - Data: uiTraces, - Errors: uiErrors, - } - aH.writeJSON(w,r,structuredRes) - -} - func (aH *APIHandler) getServices(w http.ResponseWriter, r *http.Request) { services, err := aH.queryService.GetServices(r.Context()) if aH.handleError(w, err, http.StatusInternalServerError) { @@ -229,6 +196,43 @@ func (aH *APIHandler) getOperationsLegacy(w http.ResponseWriter, r *http.Request aH.writeJSON(w, r, &structuredRes) } +func (aH *APIHandler) transformOTLP(w http.ResponseWriter, r *http.Request) { + body, err := ioutil.ReadAll(r.Body) + if aH.handleError(w, err, http.StatusBadRequest) { + return + } + + var uiErrors []structuredError + batches, err := apiv3.OTLP2model(body) + + if aH.handleError(w, err, http.StatusInternalServerError) { + return + } + + traces, err := apiv3.BatchesToTraces(batches) + + if aH.handleError(w, err, http.StatusInternalServerError) { + return + } + + uiTraces := make([]*ui.Trace, len(traces)) + + for i, v := range traces { + uiTrace, uiErr := aH.convertModelToUI(&v, false) + if uiErr != nil { + uiErrors = append(uiErrors, *uiErr) + } + uiTraces[i] = uiTrace + } + + structuredRes := structuredResponse{ + Data: uiTraces, + Errors: uiErrors, + } + aH.writeJSON(w, r, structuredRes) + +} + func (aH *APIHandler) getOperations(w http.ResponseWriter, r *http.Request) { service := r.FormValue(serviceParam) if service == "" { diff --git a/model/model.pb.go b/model/model.pb.go index 5d78c578d02..42290c9cce5 100644 --- a/model/model.pb.go +++ b/model/model.pb.go @@ -597,22 +597,6 @@ func (*Batch) ProtoMessage() {} func (*Batch) Descriptor() ([]byte, []int) { return fileDescriptor_4c16552f9fdb66d8, []int{6} } - -func (m *Batch)ConvertToTraces()(*Trace){ - ret_trace := Trace{} - ret_trace.Spans = m.Spans - for _,v := range ret_trace.Spans{ - v.Process = m.Process - } - ret_trace.ProcessMap = append(ret_trace.ProcessMap, Trace_ProcessMapping{ - ProcessID: m.Process.ServiceName, - Process: *m.Process, - }) - return &ret_trace - -} - - func (m *Batch) XXX_Unmarshal(b []byte) error { return m.Unmarshal(b) } diff --git a/model/trace.go b/model/trace.go index fcc31b07ae3..2d0dd763c28 100644 --- a/model/trace.go +++ b/model/trace.go @@ -32,3 +32,21 @@ func (t *Trace) NormalizeTimestamps() { span.NormalizeTimestamps() } } + +func (m *Trace) DenormalizeProcess(p *Process) { + for _, v := range m.Spans { + v.Process = p + } +} + +func (m *Trace) FlattenToSpansMaps(spanMap map[TraceID][]*Span) { + for _, v := range m.Spans { + val, ok := spanMap[v.TraceID] + if !ok { + spanMap[v.TraceID] = []*Span{v} + } else { + spanMap[v.TraceID] = append(val, v) + } + } + +} diff --git a/model/trace_test.go b/model/trace_test.go index 01a6f86c6c6..6256b974bc5 100644 --- a/model/trace_test.go +++ b/model/trace_test.go @@ -67,3 +67,68 @@ func TestTraceNormalizeTimestamps(t *testing.T) { assert.Equal(t, span.StartTime, tt1.UTC()) assert.Equal(t, span.Logs[0].Timestamp, tt2.UTC()) } + +func TestFlattenToSpanMaps(t *testing.T) { + b1 := &model.Trace{ + Spans: []*model.Span{ + {TraceID: model.NewTraceID(1, 2), SpanID: model.NewSpanID(1), OperationName: "x"}, + {TraceID: model.NewTraceID(1, 3), SpanID: model.NewSpanID(2), OperationName: "y"}, + }, + } + + b2 := &model.Trace{ + Spans: []*model.Span{ + {TraceID: model.NewTraceID(1, 2), SpanID: model.NewSpanID(2), OperationName: "z"}, + }, + } + + t1 := []*model.Span{ + {TraceID: model.NewTraceID(1, 2), SpanID: model.NewSpanID(1), OperationName: "x"}, + {TraceID: model.NewTraceID(1, 2), SpanID: model.NewSpanID(2), OperationName: "z"}} + + t2 := []*model.Span{{TraceID: model.NewTraceID(1, 3), SpanID: model.NewSpanID(2), OperationName: "y"}} + spanMap := make(map[model.TraceID][]*model.Span) + b1.FlattenToSpansMaps(spanMap) + b2.FlattenToSpansMaps(spanMap) + assert.Equal(t, t1, spanMap[model.NewTraceID(1, 2)]) + assert.Equal(t, t2, spanMap[model.NewTraceID(1, 3)]) +} + + +func TestDenormalizeProcess(t *testing.T){ + t1 := &model.Trace{ + Spans: []*model.Span{ + { + TraceID: model.NewTraceID(1, 2), + SpanID: model.NewSpanID(1), + OperationName: "x", + }, + { + TraceID: model.NewTraceID(1, 3), + SpanID: model.NewSpanID(2), + OperationName: "y", + }, + }, + } + p1 := model.NewProcess("process1", model.KeyValues{}) + + t2 := &model.Trace{ + Spans: []*model.Span{ + { + TraceID: model.NewTraceID(1, 2), + SpanID: model.NewSpanID(1), + OperationName: "x", + Process: model.NewProcess("process1", model.KeyValues{}), + }, + { + TraceID: model.NewTraceID(1, 3), + SpanID: model.NewSpanID(2), + OperationName: "y", + Process: model.NewProcess("process1", model.KeyValues{}), + }, + }, + } + t1.DenormalizeProcess(p1) + assert.Equal(t, t1, t2) + +} From d1f2e8ca2bad5ea3eb516b634c387527a4473d95 Mon Sep 17 00:00:00 2001 From: Navin Shrinivas Date: Wed, 31 Jan 2024 07:42:36 +0530 Subject: [PATCH 03/10] Placing functions in correct files Signed-off-by: Navin Shrinivas --- cmd/query/app/apiv3/otlp_translator.go | 36 ------- cmd/query/app/apiv3/package_test.go | 58 ----------- cmd/query/app/http_handler.go | 6 +- cmd/query/app/otlp_translator.go | 71 +++++++++++++ cmd/query/app/otlp_translator_test.go | 133 +++++++++++++++++++++++++ model/trace.go | 18 ---- model/trace_test.go | 65 ------------ 7 files changed, 206 insertions(+), 181 deletions(-) create mode 100644 cmd/query/app/otlp_translator.go create mode 100644 cmd/query/app/otlp_translator_test.go diff --git a/cmd/query/app/apiv3/otlp_translator.go b/cmd/query/app/apiv3/otlp_translator.go index 7c8277479e8..b7daf1b5567 100644 --- a/cmd/query/app/apiv3/otlp_translator.go +++ b/cmd/query/app/apiv3/otlp_translator.go @@ -19,7 +19,6 @@ import ( "github.com/gogo/protobuf/proto" model2otel "github.com/open-telemetry/opentelemetry-collector-contrib/pkg/translator/jaeger" - "go.opentelemetry.io/collector/pdata/ptrace" "go.opentelemetry.io/collector/pdata/ptrace/ptraceotlp" "github.com/jaegertracing/jaeger/model" @@ -46,38 +45,3 @@ func modelToOTLP(spans []*model.Span) ([]*tracev1.ResourceSpans, error) { } return chunk.ResourceSpans, nil } - -func OTLP2model(OTLPSpans []byte) ([]*model.Batch, error) { - ptraceUnmarshaler := ptrace.JSONUnmarshaler{} - otlpTraces, err := ptraceUnmarshaler.UnmarshalTraces(OTLPSpans) - if err != nil { - fmt.Println(err) - return nil, fmt.Errorf("cannot unmarshal OTLP : %w", err) - } - jaegerBatches, err := model2otel.ProtoFromTraces(otlpTraces) - if err != nil { - fmt.Println(err) - return nil, fmt.Errorf("cannot transform OTLP to Jaeger: %w", err) - } - - return jaegerBatches, nil -} - -func BatchesToTraces(jaegerBatches []*model.Batch) ([]model.Trace, error) { - var jaegerTraces []model.Trace - spanMap := make(map[model.TraceID][]*model.Span) - for _, v := range jaegerBatches { - jaegerTrace := model.Trace{ - Spans: v.Spans, - } - jaegerTrace.DenormalizeProcess(v.Process) - jaegerTrace.FlattenToSpansMaps(spanMap) - } - for _, v := range spanMap { - jaegerTrace := model.Trace{ - Spans: v, - } - jaegerTraces = append(jaegerTraces, jaegerTrace) - } - return jaegerTraces, nil -} diff --git a/cmd/query/app/apiv3/package_test.go b/cmd/query/app/apiv3/package_test.go index 61051eefc8b..c56173a61a1 100644 --- a/cmd/query/app/apiv3/package_test.go +++ b/cmd/query/app/apiv3/package_test.go @@ -6,67 +6,9 @@ package apiv3 import ( "testing" - "github.com/jaegertracing/jaeger/model" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" "go.uber.org/goleak" ) func TestMain(m *testing.M) { goleak.VerifyTestMain(m) } - -func TestBatchesToTraces(t *testing.T) { - b1 := &model.Batch{ - Spans: []*model.Span{ - {TraceID: model.NewTraceID(1, 2), SpanID: model.NewSpanID(1), OperationName: "x"}, - {TraceID: model.NewTraceID(1, 3), SpanID: model.NewSpanID(2), OperationName: "y"}, - }, - Process: model.NewProcess("process1", model.KeyValues{}), - } - - b2 := &model.Batch{ - Spans: []*model.Span{ - {TraceID: model.NewTraceID(1, 2), SpanID: model.NewSpanID(2), OperationName: "z"}, - }, - Process: model.NewProcess("process2", model.KeyValues{}), - } - - mainBatch := []*model.Batch{b1, b2} - - traces, err := BatchesToTraces(mainBatch) - require.Nil(t, err) - - s1 := []*model.Span{ - { - TraceID: model.NewTraceID(1, 2), - SpanID: model.NewSpanID(1), - OperationName: "x", - Process: model.NewProcess("process1", model.KeyValues{}), - }, - { - TraceID: model.NewTraceID(1, 2), - SpanID: model.NewSpanID(2), - OperationName: "z", - Process: model.NewProcess("process2", model.KeyValues{}), - }, - } - - s2 := []*model.Span{ - { - TraceID: model.NewTraceID(1, 3), - SpanID: model.NewSpanID(2), - OperationName: "y", - Process: model.NewProcess("process1", model.KeyValues{}), - }, - } - - t1 := model.Trace{ - Spans: s1, - } - t2 := model.Trace{ - Spans: s2, - } - mainTrace := []model.Trace{t1, t2} - assert.Equal(t, mainTrace, traces) -} diff --git a/cmd/query/app/http_handler.go b/cmd/query/app/http_handler.go index bd8e9980bde..283703e24d7 100644 --- a/cmd/query/app/http_handler.go +++ b/cmd/query/app/http_handler.go @@ -32,7 +32,6 @@ import ( "go.opentelemetry.io/otel/propagation" "go.uber.org/zap" - "github.com/jaegertracing/jaeger/cmd/query/app/apiv3" "github.com/jaegertracing/jaeger/cmd/query/app/querysvc" "github.com/jaegertracing/jaeger/model" uiconv "github.com/jaegertracing/jaeger/model/converter/json" @@ -203,13 +202,13 @@ func (aH *APIHandler) transformOTLP(w http.ResponseWriter, r *http.Request) { } var uiErrors []structuredError - batches, err := apiv3.OTLP2model(body) + batches, err := OTLP2model(body) if aH.handleError(w, err, http.StatusInternalServerError) { return } - traces, err := apiv3.BatchesToTraces(batches) + traces, err := BatchesToTraces(batches) if aH.handleError(w, err, http.StatusInternalServerError) { return @@ -230,7 +229,6 @@ func (aH *APIHandler) transformOTLP(w http.ResponseWriter, r *http.Request) { Errors: uiErrors, } aH.writeJSON(w, r, structuredRes) - } func (aH *APIHandler) getOperations(w http.ResponseWriter, r *http.Request) { diff --git a/cmd/query/app/otlp_translator.go b/cmd/query/app/otlp_translator.go new file mode 100644 index 00000000000..1e2d1494f82 --- /dev/null +++ b/cmd/query/app/otlp_translator.go @@ -0,0 +1,71 @@ +// Copyright (c) 2024 The Jaeger Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package app + +import ( + "fmt" + + model2otel "github.com/open-telemetry/opentelemetry-collector-contrib/pkg/translator/jaeger" + "go.opentelemetry.io/collector/pdata/ptrace" + + "github.com/jaegertracing/jaeger/model" +) + +func OTLP2model(OTLPSpans []byte) ([]*model.Batch, error) { + ptraceUnmarshaler := ptrace.JSONUnmarshaler{} + otlpTraces, err := ptraceUnmarshaler.UnmarshalTraces(OTLPSpans) + if err != nil { + return nil, fmt.Errorf("cannot unmarshal OTLP : %w", err) + } + jaegerBatches, err := model2otel.ProtoFromTraces(otlpTraces) + if err != nil { + return nil, fmt.Errorf("cannot transform OTLP to Jaeger: %w", err) + } + + return jaegerBatches, nil +} + +func BatchesToTraces(jaegerBatches []*model.Batch) ([]model.Trace, error) { + var jaegerTraces []model.Trace + spanMap := make(map[model.TraceID][]*model.Span) + for _, v := range jaegerBatches { + DenormalizeProcess(v) + FlattenToSpansMaps(v, spanMap) + } + for _, v := range spanMap { + jaegerTrace := model.Trace{ + Spans: v, + } + jaegerTraces = append(jaegerTraces, jaegerTrace) + } + return jaegerTraces, nil +} + +func DenormalizeProcess(m *model.Batch) { + for _, v := range m.Spans { + v.Process = m.Process + } +} + +func FlattenToSpansMaps(m *model.Batch, spanMap map[model.TraceID][]*model.Span) { + for _, v := range m.Spans { + val, ok := spanMap[v.TraceID] + if !ok { + spanMap[v.TraceID] = []*model.Span{v} + } else { + spanMap[v.TraceID] = append(val, v) + } + } +} diff --git a/cmd/query/app/otlp_translator_test.go b/cmd/query/app/otlp_translator_test.go new file mode 100644 index 00000000000..56edc6ba0cf --- /dev/null +++ b/cmd/query/app/otlp_translator_test.go @@ -0,0 +1,133 @@ +// Copyright (c) 2024 The Jaeger Authors. +// SPDX-License-Identifier: Apache-2.0 + +package app + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/jaegertracing/jaeger/model" +) + +func TestBatchesToTraces(t *testing.T) { + b1 := &model.Batch{ + Spans: []*model.Span{ + {TraceID: model.NewTraceID(1, 2), SpanID: model.NewSpanID(1), OperationName: "x"}, + {TraceID: model.NewTraceID(1, 3), SpanID: model.NewSpanID(2), OperationName: "y"}, + }, + Process: model.NewProcess("process1", model.KeyValues{}), + } + + b2 := &model.Batch{ + Spans: []*model.Span{ + {TraceID: model.NewTraceID(1, 2), SpanID: model.NewSpanID(2), OperationName: "z"}, + }, + Process: model.NewProcess("process2", model.KeyValues{}), + } + + mainBatch := []*model.Batch{b1, b2} + + traces, err := BatchesToTraces(mainBatch) + require.Nil(t, err) + + s1 := []*model.Span{ + { + TraceID: model.NewTraceID(1, 2), + SpanID: model.NewSpanID(1), + OperationName: "x", + Process: model.NewProcess("process1", model.KeyValues{}), + }, + { + TraceID: model.NewTraceID(1, 2), + SpanID: model.NewSpanID(2), + OperationName: "z", + Process: model.NewProcess("process2", model.KeyValues{}), + }, + } + + s2 := []*model.Span{ + { + TraceID: model.NewTraceID(1, 3), + SpanID: model.NewSpanID(2), + OperationName: "y", + Process: model.NewProcess("process1", model.KeyValues{}), + }, + } + + t1 := model.Trace{ + Spans: s1, + } + t2 := model.Trace{ + Spans: s2, + } + mainTrace := []model.Trace{t1, t2} + assert.Equal(t, mainTrace, traces) +} + +func TestFlattenToSpanMaps(t *testing.T) { + b1 := &model.Batch{ + Spans: []*model.Span{ + {TraceID: model.NewTraceID(1, 2), SpanID: model.NewSpanID(1), OperationName: "x"}, + {TraceID: model.NewTraceID(1, 3), SpanID: model.NewSpanID(2), OperationName: "y"}, + }, + } + + b2 := &model.Batch{ + Spans: []*model.Span{ + {TraceID: model.NewTraceID(1, 2), SpanID: model.NewSpanID(2), OperationName: "z"}, + }, + } + + t1 := []*model.Span{ + {TraceID: model.NewTraceID(1, 2), SpanID: model.NewSpanID(1), OperationName: "x"}, + {TraceID: model.NewTraceID(1, 2), SpanID: model.NewSpanID(2), OperationName: "z"}, + } + + t2 := []*model.Span{{TraceID: model.NewTraceID(1, 3), SpanID: model.NewSpanID(2), OperationName: "y"}} + spanMap := make(map[model.TraceID][]*model.Span) + FlattenToSpansMaps(b1, spanMap) + FlattenToSpansMaps(b2, spanMap) + assert.Equal(t, t1, spanMap[model.NewTraceID(1, 2)]) + assert.Equal(t, t2, spanMap[model.NewTraceID(1, 3)]) +} + +func TestDenormalizeProcess(t *testing.T) { + b1 := &model.Batch{ + Spans: []*model.Span{ + { + TraceID: model.NewTraceID(1, 2), + SpanID: model.NewSpanID(1), + OperationName: "x", + }, + { + TraceID: model.NewTraceID(1, 3), + SpanID: model.NewSpanID(2), + OperationName: "y", + }, + }, + Process: model.NewProcess("process1", model.KeyValues{}), + } + + b2 := &model.Batch{ + Spans: []*model.Span{ + { + TraceID: model.NewTraceID(1, 2), + SpanID: model.NewSpanID(1), + OperationName: "x", + Process: model.NewProcess("process1", model.KeyValues{}), + }, + { + TraceID: model.NewTraceID(1, 3), + SpanID: model.NewSpanID(2), + OperationName: "y", + Process: model.NewProcess("process1", model.KeyValues{}), + }, + }, + Process: model.NewProcess("process1", model.KeyValues{}), + } + DenormalizeProcess(b1) + assert.Equal(t, b1, b2) +} diff --git a/model/trace.go b/model/trace.go index 2d0dd763c28..fcc31b07ae3 100644 --- a/model/trace.go +++ b/model/trace.go @@ -32,21 +32,3 @@ func (t *Trace) NormalizeTimestamps() { span.NormalizeTimestamps() } } - -func (m *Trace) DenormalizeProcess(p *Process) { - for _, v := range m.Spans { - v.Process = p - } -} - -func (m *Trace) FlattenToSpansMaps(spanMap map[TraceID][]*Span) { - for _, v := range m.Spans { - val, ok := spanMap[v.TraceID] - if !ok { - spanMap[v.TraceID] = []*Span{v} - } else { - spanMap[v.TraceID] = append(val, v) - } - } - -} diff --git a/model/trace_test.go b/model/trace_test.go index 6256b974bc5..01a6f86c6c6 100644 --- a/model/trace_test.go +++ b/model/trace_test.go @@ -67,68 +67,3 @@ func TestTraceNormalizeTimestamps(t *testing.T) { assert.Equal(t, span.StartTime, tt1.UTC()) assert.Equal(t, span.Logs[0].Timestamp, tt2.UTC()) } - -func TestFlattenToSpanMaps(t *testing.T) { - b1 := &model.Trace{ - Spans: []*model.Span{ - {TraceID: model.NewTraceID(1, 2), SpanID: model.NewSpanID(1), OperationName: "x"}, - {TraceID: model.NewTraceID(1, 3), SpanID: model.NewSpanID(2), OperationName: "y"}, - }, - } - - b2 := &model.Trace{ - Spans: []*model.Span{ - {TraceID: model.NewTraceID(1, 2), SpanID: model.NewSpanID(2), OperationName: "z"}, - }, - } - - t1 := []*model.Span{ - {TraceID: model.NewTraceID(1, 2), SpanID: model.NewSpanID(1), OperationName: "x"}, - {TraceID: model.NewTraceID(1, 2), SpanID: model.NewSpanID(2), OperationName: "z"}} - - t2 := []*model.Span{{TraceID: model.NewTraceID(1, 3), SpanID: model.NewSpanID(2), OperationName: "y"}} - spanMap := make(map[model.TraceID][]*model.Span) - b1.FlattenToSpansMaps(spanMap) - b2.FlattenToSpansMaps(spanMap) - assert.Equal(t, t1, spanMap[model.NewTraceID(1, 2)]) - assert.Equal(t, t2, spanMap[model.NewTraceID(1, 3)]) -} - - -func TestDenormalizeProcess(t *testing.T){ - t1 := &model.Trace{ - Spans: []*model.Span{ - { - TraceID: model.NewTraceID(1, 2), - SpanID: model.NewSpanID(1), - OperationName: "x", - }, - { - TraceID: model.NewTraceID(1, 3), - SpanID: model.NewSpanID(2), - OperationName: "y", - }, - }, - } - p1 := model.NewProcess("process1", model.KeyValues{}) - - t2 := &model.Trace{ - Spans: []*model.Span{ - { - TraceID: model.NewTraceID(1, 2), - SpanID: model.NewSpanID(1), - OperationName: "x", - Process: model.NewProcess("process1", model.KeyValues{}), - }, - { - TraceID: model.NewTraceID(1, 3), - SpanID: model.NewSpanID(2), - OperationName: "y", - Process: model.NewProcess("process1", model.KeyValues{}), - }, - }, - } - t1.DenormalizeProcess(p1) - assert.Equal(t, t1, t2) - -} From c98bb4dc71380038fa414b27ccc2aeb63affd816 Mon Sep 17 00:00:00 2001 From: Navin Shrinivas Date: Wed, 31 Jan 2024 12:04:11 +0530 Subject: [PATCH 04/10] Add fixtures and Unit tests for API Signed-off-by: Navin Shrinivas --- cmd/query/app/fixture/otlp2jaeger-in.json | 52 ++++++++++++++++ cmd/query/app/fixture/otlp2jaeger-out.json | 59 ++++++++++++++++++ cmd/query/app/http_handler.go | 6 +- cmd/query/app/http_handler_test.go | 51 ++++++++++++++++ cmd/query/app/otlp_translator.go | 51 +++++++--------- cmd/query/app/otlp_translator_test.go | 69 +--------------------- 6 files changed, 187 insertions(+), 101 deletions(-) create mode 100644 cmd/query/app/fixture/otlp2jaeger-in.json create mode 100644 cmd/query/app/fixture/otlp2jaeger-out.json diff --git a/cmd/query/app/fixture/otlp2jaeger-in.json b/cmd/query/app/fixture/otlp2jaeger-in.json new file mode 100644 index 00000000000..39577361e72 --- /dev/null +++ b/cmd/query/app/fixture/otlp2jaeger-in.json @@ -0,0 +1,52 @@ +{ + "resourceSpans":[ + { + "resource":{ + "attributes":[ + { + "key":"service.name", + "value":{ + "stringValue":"telemetrygen" + } + } + ] + }, + "scopeSpans":[ + { + "scope":{ + "name":"telemetrygen" + }, + "spans":[ + { + "traceId":"83a9efd15c1c98a977e0711cc93ee28b", + "spanId":"e127af99e3b3e074", + "parentSpanId":"909541b92cf05311", + "name":"okey-dokey-0", + "kind":2, + "startTimeUnixNano":"1706678909209712000", + "endTimeUnixNano":"1706678909209835000", + "attributes":[ + { + "key":"net.peer.ip", + "value":{ + "stringValue":"1.2.3.4" + } + }, + { + "key":"peer.service", + "value":{ + "stringValue":"telemetrygen-client" + } + } + ], + "status":{ + + } + } + ] + } + ], + "schemaUrl":"https://opentelemetry.io/schemas/1.4.0" + } + ] +} diff --git a/cmd/query/app/fixture/otlp2jaeger-out.json b/cmd/query/app/fixture/otlp2jaeger-out.json new file mode 100644 index 00000000000..3041467c0d4 --- /dev/null +++ b/cmd/query/app/fixture/otlp2jaeger-out.json @@ -0,0 +1,59 @@ +{ + "data": [ + { + "traceID": "83a9efd15c1c98a977e0711cc93ee28b", + "spans": [ + { + "traceID": "83a9efd15c1c98a977e0711cc93ee28b", + "spanID": "e127af99e3b3e074", + "operationName": "okey-dokey-0", + "references": [ + { + "refType": "CHILD_OF", + "traceID": "83a9efd15c1c98a977e0711cc93ee28b", + "spanID": "909541b92cf05311" + } + ], + "startTime": 1706678909209712, + "duration": 123, + "tags": [ + { + "key": "otel.library.name", + "type": "string", + "value": "telemetrygen" + }, + { + "key": "net.peer.ip", + "type": "string", + "value": "1.2.3.4" + }, + { + "key": "peer.service", + "type": "string", + "value": "telemetrygen-client" + }, + { + "key": "span.kind", + "type": "string", + "value": "server" + } + ], + "logs": [], + "processID": "p1", + "warnings": null + } + ], + "processes": { + "p1": { + "serviceName": "telemetrygen", + "tags": [] + } + }, + "warnings": null + } + ], + "total": 0, + "limit": 0, + "offset": 0, + "errors": null +} diff --git a/cmd/query/app/http_handler.go b/cmd/query/app/http_handler.go index 283703e24d7..bc769de0579 100644 --- a/cmd/query/app/http_handler.go +++ b/cmd/query/app/http_handler.go @@ -202,13 +202,13 @@ func (aH *APIHandler) transformOTLP(w http.ResponseWriter, r *http.Request) { } var uiErrors []structuredError - batches, err := OTLP2model(body) + batches, err := otlp2model(body) if aH.handleError(w, err, http.StatusInternalServerError) { return } - traces, err := BatchesToTraces(batches) + traces, err := batchesToTraces(batches) if aH.handleError(w, err, http.StatusInternalServerError) { return @@ -217,7 +217,7 @@ func (aH *APIHandler) transformOTLP(w http.ResponseWriter, r *http.Request) { uiTraces := make([]*ui.Trace, len(traces)) for i, v := range traces { - uiTrace, uiErr := aH.convertModelToUI(&v, false) + uiTrace, uiErr := aH.convertModelToUI(v, false) if uiErr != nil { uiErrors = append(uiErrors, *uiErr) } diff --git a/cmd/query/app/http_handler_test.go b/cmd/query/app/http_handler_test.go index d3d1507078e..3602cd18f77 100644 --- a/cmd/query/app/http_handler_test.go +++ b/cmd/query/app/http_handler_test.go @@ -25,6 +25,7 @@ import ( "math" "net/http" "net/http/httptest" + "os" "testing" "time" @@ -270,6 +271,33 @@ func TestWriteJSON(t *testing.T) { } } +func readOTLPTraces(t *testing.T) (interface{}, error) { + dat, err := os.ReadFile("./fixture/otlp2jaeger-in.json") + if err != nil { + return nil, err + } else { + require.NoError(t, err) + var out interface{} + err := json.Unmarshal(dat, out) + require.NoError(t, err) + return out, nil + + } +} + +func readJaegerTraces(t *testing.T) (interface{}, error) { + dat, err := os.ReadFile("./fixture/otlp2jaeger-out.json") + if err != nil { + return nil, err + } else { + var out interface{} + require.NoError(t, err) + err := json.Unmarshal(dat, out) + require.NoError(t, err) + return out, nil + } +} + func TestGetTrace(t *testing.T) { testCases := []struct { suffix string @@ -623,6 +651,29 @@ func TestGetOperationsLegacyStorageFailure(t *testing.T) { require.Error(t, err) } +func TestTransformOTLPSuccess(t *testing.T) { + withTestServer(func(ts *testServer) { + var response interface{} + request, err := readOTLPTraces(t) + require.NoError(t, err) + err = postJSON(ts.server.URL+"/api/transform", request, response) + require.NoError(t, err) + corectResponse, err := readJaegerTraces(t) + require.NoError(t, err) + assert.Equal(t, response, corectResponse) + }, querysvc.QueryServiceOptions{}) +} + +func TestTransformOTLPEmptyFailure(t *testing.T) { + withTestServer(func(ts *testServer) { + var response interface{} + var request interface{} // Keeping request empty for checking behaviour + request = "" + err := postJSON(ts.server.URL+"/api/transform", request, response) + require.Error(t, err) + }, querysvc.QueryServiceOptions{}) +} + func TestGetMetricsSuccess(t *testing.T) { mr := &metricsmocks.Reader{} apiHandlerOptions := []HandlerOption{ diff --git a/cmd/query/app/otlp_translator.go b/cmd/query/app/otlp_translator.go index 1e2d1494f82..26a4728c92c 100644 --- a/cmd/query/app/otlp_translator.go +++ b/cmd/query/app/otlp_translator.go @@ -23,7 +23,7 @@ import ( "github.com/jaegertracing/jaeger/model" ) -func OTLP2model(OTLPSpans []byte) ([]*model.Batch, error) { +func otlp2model(OTLPSpans []byte) ([]*model.Batch, error) { ptraceUnmarshaler := ptrace.JSONUnmarshaler{} otlpTraces, err := ptraceUnmarshaler.UnmarshalTraces(OTLPSpans) if err != nil { @@ -33,39 +33,28 @@ func OTLP2model(OTLPSpans []byte) ([]*model.Batch, error) { if err != nil { return nil, fmt.Errorf("cannot transform OTLP to Jaeger: %w", err) } - return jaegerBatches, nil } -func BatchesToTraces(jaegerBatches []*model.Batch) ([]model.Trace, error) { - var jaegerTraces []model.Trace - spanMap := make(map[model.TraceID][]*model.Span) - for _, v := range jaegerBatches { - DenormalizeProcess(v) - FlattenToSpansMaps(v, spanMap) - } - for _, v := range spanMap { - jaegerTrace := model.Trace{ - Spans: v, - } - jaegerTraces = append(jaegerTraces, jaegerTrace) - } - return jaegerTraces, nil -} - -func DenormalizeProcess(m *model.Batch) { - for _, v := range m.Spans { - v.Process = m.Process - } -} - -func FlattenToSpansMaps(m *model.Batch, spanMap map[model.TraceID][]*model.Span) { - for _, v := range m.Spans { - val, ok := spanMap[v.TraceID] - if !ok { - spanMap[v.TraceID] = []*model.Span{v} - } else { - spanMap[v.TraceID] = append(val, v) +func batchesToTraces(jaegerBatches []*model.Batch) ([]*model.Trace, error) { + var traces []*model.Trace + traceMap := make(map[model.TraceID]*model.Trace) + for _, batch := range jaegerBatches { + for _, span := range batch.Spans { + if span.Process == nil { + span.Process = batch.Process + } + trace, ok := traceMap[span.TraceID] + if !ok { + newtrace := model.Trace{ + Spans: []*model.Span{span}, + } + traceMap[span.TraceID] = &newtrace + traces = append(traces, &newtrace) + } else { + trace.Spans = append(trace.Spans, span) + } } } + return traces, nil } diff --git a/cmd/query/app/otlp_translator_test.go b/cmd/query/app/otlp_translator_test.go index 56edc6ba0cf..60b6fd77286 100644 --- a/cmd/query/app/otlp_translator_test.go +++ b/cmd/query/app/otlp_translator_test.go @@ -30,7 +30,7 @@ func TestBatchesToTraces(t *testing.T) { mainBatch := []*model.Batch{b1, b2} - traces, err := BatchesToTraces(mainBatch) + traces, err := batchesToTraces(mainBatch) require.Nil(t, err) s1 := []*model.Span{ @@ -63,71 +63,6 @@ func TestBatchesToTraces(t *testing.T) { t2 := model.Trace{ Spans: s2, } - mainTrace := []model.Trace{t1, t2} + mainTrace := []*model.Trace{&t1, &t2} assert.Equal(t, mainTrace, traces) } - -func TestFlattenToSpanMaps(t *testing.T) { - b1 := &model.Batch{ - Spans: []*model.Span{ - {TraceID: model.NewTraceID(1, 2), SpanID: model.NewSpanID(1), OperationName: "x"}, - {TraceID: model.NewTraceID(1, 3), SpanID: model.NewSpanID(2), OperationName: "y"}, - }, - } - - b2 := &model.Batch{ - Spans: []*model.Span{ - {TraceID: model.NewTraceID(1, 2), SpanID: model.NewSpanID(2), OperationName: "z"}, - }, - } - - t1 := []*model.Span{ - {TraceID: model.NewTraceID(1, 2), SpanID: model.NewSpanID(1), OperationName: "x"}, - {TraceID: model.NewTraceID(1, 2), SpanID: model.NewSpanID(2), OperationName: "z"}, - } - - t2 := []*model.Span{{TraceID: model.NewTraceID(1, 3), SpanID: model.NewSpanID(2), OperationName: "y"}} - spanMap := make(map[model.TraceID][]*model.Span) - FlattenToSpansMaps(b1, spanMap) - FlattenToSpansMaps(b2, spanMap) - assert.Equal(t, t1, spanMap[model.NewTraceID(1, 2)]) - assert.Equal(t, t2, spanMap[model.NewTraceID(1, 3)]) -} - -func TestDenormalizeProcess(t *testing.T) { - b1 := &model.Batch{ - Spans: []*model.Span{ - { - TraceID: model.NewTraceID(1, 2), - SpanID: model.NewSpanID(1), - OperationName: "x", - }, - { - TraceID: model.NewTraceID(1, 3), - SpanID: model.NewSpanID(2), - OperationName: "y", - }, - }, - Process: model.NewProcess("process1", model.KeyValues{}), - } - - b2 := &model.Batch{ - Spans: []*model.Span{ - { - TraceID: model.NewTraceID(1, 2), - SpanID: model.NewSpanID(1), - OperationName: "x", - Process: model.NewProcess("process1", model.KeyValues{}), - }, - { - TraceID: model.NewTraceID(1, 3), - SpanID: model.NewSpanID(2), - OperationName: "y", - Process: model.NewProcess("process1", model.KeyValues{}), - }, - }, - Process: model.NewProcess("process1", model.KeyValues{}), - } - DenormalizeProcess(b1) - assert.Equal(t, b1, b2) -} From 58c92be9f2691544ba64861b504435d1b2aa800c Mon Sep 17 00:00:00 2001 From: Navin Shrinivas Date: Wed, 7 Feb 2024 12:00:16 +0530 Subject: [PATCH 05/10] Fixing test and complying with linter Signed-off-by: Navin Shrinivas --- cmd/query/app/http_handler.go | 4 +-- cmd/query/app/http_handler_test.go | 36 ++++++++++----------------- cmd/query/app/otlp_translator.go | 4 +-- cmd/query/app/otlp_translator_test.go | 2 +- 4 files changed, 18 insertions(+), 28 deletions(-) diff --git a/cmd/query/app/http_handler.go b/cmd/query/app/http_handler.go index bc769de0579..8f1dfbbf96a 100644 --- a/cmd/query/app/http_handler.go +++ b/cmd/query/app/http_handler.go @@ -20,7 +20,7 @@ import ( "encoding/json" "errors" "fmt" - "io/ioutil" + "io" "net/http" "net/url" "strconv" @@ -196,7 +196,7 @@ func (aH *APIHandler) getOperationsLegacy(w http.ResponseWriter, r *http.Request } func (aH *APIHandler) transformOTLP(w http.ResponseWriter, r *http.Request) { - body, err := ioutil.ReadAll(r.Body) + body, err := io.ReadAll(r.Body) if aH.handleError(w, err, http.StatusBadRequest) { return } diff --git a/cmd/query/app/http_handler_test.go b/cmd/query/app/http_handler_test.go index 3602cd18f77..666690f27db 100644 --- a/cmd/query/app/http_handler_test.go +++ b/cmd/query/app/http_handler_test.go @@ -273,29 +273,20 @@ func TestWriteJSON(t *testing.T) { func readOTLPTraces(t *testing.T) (interface{}, error) { dat, err := os.ReadFile("./fixture/otlp2jaeger-in.json") - if err != nil { - return nil, err - } else { - require.NoError(t, err) - var out interface{} - err := json.Unmarshal(dat, out) - require.NoError(t, err) - return out, nil - - } + require.NoError(t, err) + out := new(interface{}) + err = json.Unmarshal(dat, out) + require.NoError(t, err) + return out, nil } func readJaegerTraces(t *testing.T) (interface{}, error) { dat, err := os.ReadFile("./fixture/otlp2jaeger-out.json") - if err != nil { - return nil, err - } else { - var out interface{} - require.NoError(t, err) - err := json.Unmarshal(dat, out) - require.NoError(t, err) - return out, nil - } + out := new(interface{}) + require.NoError(t, err) + err = json.Unmarshal(dat, out) + require.NoError(t, err) + return out, nil } func TestGetTrace(t *testing.T) { @@ -653,7 +644,7 @@ func TestGetOperationsLegacyStorageFailure(t *testing.T) { func TestTransformOTLPSuccess(t *testing.T) { withTestServer(func(ts *testServer) { - var response interface{} + response := new(interface{}) request, err := readOTLPTraces(t) require.NoError(t, err) err = postJSON(ts.server.URL+"/api/transform", request, response) @@ -666,9 +657,8 @@ func TestTransformOTLPSuccess(t *testing.T) { func TestTransformOTLPEmptyFailure(t *testing.T) { withTestServer(func(ts *testServer) { - var response interface{} - var request interface{} // Keeping request empty for checking behaviour - request = "" + response := new(interface{}) + request := "" err := postJSON(ts.server.URL+"/api/transform", request, response) require.Error(t, err) }, querysvc.QueryServiceOptions{}) diff --git a/cmd/query/app/otlp_translator.go b/cmd/query/app/otlp_translator.go index 26a4728c92c..25a3cad1f16 100644 --- a/cmd/query/app/otlp_translator.go +++ b/cmd/query/app/otlp_translator.go @@ -23,9 +23,9 @@ import ( "github.com/jaegertracing/jaeger/model" ) -func otlp2model(OTLPSpans []byte) ([]*model.Batch, error) { +func otlp2model(otlpSpans []byte) ([]*model.Batch, error) { ptraceUnmarshaler := ptrace.JSONUnmarshaler{} - otlpTraces, err := ptraceUnmarshaler.UnmarshalTraces(OTLPSpans) + otlpTraces, err := ptraceUnmarshaler.UnmarshalTraces(otlpSpans) if err != nil { return nil, fmt.Errorf("cannot unmarshal OTLP : %w", err) } diff --git a/cmd/query/app/otlp_translator_test.go b/cmd/query/app/otlp_translator_test.go index 60b6fd77286..894cefcdbb8 100644 --- a/cmd/query/app/otlp_translator_test.go +++ b/cmd/query/app/otlp_translator_test.go @@ -31,7 +31,7 @@ func TestBatchesToTraces(t *testing.T) { mainBatch := []*model.Batch{b1, b2} traces, err := batchesToTraces(mainBatch) - require.Nil(t, err) + require.NoError(t, err) s1 := []*model.Span{ { From ea7a80c1f3f67566ec74d6665dd40d093cb47cde Mon Sep 17 00:00:00 2001 From: Navin Shrinivas Date: Sat, 10 Feb 2024 15:38:52 +0530 Subject: [PATCH 06/10] Fixings tests and merging into one function Signed-off-by: Navin Shrinivas --- cmd/query/app/http_handler.go | 8 +--- cmd/query/app/http_handler_test.go | 33 +++++-------- cmd/query/app/otlp_translator.go | 11 ++--- cmd/query/app/otlp_translator_test.go | 68 ++++++--------------------- 4 files changed, 31 insertions(+), 89 deletions(-) diff --git a/cmd/query/app/http_handler.go b/cmd/query/app/http_handler.go index 8f1dfbbf96a..a7600c0e5e9 100644 --- a/cmd/query/app/http_handler.go +++ b/cmd/query/app/http_handler.go @@ -202,13 +202,7 @@ func (aH *APIHandler) transformOTLP(w http.ResponseWriter, r *http.Request) { } var uiErrors []structuredError - batches, err := otlp2model(body) - - if aH.handleError(w, err, http.StatusInternalServerError) { - return - } - - traces, err := batchesToTraces(batches) + traces, err := otlp2traces(body) if aH.handleError(w, err, http.StatusInternalServerError) { return diff --git a/cmd/query/app/http_handler_test.go b/cmd/query/app/http_handler_test.go index 666690f27db..0a23866e137 100644 --- a/cmd/query/app/http_handler_test.go +++ b/cmd/query/app/http_handler_test.go @@ -25,7 +25,6 @@ import ( "math" "net/http" "net/http/httptest" - "os" "testing" "time" @@ -271,24 +270,6 @@ func TestWriteJSON(t *testing.T) { } } -func readOTLPTraces(t *testing.T) (interface{}, error) { - dat, err := os.ReadFile("./fixture/otlp2jaeger-in.json") - require.NoError(t, err) - out := new(interface{}) - err = json.Unmarshal(dat, out) - require.NoError(t, err) - return out, nil -} - -func readJaegerTraces(t *testing.T) (interface{}, error) { - dat, err := os.ReadFile("./fixture/otlp2jaeger-out.json") - out := new(interface{}) - require.NoError(t, err) - err = json.Unmarshal(dat, out) - require.NoError(t, err) - return out, nil -} - func TestGetTrace(t *testing.T) { testCases := []struct { suffix string @@ -645,12 +626,22 @@ func TestGetOperationsLegacyStorageFailure(t *testing.T) { func TestTransformOTLPSuccess(t *testing.T) { withTestServer(func(ts *testServer) { response := new(interface{}) - request, err := readOTLPTraces(t) + request := new(interface{}) + + requestBytes := readOTLPTraces(t) + + err := json.Unmarshal(requestBytes, request) require.NoError(t, err) + err = postJSON(ts.server.URL+"/api/transform", request, response) require.NoError(t, err) - corectResponse, err := readJaegerTraces(t) + + corectResponseBytes := readJaegerTraces(t) + corectResponse := new(interface{}) + + err = json.Unmarshal(corectResponseBytes, corectResponse) require.NoError(t, err) + assert.Equal(t, response, corectResponse) }, querysvc.QueryServiceOptions{}) } diff --git a/cmd/query/app/otlp_translator.go b/cmd/query/app/otlp_translator.go index 25a3cad1f16..2e29d1bae99 100644 --- a/cmd/query/app/otlp_translator.go +++ b/cmd/query/app/otlp_translator.go @@ -23,20 +23,15 @@ import ( "github.com/jaegertracing/jaeger/model" ) -func otlp2model(otlpSpans []byte) ([]*model.Batch, error) { +func otlp2traces(otlpSpans []byte) ([]*model.Trace, error) { ptraceUnmarshaler := ptrace.JSONUnmarshaler{} otlpTraces, err := ptraceUnmarshaler.UnmarshalTraces(otlpSpans) if err != nil { return nil, fmt.Errorf("cannot unmarshal OTLP : %w", err) } - jaegerBatches, err := model2otel.ProtoFromTraces(otlpTraces) - if err != nil { - return nil, fmt.Errorf("cannot transform OTLP to Jaeger: %w", err) - } - return jaegerBatches, nil -} + jaegerBatches, _ := model2otel.ProtoFromTraces(otlpTraces) + // ProtoFromTraces will not give an error -func batchesToTraces(jaegerBatches []*model.Batch) ([]*model.Trace, error) { var traces []*model.Trace traceMap := make(map[model.TraceID]*model.Trace) for _, batch := range jaegerBatches { diff --git a/cmd/query/app/otlp_translator_test.go b/cmd/query/app/otlp_translator_test.go index 894cefcdbb8..da78440f4b7 100644 --- a/cmd/query/app/otlp_translator_test.go +++ b/cmd/query/app/otlp_translator_test.go @@ -4,65 +4,27 @@ package app import ( + "os" "testing" - "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - - "github.com/jaegertracing/jaeger/model" ) -func TestBatchesToTraces(t *testing.T) { - b1 := &model.Batch{ - Spans: []*model.Span{ - {TraceID: model.NewTraceID(1, 2), SpanID: model.NewSpanID(1), OperationName: "x"}, - {TraceID: model.NewTraceID(1, 3), SpanID: model.NewSpanID(2), OperationName: "y"}, - }, - Process: model.NewProcess("process1", model.KeyValues{}), - } - - b2 := &model.Batch{ - Spans: []*model.Span{ - {TraceID: model.NewTraceID(1, 2), SpanID: model.NewSpanID(2), OperationName: "z"}, - }, - Process: model.NewProcess("process2", model.KeyValues{}), - } - - mainBatch := []*model.Batch{b1, b2} - - traces, err := batchesToTraces(mainBatch) +func readOTLPTraces(t *testing.T) []byte { + dat, err := os.ReadFile("./fixture/otlp2jaeger-in.json") require.NoError(t, err) + return dat +} - s1 := []*model.Span{ - { - TraceID: model.NewTraceID(1, 2), - SpanID: model.NewSpanID(1), - OperationName: "x", - Process: model.NewProcess("process1", model.KeyValues{}), - }, - { - TraceID: model.NewTraceID(1, 2), - SpanID: model.NewSpanID(2), - OperationName: "z", - Process: model.NewProcess("process2", model.KeyValues{}), - }, - } - - s2 := []*model.Span{ - { - TraceID: model.NewTraceID(1, 3), - SpanID: model.NewSpanID(2), - OperationName: "y", - Process: model.NewProcess("process1", model.KeyValues{}), - }, - } +func readJaegerTraces(t *testing.T) []byte { + dat, err := os.ReadFile("./fixture/otlp2jaeger-out.json") + require.NoError(t, err) + return dat +} - t1 := model.Trace{ - Spans: s1, - } - t2 := model.Trace{ - Spans: s2, - } - mainTrace := []*model.Trace{&t1, &t2} - assert.Equal(t, mainTrace, traces) +func TestOtlp2Traces(t *testing.T) { + OTLPTraces := readOTLPTraces(t) + _, err := otlp2traces(OTLPTraces) + require.NoError(t, err) + // Correctness of outputs wrt to fixtures are tested while testing http endpoints } From 43cf0ee5a7d8cd8f64cb3f1b866a2a80b335eba7 Mon Sep 17 00:00:00 2001 From: Navin Shrinivas Date: Sun, 11 Feb 2024 20:21:24 +0530 Subject: [PATCH 07/10] Reducing marshalling and Unmarshal in test and higher test coverage Signed-off-by: Navin Shrinivas --- cmd/query/app/fixture/otlp2jaeger-out.json | 60 +--------------------- cmd/query/app/http_handler_test.go | 30 +++++++---- cmd/query/app/otlp_translator_test.go | 9 ++-- 3 files changed, 27 insertions(+), 72 deletions(-) diff --git a/cmd/query/app/fixture/otlp2jaeger-out.json b/cmd/query/app/fixture/otlp2jaeger-out.json index 3041467c0d4..7272368ca71 100644 --- a/cmd/query/app/fixture/otlp2jaeger-out.json +++ b/cmd/query/app/fixture/otlp2jaeger-out.json @@ -1,59 +1 @@ -{ - "data": [ - { - "traceID": "83a9efd15c1c98a977e0711cc93ee28b", - "spans": [ - { - "traceID": "83a9efd15c1c98a977e0711cc93ee28b", - "spanID": "e127af99e3b3e074", - "operationName": "okey-dokey-0", - "references": [ - { - "refType": "CHILD_OF", - "traceID": "83a9efd15c1c98a977e0711cc93ee28b", - "spanID": "909541b92cf05311" - } - ], - "startTime": 1706678909209712, - "duration": 123, - "tags": [ - { - "key": "otel.library.name", - "type": "string", - "value": "telemetrygen" - }, - { - "key": "net.peer.ip", - "type": "string", - "value": "1.2.3.4" - }, - { - "key": "peer.service", - "type": "string", - "value": "telemetrygen-client" - }, - { - "key": "span.kind", - "type": "string", - "value": "server" - } - ], - "logs": [], - "processID": "p1", - "warnings": null - } - ], - "processes": { - "p1": { - "serviceName": "telemetrygen", - "tags": [] - } - }, - "warnings": null - } - ], - "total": 0, - "limit": 0, - "offset": 0, - "errors": null -} +{"data":[{"traceID":"83a9efd15c1c98a977e0711cc93ee28b","spans":[{"traceID":"83a9efd15c1c98a977e0711cc93ee28b","spanID":"e127af99e3b3e074","operationName":"okey-dokey-0","references":[{"refType":"CHILD_OF","traceID":"83a9efd15c1c98a977e0711cc93ee28b","spanID":"909541b92cf05311"}],"startTime":1706678909209712,"duration":123,"tags":[{"key":"otel.library.name","type":"string","value":"telemetrygen"},{"key":"net.peer.ip","type":"string","value":"1.2.3.4"},{"key":"peer.service","type":"string","value":"telemetrygen-client"},{"key":"span.kind","type":"string","value":"server"}],"logs":[],"processID":"p1","warnings":null}],"processes":{"p1":{"serviceName":"telemetrygen","tags":[]}},"warnings":null}],"total":0,"limit":0,"offset":0,"errors":null} diff --git a/cmd/query/app/http_handler_test.go b/cmd/query/app/http_handler_test.go index 0a23866e137..b0c1f9f3d89 100644 --- a/cmd/query/app/http_handler_test.go +++ b/cmd/query/app/http_handler_test.go @@ -56,6 +56,15 @@ import ( const millisToNanosMultiplier = int64(time.Millisecond / time.Nanosecond) +type IoReaderMock struct { + mock.Mock +} + +func (m *IoReaderMock) Read(b []byte) (int, error) { + args := m.Called(b) + return args.Int(0), args.Error(1) +} + var ( errStorageMsg = "storage error" errStorage = errors.New(errStorageMsg) @@ -625,24 +634,25 @@ func TestGetOperationsLegacyStorageFailure(t *testing.T) { func TestTransformOTLPSuccess(t *testing.T) { withTestServer(func(ts *testServer) { - response := new(interface{}) - request := new(interface{}) - requestBytes := readOTLPTraces(t) - err := json.Unmarshal(requestBytes, request) + resp, err := ts.server.Client().Post(ts.server.URL+"/api/transform", "application/json", bytes.NewReader(requestBytes)) require.NoError(t, err) - err = postJSON(ts.server.URL+"/api/transform", request, response) + responseBytes, err := io.ReadAll(resp.Body) require.NoError(t, err) corectResponseBytes := readJaegerTraces(t) - corectResponse := new(interface{}) - - err = json.Unmarshal(corectResponseBytes, corectResponse) - require.NoError(t, err) + assert.Equal(t, corectResponseBytes, responseBytes) + }, querysvc.QueryServiceOptions{}) +} - assert.Equal(t, response, corectResponse) +func TestTransformOTLPReadError(t *testing.T) { + withTestServer(func(ts *testServer) { + bytesReader := &IoReaderMock{} + bytesReader.On("Read", mock.AnythingOfType("[]uint8")).Return(0, errors.New("Mocked error")) + _, err := ts.server.Client().Post(ts.server.URL+"/api/transform", "application/json", bytesReader) + require.Error(t, err) }, querysvc.QueryServiceOptions{}) } diff --git a/cmd/query/app/otlp_translator_test.go b/cmd/query/app/otlp_translator_test.go index da78440f4b7..a6b32c39c3f 100644 --- a/cmd/query/app/otlp_translator_test.go +++ b/cmd/query/app/otlp_translator_test.go @@ -13,18 +13,21 @@ import ( func readOTLPTraces(t *testing.T) []byte { dat, err := os.ReadFile("./fixture/otlp2jaeger-in.json") require.NoError(t, err) - return dat + return dat[:len(dat)-1] + // As we compare as bytes in fixtures we have to trim the EOF char } func readJaegerTraces(t *testing.T) []byte { dat, err := os.ReadFile("./fixture/otlp2jaeger-out.json") require.NoError(t, err) - return dat + return dat[:len(dat)-1] + // As we compare as bytes in fixtures we have to trim the EOF char + // We also have to make sure the JSON output expected doesnt have any formatting } func TestOtlp2Traces(t *testing.T) { OTLPTraces := readOTLPTraces(t) _, err := otlp2traces(OTLPTraces) require.NoError(t, err) - // Correctness of outputs wrt to fixtures are tested while testing http endpoints + // Correctness of outputs wrt fixtures are tested while testing http endpoints } From 55a1f84d60d84a6a22b28082c52e4cd24e69c0c1 Mon Sep 17 00:00:00 2001 From: Navin Shrinivas Date: Tue, 13 Feb 2024 10:03:38 +0530 Subject: [PATCH 08/10] Making tests simpler Signed-off-by: Navin Shrinivas --- cmd/query/app/http_handler_test.go | 19 +++++++++------ cmd/query/app/otlp_translator_test.go | 33 --------------------------- 2 files changed, 12 insertions(+), 40 deletions(-) delete mode 100644 cmd/query/app/otlp_translator_test.go diff --git a/cmd/query/app/http_handler_test.go b/cmd/query/app/http_handler_test.go index b0c1f9f3d89..4dd816bbe9d 100644 --- a/cmd/query/app/http_handler_test.go +++ b/cmd/query/app/http_handler_test.go @@ -25,6 +25,7 @@ import ( "math" "net/http" "net/http/httptest" + "os" "testing" "time" @@ -634,16 +635,20 @@ func TestGetOperationsLegacyStorageFailure(t *testing.T) { func TestTransformOTLPSuccess(t *testing.T) { withTestServer(func(ts *testServer) { - requestBytes := readOTLPTraces(t) + inFile, err := os.Open("./fixture/otlp2jaeger-in.json") + require.NoError(t, err) - resp, err := ts.server.Client().Post(ts.server.URL+"/api/transform", "application/json", bytes.NewReader(requestBytes)) + resp, err := ts.server.Client().Post(ts.server.URL+"/api/transform", "application/json", inFile) require.NoError(t, err) responseBytes, err := io.ReadAll(resp.Body) require.NoError(t, err) - corectResponseBytes := readJaegerTraces(t) - assert.Equal(t, corectResponseBytes, responseBytes) + expectedBytes, err := os.ReadFile("./fixture/otlp2jaeger-out.json") + require.NoError(t, err) + + // removing EOF char as comparing bytes and not objects + assert.Equal(t, expectedBytes[:len(expectedBytes)-1], responseBytes) }, querysvc.QueryServiceOptions{}) } @@ -656,12 +661,12 @@ func TestTransformOTLPReadError(t *testing.T) { }, querysvc.QueryServiceOptions{}) } -func TestTransformOTLPEmptyFailure(t *testing.T) { +func TestTransformOTLPBadPayload(t *testing.T) { withTestServer(func(ts *testServer) { response := new(interface{}) - request := "" + request := "Bad Payload" err := postJSON(ts.server.URL+"/api/transform", request, response) - require.Error(t, err) + require.ErrorContains(t, err, "cannot unmarshal OTLP") }, querysvc.QueryServiceOptions{}) } diff --git a/cmd/query/app/otlp_translator_test.go b/cmd/query/app/otlp_translator_test.go deleted file mode 100644 index a6b32c39c3f..00000000000 --- a/cmd/query/app/otlp_translator_test.go +++ /dev/null @@ -1,33 +0,0 @@ -// Copyright (c) 2024 The Jaeger Authors. -// SPDX-License-Identifier: Apache-2.0 - -package app - -import ( - "os" - "testing" - - "github.com/stretchr/testify/require" -) - -func readOTLPTraces(t *testing.T) []byte { - dat, err := os.ReadFile("./fixture/otlp2jaeger-in.json") - require.NoError(t, err) - return dat[:len(dat)-1] - // As we compare as bytes in fixtures we have to trim the EOF char -} - -func readJaegerTraces(t *testing.T) []byte { - dat, err := os.ReadFile("./fixture/otlp2jaeger-out.json") - require.NoError(t, err) - return dat[:len(dat)-1] - // As we compare as bytes in fixtures we have to trim the EOF char - // We also have to make sure the JSON output expected doesnt have any formatting -} - -func TestOtlp2Traces(t *testing.T) { - OTLPTraces := readOTLPTraces(t) - _, err := otlp2traces(OTLPTraces) - require.NoError(t, err) - // Correctness of outputs wrt fixtures are tested while testing http endpoints -} From 6e20daffd94e5304cd1e2a86daac3d45d782c0ca Mon Sep 17 00:00:00 2001 From: Yuri Shkuro Date: Tue, 13 Feb 2024 21:05:16 -0500 Subject: [PATCH 09/10] improve tests Signed-off-by: Yuri Shkuro --- cmd/query/app/fixture/otlp2jaeger-in.json | 114 ++++++++++++--------- cmd/query/app/fixture/otlp2jaeger-out.json | 111 +++++++++++++++++++- cmd/query/app/http_handler.go | 46 +++------ cmd/query/app/http_handler_test.go | 13 ++- 4 files changed, 202 insertions(+), 82 deletions(-) diff --git a/cmd/query/app/fixture/otlp2jaeger-in.json b/cmd/query/app/fixture/otlp2jaeger-in.json index 39577361e72..585d7a29e95 100644 --- a/cmd/query/app/fixture/otlp2jaeger-in.json +++ b/cmd/query/app/fixture/otlp2jaeger-in.json @@ -1,52 +1,74 @@ { - "resourceSpans":[ - { - "resource":{ - "attributes":[ - { - "key":"service.name", - "value":{ - "stringValue":"telemetrygen" + "resourceSpans": [ + { + "resource": { + "attributes": [ + { + "key": "service.name", + "value": { + "stringValue": "telemetrygen" + } + } + ] + }, + "scopeSpans": [ + { + "scope": { + "name": "telemetrygen" + }, + "spans": [ + { + "traceId": "83a9efd15c1c98a977e0711cc93ee28b", + "spanId": "e127af99e3b3e074", + "parentSpanId": "909541b92cf05311", + "name": "okey-dokey-0", + "kind": 2, + "startTimeUnixNano": "1706678909209712000", + "endTimeUnixNano": "1706678909209835000", + "attributes": [ + { + "key": "net.peer.ip", + "value": { + "stringValue": "1.2.3.4" } - } - ] - }, - "scopeSpans":[ + }, + { + "key": "peer.service", + "value": { + "stringValue": "telemetrygen-client" + } + } + ], + "status": {} + }, { - "scope":{ - "name":"telemetrygen" - }, - "spans":[ - { - "traceId":"83a9efd15c1c98a977e0711cc93ee28b", - "spanId":"e127af99e3b3e074", - "parentSpanId":"909541b92cf05311", - "name":"okey-dokey-0", - "kind":2, - "startTimeUnixNano":"1706678909209712000", - "endTimeUnixNano":"1706678909209835000", - "attributes":[ - { - "key":"net.peer.ip", - "value":{ - "stringValue":"1.2.3.4" - } - }, - { - "key":"peer.service", - "value":{ - "stringValue":"telemetrygen-client" - } - } - ], - "status":{ - - } + "traceId": "000083a9efd15c1c98a977e0711cc93e", + "spanId": "e127af99e3b3e074", + "parentSpanId": "909541b92cf05311", + "name": "okey-dokey-0", + "kind": 2, + "startTimeUnixNano": "1706678909209712000", + "endTimeUnixNano": "1706678909209835000", + "attributes": [ + { + "key": "net.peer.ip", + "value": { + "stringValue": "1.2.3.4" + } + }, + { + "key": "peer.service", + "value": { + "stringValue": "telemetrygen-client" } - ] + } + ], + "status": {} } - ], - "schemaUrl":"https://opentelemetry.io/schemas/1.4.0" - } - ] + ] + } + ], + "schemaUrl": "https://opentelemetry.io/schemas/1.4.0" + } + ] } diff --git a/cmd/query/app/fixture/otlp2jaeger-out.json b/cmd/query/app/fixture/otlp2jaeger-out.json index 7272368ca71..d6784ff35ba 100644 --- a/cmd/query/app/fixture/otlp2jaeger-out.json +++ b/cmd/query/app/fixture/otlp2jaeger-out.json @@ -1 +1,110 @@ -{"data":[{"traceID":"83a9efd15c1c98a977e0711cc93ee28b","spans":[{"traceID":"83a9efd15c1c98a977e0711cc93ee28b","spanID":"e127af99e3b3e074","operationName":"okey-dokey-0","references":[{"refType":"CHILD_OF","traceID":"83a9efd15c1c98a977e0711cc93ee28b","spanID":"909541b92cf05311"}],"startTime":1706678909209712,"duration":123,"tags":[{"key":"otel.library.name","type":"string","value":"telemetrygen"},{"key":"net.peer.ip","type":"string","value":"1.2.3.4"},{"key":"peer.service","type":"string","value":"telemetrygen-client"},{"key":"span.kind","type":"string","value":"server"}],"logs":[],"processID":"p1","warnings":null}],"processes":{"p1":{"serviceName":"telemetrygen","tags":[]}},"warnings":null}],"total":0,"limit":0,"offset":0,"errors":null} +{ + "data": [ + { + "traceID": "83a9efd15c1c98a977e0711cc93ee28b", + "spans": [ + { + "traceID": "83a9efd15c1c98a977e0711cc93ee28b", + "spanID": "e127af99e3b3e074", + "operationName": "okey-dokey-0", + "references": [ + { + "refType": "CHILD_OF", + "traceID": "83a9efd15c1c98a977e0711cc93ee28b", + "spanID": "909541b92cf05311" + } + ], + "startTime": 1706678909209712, + "duration": 123, + "tags": [ + { + "key": "otel.library.name", + "type": "string", + "value": "telemetrygen" + }, + { + "key": "net.peer.ip", + "type": "string", + "value": "1.2.3.4" + }, + { + "key": "peer.service", + "type": "string", + "value": "telemetrygen-client" + }, + { + "key": "span.kind", + "type": "string", + "value": "server" + } + ], + "logs": [], + "processID": "p1", + "warnings": null + } + ], + "processes": { + "p1": { + "serviceName": "telemetrygen", + "tags": [] + } + }, + "warnings": null + }, + { + "traceID": "000083a9efd15c1c98a977e0711cc93e", + "spans": [ + { + "traceID": "000083a9efd15c1c98a977e0711cc93e", + "spanID": "e127af99e3b3e074", + "operationName": "okey-dokey-0", + "references": [ + { + "refType": "CHILD_OF", + "traceID": "000083a9efd15c1c98a977e0711cc93e", + "spanID": "909541b92cf05311" + } + ], + "startTime": 1706678909209712, + "duration": 123, + "tags": [ + { + "key": "otel.library.name", + "type": "string", + "value": "telemetrygen" + }, + { + "key": "net.peer.ip", + "type": "string", + "value": "1.2.3.4" + }, + { + "key": "peer.service", + "type": "string", + "value": "telemetrygen-client" + }, + { + "key": "span.kind", + "type": "string", + "value": "server" + } + ], + "logs": [], + "processID": "p1", + "warnings": null + } + ], + "processes": { + "p1": { + "serviceName": "telemetrygen", + "tags": [] + } + }, + "warnings": null + } + ], + "total": 0, + "limit": 0, + "offset": 0, + "errors": null +} diff --git a/cmd/query/app/http_handler.go b/cmd/query/app/http_handler.go index a7600c0e5e9..09d647e90ed 100644 --- a/cmd/query/app/http_handler.go +++ b/cmd/query/app/http_handler.go @@ -201,27 +201,13 @@ func (aH *APIHandler) transformOTLP(w http.ResponseWriter, r *http.Request) { return } - var uiErrors []structuredError traces, err := otlp2traces(body) - if aH.handleError(w, err, http.StatusInternalServerError) { return } - uiTraces := make([]*ui.Trace, len(traces)) - - for i, v := range traces { - uiTrace, uiErr := aH.convertModelToUI(v, false) - if uiErr != nil { - uiErrors = append(uiErrors, *uiErr) - } - uiTraces[i] = uiTrace - } - - structuredRes := structuredResponse{ - Data: uiTraces, - Errors: uiErrors, - } + var uiErrors []structuredError + structuredRes := aH.tracesToResponse(traces, false, uiErrors) aH.writeJSON(w, r, structuredRes) } @@ -275,20 +261,24 @@ func (aH *APIHandler) search(w http.ResponseWriter, r *http.Request) { } } - uiTraces := make([]*ui.Trace, len(tracesFromStorage)) - for i, v := range tracesFromStorage { - uiTrace, uiErr := aH.convertModelToUI(v, true) + structuredRes := aH.tracesToResponse(tracesFromStorage, true, uiErrors) + aH.writeJSON(w, r, structuredRes) +} + +func (aH *APIHandler) tracesToResponse(traces []*model.Trace, adjust bool, uiErrors []structuredError) *structuredResponse { + uiTraces := make([]*ui.Trace, len(traces)) + for i, v := range traces { + uiTrace, uiErr := aH.convertModelToUI(v, adjust) if uiErr != nil { uiErrors = append(uiErrors, *uiErr) } uiTraces[i] = uiTrace } - structuredRes := structuredResponse{ + return &structuredResponse{ Data: uiTraces, Errors: uiErrors, } - aH.writeJSON(w, r, &structuredRes) } func (aH *APIHandler) tracesByIDs(ctx context.Context, traceIDs []model.TraceID) ([]*model.Trace, []structuredError, error) { @@ -468,18 +458,8 @@ func (aH *APIHandler) getTrace(w http.ResponseWriter, r *http.Request) { } var uiErrors []structuredError - uiTrace, uiErr := aH.convertModelToUI(trace, shouldAdjust(r)) - if uiErr != nil { - uiErrors = append(uiErrors, *uiErr) - } - - structuredRes := structuredResponse{ - Data: []*ui.Trace{ - uiTrace, - }, - Errors: uiErrors, - } - aH.writeJSON(w, r, &structuredRes) + structuredRes := aH.tracesToResponse([]*model.Trace{trace}, shouldAdjust(r), uiErrors) + aH.writeJSON(w, r, structuredRes) } func shouldAdjust(r *http.Request) bool { diff --git a/cmd/query/app/http_handler_test.go b/cmd/query/app/http_handler_test.go index 4dd816bbe9d..f49ac6783cc 100644 --- a/cmd/query/app/http_handler_test.go +++ b/cmd/query/app/http_handler_test.go @@ -634,6 +634,14 @@ func TestGetOperationsLegacyStorageFailure(t *testing.T) { } func TestTransformOTLPSuccess(t *testing.T) { + reformat := func(in []byte) []byte { + obj := new(interface{}) + require.NoError(t, json.Unmarshal(in, obj)) + // format json similar to `jq .` + out, err := json.MarshalIndent(obj, "", " ") + require.NoError(t, err) + return out + } withTestServer(func(ts *testServer) { inFile, err := os.Open("./fixture/otlp2jaeger-in.json") require.NoError(t, err) @@ -643,12 +651,13 @@ func TestTransformOTLPSuccess(t *testing.T) { responseBytes, err := io.ReadAll(resp.Body) require.NoError(t, err) + responseBytes = reformat(responseBytes) expectedBytes, err := os.ReadFile("./fixture/otlp2jaeger-out.json") require.NoError(t, err) + expectedBytes = reformat(expectedBytes) - // removing EOF char as comparing bytes and not objects - assert.Equal(t, expectedBytes[:len(expectedBytes)-1], responseBytes) + assert.Equal(t, string(expectedBytes), string(responseBytes)) }, querysvc.QueryServiceOptions{}) } From 463556ce93c34f91fa5d0dd0d354d324b4b95c36 Mon Sep 17 00:00:00 2001 From: Yuri Shkuro Date: Tue, 13 Feb 2024 21:26:38 -0500 Subject: [PATCH 10/10] use same trace id, two spans Signed-off-by: Yuri Shkuro --- cmd/query/app/fixture/otlp2jaeger-in.json | 2 +- cmd/query/app/fixture/otlp2jaeger-out.json | 18 +++--------------- 2 files changed, 4 insertions(+), 16 deletions(-) diff --git a/cmd/query/app/fixture/otlp2jaeger-in.json b/cmd/query/app/fixture/otlp2jaeger-in.json index 585d7a29e95..1a62a0a1e87 100644 --- a/cmd/query/app/fixture/otlp2jaeger-in.json +++ b/cmd/query/app/fixture/otlp2jaeger-in.json @@ -42,7 +42,7 @@ "status": {} }, { - "traceId": "000083a9efd15c1c98a977e0711cc93e", + "traceId": "83a9efd15c1c98a977e0711cc93ee28b", "spanId": "e127af99e3b3e074", "parentSpanId": "909541b92cf05311", "name": "okey-dokey-0", diff --git a/cmd/query/app/fixture/otlp2jaeger-out.json b/cmd/query/app/fixture/otlp2jaeger-out.json index d6784ff35ba..313659814b3 100644 --- a/cmd/query/app/fixture/otlp2jaeger-out.json +++ b/cmd/query/app/fixture/otlp2jaeger-out.json @@ -41,27 +41,15 @@ "logs": [], "processID": "p1", "warnings": null - } - ], - "processes": { - "p1": { - "serviceName": "telemetrygen", - "tags": [] - } - }, - "warnings": null - }, - { - "traceID": "000083a9efd15c1c98a977e0711cc93e", - "spans": [ + }, { - "traceID": "000083a9efd15c1c98a977e0711cc93e", + "traceID": "83a9efd15c1c98a977e0711cc93ee28b", "spanID": "e127af99e3b3e074", "operationName": "okey-dokey-0", "references": [ { "refType": "CHILD_OF", - "traceID": "000083a9efd15c1c98a977e0711cc93e", + "traceID": "83a9efd15c1c98a977e0711cc93ee28b", "spanID": "909541b92cf05311" } ],