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.

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

Add gogo/protobuf to OpenTelemetry OTLP data model #5067

Merged
merged 15 commits into from
Jan 4, 2024
12 changes: 8 additions & 4 deletions cmd/query/app/apiv3/gateway_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,10 @@ func makeTestTrace() (*model.Trace, model.TraceID) {
TraceID: traceID,
SpanID: model.NewSpanID(180),
OperationName: "foobar",
Tags: []model.KeyValue{
model.String("span.kind", "server"),
model.Bool("error", true),
},
},
},
}, traceID
Expand Down Expand Up @@ -160,9 +164,9 @@ func (gw *testGateway) runGatewayGetTrace(t *testing.T) {
parseResponse(t, body, &response)

assert.Len(t, response.Result.ResourceSpans, 1)
assert.Equal(t,
assert.EqualValues(t,
bytesOfTraceID(t, traceID.High, traceID.Low),
response.Result.ResourceSpans[0].ScopeSpans[0].Spans[0].TraceId)
response.Result.ResourceSpans[0].ScopeSpans[0].Spans[0].TraceID)
}

func (gw *testGateway) runGatewayFindTraces(t *testing.T) {
Expand All @@ -179,9 +183,9 @@ func (gw *testGateway) runGatewayFindTraces(t *testing.T) {
parseResponse(t, body, &response)

assert.Len(t, response.Result.ResourceSpans, 1)
assert.Equal(t,
assert.EqualValues(t,
bytesOfTraceID(t, traceID.High, traceID.Low),
response.Result.ResourceSpans[0].ScopeSpans[0].Spans[0].TraceId)
response.Result.ResourceSpans[0].ScopeSpans[0].Spans[0].TraceID)
}

func bytesOfTraceID(t *testing.T, high, low uint64) []byte {
Expand Down
11 changes: 0 additions & 11 deletions cmd/query/app/apiv3/http_gateway.go
Original file line number Diff line number Diff line change
Expand Up @@ -124,17 +124,6 @@ func (h *HTTPGateway) returnSpansTestable(
if h.tryHandleError(w, err, http.StatusInternalServerError) {
return
}
for _, rs := range resourceSpans {
for _, ss := range rs.ScopeSpans {
for _, s := range ss.Spans {
if len(s.ParentSpanId) == 0 {
// If ParentSpanId is empty array then gogo/jsonpb renders it as empty string.
// To match the output with grpc-gateway we set it to nil and it won't be included.
s.ParentSpanId = nil
Copy link
Member Author

Choose a reason for hiding this comment

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

field is not nullable so this fix is no longer possible

}
}
}
}
response := &api_v3.GRPCGatewayWrapper{
Result: &api_v3.SpansResponseChunk{
ResourceSpans: resourceSpans,
Expand Down
6 changes: 5 additions & 1 deletion cmd/query/app/apiv3/snapshots/FindTraces.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,14 @@
"spans": [
{
"endTimeUnixNano": "11651379494838206464",
"kind": "SPAN_KIND_SERVER",
"name": "foobar",
"parentSpanId": "",
Copy link
Member Author

Choose a reason for hiding this comment

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

Could not find a way to suppress this empty field. In practice not a big deal all but one span in a trace will have non-empty parent. Also, verified that OTLP-JSON also includes empty parent ID.

"spanId": "AAAAAAAAALQ=",
"startTimeUnixNano": "11651379494838206464",
"status": {},
"status": {
"code": "STATUS_CODE_ERROR"
},
Copy link
Member Author

Choose a reason for hiding this comment

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

Added this and span kind as examples of enums. This matches the previous output when using grpc-gateway marshaling (and is different from OTLP-JSON encoding).

"traceId": "AAAAAAAAAJYAAAAAAAAAoA=="
}
]
Expand Down
6 changes: 5 additions & 1 deletion cmd/query/app/apiv3/snapshots/GetTrace.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,14 @@
"spans": [
{
"endTimeUnixNano": "11651379494838206464",
"kind": "SPAN_KIND_SERVER",
"name": "foobar",
"parentSpanId": "",
"spanId": "AAAAAAAAALQ=",
"startTimeUnixNano": "11651379494838206464",
"status": {},
"status": {
"code": "STATUS_CODE_ERROR"
},
"traceId": "AAAAAAAAAJYAAAAAAAAAoA=="
}
]
Expand Down
19 changes: 19 additions & 0 deletions model/v2/customtype.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
// Copyright (c) 2024 The Jaeger Authors.
// SPDX-License-Identifier: Apache-2.0

package model

import "github.com/gogo/protobuf/proto"

// gogoCustom is an interface that Gogo expects custom types to implement.
// https://github.com/gogo/protobuf/blob/master/custom_types.md
type gogoCustom interface {
Marshal() ([]byte, error)
MarshalTo(data []byte) (n int, err error)
Unmarshal(data []byte) error

proto.Sizer

MarshalJSON() ([]byte, error)
UnmarshalJSON(data []byte) error
}
45 changes: 45 additions & 0 deletions model/v2/jsonid.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
// Copyright (c) 2023 The Jaeger Authors.
// Copyright The OpenTelemetry Authors
Copy link
Member Author

Choose a reason for hiding this comment

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

Copied from OTEL, but had to change to marshal IDs as base64, to keep with the original grpc-gateway version.

// SPDX-License-Identifier: Apache-2.0

package model

import (
"encoding/base64"
"fmt"
)

// marshalJSON converts trace id into a base64 string enclosed in quotes.
// Called by Protobuf JSON deserialization.
func marshalJSON(id []byte) ([]byte, error) {
// Plus 2 quote chars at the start and end.
hexLen := base64.StdEncoding.EncodedLen(len(id)) + 2

b := make([]byte, hexLen)
base64.StdEncoding.Encode(b[1:hexLen-1], id)
b[0], b[hexLen-1] = '"', '"'

return b, nil
}

// unmarshalJSON inflates trace id from base64 string, possibly enclosed in quotes.
// Called by Protobuf JSON deserialization.
func unmarshalJSON(dst []byte, src []byte) error {
if l := len(src); l >= 2 && src[0] == '"' && src[l-1] == '"' {
src = src[1 : l-1]
}
nLen := len(src)
if nLen == 0 {
return nil
}

if l := base64.StdEncoding.DecodedLen(nLen); len(dst) != l {
return fmt.Errorf("invalid length for ID '%s', dst=%d, src=%d", string(src), len(dst), l)
}

_, err := base64.StdEncoding.Decode(dst, src)
if err != nil {
return fmt.Errorf("cannot unmarshal ID from string '%s': %w", string(src), err)
}
return nil
}
158 changes: 158 additions & 0 deletions model/v2/jsonid_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,158 @@
// Copyright (c) 2024 The Jaeger Authors.
// SPDX-License-Identifier: Apache-2.0

package model

import (
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

func TestUnmarshalJSON(t *testing.T) {
tests := []struct {
name string
dst []byte
src []byte
expErr string
}{
{
name: "Valid input",
dst: make([]byte, 16+2),
src: []byte(`"AAAAAAAAAJYAAAAAAAAAoA=="`),
expErr: "",
},
{
name: "Empty input",
dst: make([]byte, 16),
src: []byte(`""`),
expErr: "",
},
{
name: "Invalid length",
dst: make([]byte, 16),
src: []byte(`"AAAAAAAAAJYAAAAAAAAAoA=="`),
expErr: "invalid length for ID",
},
{
name: "Decode error",
dst: make([]byte, 16+2),
src: []byte(`"invalid_base64_length_18"`),
expErr: "cannot unmarshal ID from string",
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
err := unmarshalJSON(tt.dst, tt.src)
if tt.expErr == "" {
require.NoError(t, err)
} else {
require.ErrorContains(t, err, tt.expErr)
}
})
}
}

func TestMarshal(t *testing.T) {
validSpanID := SpanID{1, 2, 3, 4, 5, 6, 7, 8}
validTraceID := TraceID{1, 2, 3, 4, 5, 6, 7, 8, 8, 7, 6, 5, 4, 3, 2, 1}
tests := []struct {
name string
id gogoCustom
expErr string
}{
{
name: "Valid span id",
id: &validSpanID,
},
{
name: "Valid trace id",
id: &validTraceID,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
d, err := tt.id.Marshal()
require.NoError(t, err)
assert.Len(t, d, tt.id.Size())
})
}
}

func TestMarshalTo(t *testing.T) {
validSpanID := SpanID{1, 2, 3, 4, 5, 6, 7, 8}
validTraceID := TraceID{1, 2, 3, 4, 5, 6, 7, 8, 8, 7, 6, 5, 4, 3, 2, 1}
tests := []struct {
name string
id gogoCustom
len int
expErr string
}{
{
name: "Valid span id buffer",
id: &validSpanID,
len: 8,
},
{
name: "Valid trace id buffer",
id: &validTraceID,
len: 16,
},
{
name: "Invalid span id buffer",
id: &validSpanID,
len: 4,
expErr: errMarshalSpanID.Error(),
},
{
name: "Invalid trace id buffer",
id: &validTraceID,
len: 4,
expErr: errMarshalTraceID.Error(),
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
buf := make([]byte, tt.len)
n, err := tt.id.MarshalTo(buf)
if tt.expErr == "" {
require.NoError(t, err)
assert.Equal(t, n, tt.id.Size())
} else {
require.ErrorContains(t, err, tt.expErr)
}
})
}
}

func TestUnmarshalError(t *testing.T) {
validSpanID := SpanID{1, 2, 3, 4, 5, 6, 7, 8}
validTraceID := TraceID{1, 2, 3, 4, 5, 6, 7, 8, 8, 7, 6, 5, 4, 3, 2, 1}
tests := []struct {
name string
id gogoCustom
}{
{
name: "span id",
id: &validSpanID,
},
{
name: "trace id",
id: &validTraceID,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Run("Protobuf", func(t *testing.T) {
err := tt.id.Unmarshal([]byte("invalid"))
require.ErrorContains(t, err, "length")
})
t.Run("JSON", func(t *testing.T) {
err := tt.id.UnmarshalJSON([]byte("invalid"))
require.ErrorContains(t, err, "length")
})
})
}
}
53 changes: 53 additions & 0 deletions model/v2/marshal_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
// Copyright (c) 2024 The Jaeger Authors.
// SPDX-License-Identifier: Apache-2.0

package model_test

import (
"bytes"
"testing"

"github.com/gogo/protobuf/jsonpb"
"github.com/gogo/protobuf/proto"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"github.com/jaegertracing/jaeger/model/v2"
tracev1 "github.com/jaegertracing/jaeger/proto-gen/otel/trace/v1"
)

// TestMarshal ensures that we can marshal and unmarshal a Span using both JSON and Protobuf.
// Since it depends on proto-gen types, it's in the model_test package to avoid circular dependencies.
func TestMarshalSpan(t *testing.T) {
tests := []struct {
name string
span *tracev1.Span
}{
{name: "valid IDs", span: &tracev1.Span{
TraceID: model.TraceID{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16},
SpanID: model.SpanID{1, 2, 3, 4, 5, 6, 7, 8},
}},
{name: "invalid IDs", span: &tracev1.Span{
TraceID: model.TraceID{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0},
SpanID: model.SpanID{0, 0, 0, 0, 0, 0, 0, 0},
}},
}
for _, tt := range tests {
t.Run(tt.name+"/Protobuf", func(t *testing.T) {
data, err := proto.Marshal(tt.span)
require.NoError(t, err)

var span tracev1.Span
require.NoError(t, proto.Unmarshal(data, &span))
assert.Equal(t, tt.span, &span)
})
t.Run(tt.name+"/JSON", func(t *testing.T) {
var buf bytes.Buffer
require.NoError(t, new(jsonpb.Marshaler).Marshal(&buf, tt.span))

var span tracev1.Span
require.NoError(t, jsonpb.Unmarshal(&buf, &span))
assert.Equal(t, tt.span, &span)
})
}
}
14 changes: 14 additions & 0 deletions model/v2/package_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
// Copyright (c) 2023 The Jaeger Authors.
// SPDX-License-Identifier: Apache-2.0

package model

import (
"testing"

"go.uber.org/goleak"
)

func TestMain(m *testing.M) {
goleak.VerifyTestMain(m)
}
Loading
Loading