Additional model/ cleanup (#6610)

## Which problem is this PR solving? - Part of #6494 ## Description of the changes - Move OTEL-IDs helpers to v1adapter - Clean-up model aliases - Remove unnecessary test in model (it exists in idl/model/v1) - Prohibit dependencies on `github.com/jaegertracing/jaeger/model` via linter - Fix `storage.proto` code-gen to not rely on `model/` ## How was this change tested? - CI --------- Signed-off-by: Yuri Shkuro <[email protected]>
jaegertracing · Jan 25, 2025 · 7168853 · 7168853
1 parent d7ab0f8
commit 7168853
Show file tree

Hide file tree

Showing 19 changed files with 75 additions and 106 deletions.
diff --git a/.golangci.yml b/.golangci.yml
@@ -123,6 +123,8 @@ linters-settings:
             desc: "Use errors.Join instead of github.com/hashicorp/go-multierror"
           - pkg: go.uber.org/multierr
             desc: "Use errors.Join instead of github.com/hashicorp/go-multierror"
+          - pkg: "github.com/jaegertracing/jaeger/model$"
+            desc: "Use github.com/jaegertracing/jaeger-idl/model/v1"
       # crossdock-go provides assert/require similar to stretchr/testify
       # but we never want to use them outside of the crossdock tests.
       disallow-crossdock:

diff --git a/Makefile.Protobuf.mk b/Makefile.Protobuf.mk
@@ -33,7 +33,7 @@ PROTO_GOGO_MAPPINGS := $(shell echo \
 		Mgoogle/protobuf/duration.proto=github.com/gogo/protobuf/types \
 		Mgoogle/protobuf/empty.proto=github.com/gogo/protobuf/types \
 		Mgoogle/api/annotations.proto=github.com/gogo/googleapis/google/api \
-		Mmodel.proto=github.com/jaegertracing/jaeger/model \
+		Mmodel.proto=github.com/jaegertracing/jaeger-idl/model/v1 \
 	| $(SED) 's/  */,/g')
 
 OPENMETRICS_PROTO_FILES=$(wildcard model/proto/metrics/*.proto)

diff --git a/cmd/jaeger/internal/integration/trace_reader.go b/cmd/jaeger/internal/integration/trace_reader.go
@@ -19,11 +19,11 @@ import (
 	"google.golang.org/grpc/status"
 
 	"github.com/jaegertracing/jaeger/internal/proto/api_v3"
-	OTELModel "github.com/jaegertracing/jaeger/model"
 	"github.com/jaegertracing/jaeger/pkg/iter"
 	"github.com/jaegertracing/jaeger/ports"
 	"github.com/jaegertracing/jaeger/storage/spanstore"
 	"github.com/jaegertracing/jaeger/storage_v2/tracestore"
+	"github.com/jaegertracing/jaeger/storage_v2/v1adapter"
 )
 
 var (
@@ -68,7 +68,7 @@ func (r *traceReader) GetTraces(
 	return func(yield func([]ptrace.Traces, error) bool) {
 		// api_v3 does not support multi-get, so loop through IDs
 		for _, idParams := range traceIDs {
-			idStr := OTELModel.TraceIDFromOTEL(idParams.TraceID).String()
+			idStr := v1adapter.ToV1TraceID(idParams.TraceID).String()
 			r.logger.Info("Calling api_v3.GetTrace", zap.String("trace_id", idStr))
 			stream, err := r.client.GetTrace(ctx, &api_v3.GetTraceRequest{
 				TraceId:   idStr,

diff --git a/cmd/query/app/apiv3/grpc_handler.go b/cmd/query/app/apiv3/grpc_handler.go
@@ -15,9 +15,9 @@ import (
 	"github.com/jaegertracing/jaeger-idl/model/v1"
 	"github.com/jaegertracing/jaeger/cmd/query/app/querysvc/v2/querysvc"
 	"github.com/jaegertracing/jaeger/internal/proto/api_v3"
-	OTELModel "github.com/jaegertracing/jaeger/model"
 	"github.com/jaegertracing/jaeger/pkg/iter"
 	"github.com/jaegertracing/jaeger/storage_v2/tracestore"
+	"github.com/jaegertracing/jaeger/storage_v2/v1adapter"
 )
 
 // Handler implements api_v3.QueryServiceServer
@@ -38,7 +38,7 @@ func (h *Handler) GetTrace(request *api_v3.GetTraceRequest, stream api_v3.QueryS
 	query := querysvc.GetTraceParams{
 		TraceIDs: []tracestore.GetTraceParams{
 			{
-				TraceID: OTELModel.ToOTELTraceID(traceID),
+				TraceID: v1adapter.FromV1TraceID(traceID),
 				Start:   request.GetStartTime(),
 				End:     request.GetEndTime(),
 			},

diff --git a/cmd/query/app/apiv3/http_gateway.go b/cmd/query/app/apiv3/http_gateway.go
@@ -23,10 +23,10 @@ import (
 	"github.com/jaegertracing/jaeger-idl/model/v1"
 	"github.com/jaegertracing/jaeger/cmd/query/app/querysvc/v2/querysvc"
 	"github.com/jaegertracing/jaeger/internal/proto/api_v3"
-	OTELModel "github.com/jaegertracing/jaeger/model"
 	"github.com/jaegertracing/jaeger/pkg/iter"
 	"github.com/jaegertracing/jaeger/storage/spanstore"
 	"github.com/jaegertracing/jaeger/storage_v2/tracestore"
+	"github.com/jaegertracing/jaeger/storage_v2/v1adapter"
 )
 
 const (
@@ -162,7 +162,7 @@ func (h *HTTPGateway) getTrace(w http.ResponseWriter, r *http.Request) {
 	request := querysvc.GetTraceParams{
 		TraceIDs: []tracestore.GetTraceParams{
 			{
-				TraceID: OTELModel.ToOTELTraceID(traceID),
+				TraceID: v1adapter.FromV1TraceID(traceID),
 			},
 		},
 	}

diff --git a/model/ids.go b/model/ids.go
@@ -8,11 +8,6 @@ import (
 	modelv1 "github.com/jaegertracing/jaeger-idl/model/v1"
 )
 
-const (
-	// traceIDShortBytesLen indicates length of 64bit traceID when represented as list of bytes
-	traceIDShortBytesLen = 8
-)
-
 // TraceID is a random 128bit identifier for a trace
 type TraceID = modelv1.TraceID
 

diff --git a/model/keyvalue.go b/model/keyvalue.go
@@ -10,16 +10,11 @@ import (
 
 // These constants are kept mostly for backwards compatibility.
 const (
-	// StringType indicates the value is a unicode string
-	StringType = ValueType_STRING
-	// BoolType indicates the value is a Boolean encoded as int64 number 0 or 1
-	BoolType = ValueType_BOOL
-	// Int64Type indicates the value is an int64 number
-	Int64Type = ValueType_INT64
-	// Float64Type indicates the value is a float64 number stored as int64
-	Float64Type = ValueType_FLOAT64
-	// BinaryType indicates the value is binary blob stored as a byte array
-	BinaryType = ValueType_BINARY
+	StringType  = modelv1.StringType
+	BoolType    = modelv1.BoolType
+	Int64Type   = modelv1.Int64Type
+	Float64Type = modelv1.Float64Type
+	BinaryType  = modelv1.BinaryType
 
 	SpanKindKey     = modelv1.SpanKindKey
 	SamplerTypeKey  = modelv1.SamplerTypeKey

diff --git a/model/span.go b/model/span.go
@@ -18,12 +18,4 @@ const (
 	SamplerTypeConst                     = modelv1.SamplerTypeConst
 )
 
-var toSamplerType = map[string]SamplerType{
-	"unrecognized":  SamplerTypeUnrecognized,
-	"probabilistic": SamplerTypeProbabilistic,
-	"lowerbound":    SamplerTypeLowerBound,
-	"ratelimiting":  SamplerTypeRateLimiting,
-	"const":         SamplerTypeConst,
-}
-
 var SpanKindTag = modelv1.SpanKindTag
diff --git a/model/span_pkg_test.go b/model/span_pkg_test.go
diff --git a/model/spanref.go b/model/spanref.go
@@ -11,11 +11,11 @@ import (
 const (
 	// ChildOf span reference type describes a reference to a parent span
 	// that depends on the response from the current (child) span
-	ChildOf = SpanRefType_CHILD_OF
+	ChildOf = modelv1.ChildOf
 
 	// FollowsFrom span reference type describes a reference to a "parent" span
 	// that does not depend on the response from the current (child) span
-	FollowsFrom = SpanRefType_FOLLOWS_FROM
+	FollowsFrom = modelv1.FollowsFrom
 )
 
 // MaybeAddParentSpanID adds non-zero parentSpanID to refs as a child-of reference.

diff --git a/plugin/storage/integration/integration.go b/plugin/storage/integration/integration.go
@@ -23,7 +23,6 @@ import (
 	"github.com/stretchr/testify/require"
 
 	"github.com/jaegertracing/jaeger-idl/model/v1"
-	OTELModel "github.com/jaegertracing/jaeger/model"
 	"github.com/jaegertracing/jaeger/storage/samplingstore"
 	samplemodel "github.com/jaegertracing/jaeger/storage/samplingstore/model"
 	"github.com/jaegertracing/jaeger/storage/spanstore"
@@ -218,7 +217,7 @@ func (s *StorageIntegration) testGetLargeSpan(t *testing.T) {
 	t.Log("Testing Large Trace over 10K with duplicate IDs...")
 
 	expected := s.writeLargeTraceWithDuplicateSpanIds(t)
-	expectedTraceID := OTELModel.ToOTELTraceID(expected.Spans[0].TraceID)
+	expectedTraceID := v1adapter.FromV1TraceID(expected.Spans[0].TraceID)
 
 	actual := &model.Trace{} // no spans
 	found := s.waitForCondition(t, func(_ *testing.T) bool {
@@ -294,7 +293,7 @@ func (s *StorageIntegration) testGetTrace(t *testing.T) {
 	defer s.cleanUp(t)
 
 	expected := s.loadParseAndWriteExampleTrace(t)
-	expectedTraceID := OTELModel.ToOTELTraceID(expected.Spans[0].TraceID)
+	expectedTraceID := v1adapter.FromV1TraceID(expected.Spans[0].TraceID)
 
 	actual := &model.Trace{} // no spans
 	found := s.waitForCondition(t, func(t *testing.T) bool {
@@ -314,7 +313,7 @@ func (s *StorageIntegration) testGetTrace(t *testing.T) {
 	}
 
 	t.Run("NotFound error", func(t *testing.T) {
-		fakeTraceID := OTELModel.ToOTELTraceID(model.TraceID{High: 0, Low: 1})
+		fakeTraceID := v1adapter.FromV1TraceID(model.TraceID{High: 0, Low: 1})
 		iterTraces := s.TraceReader.GetTraces(context.Background(), tracestore.GetTraceParams{TraceID: fakeTraceID})
 		traces, err := v1adapter.V1TracesFromSeq2(iterTraces)
 		require.NoError(t, err) // v2 TraceReader no longer returns an error for not found

diff --git a/proto-gen/storage_v1/storage.pb.go b/proto-gen/storage_v1/storage.pb.go
diff --git a/scripts/lint/check-go-version.sh b/scripts/lint/check-go-version.sh
@@ -88,6 +88,9 @@ check .golangci.yml "go:\s\+\"$version_regex\"" "$go_previous_version"
 
 # find all other go.mod files in the repository and check for latest Go version
 for file in $(find . -type f -name go.mod | grep -v '^./go.mod'); do
+    if [[ $file == "./idl/go.mod" ]]; then
+        continue
+    fi
     check "$file" "^go\s\+$version_regex" "$go_latest_version"
 done
 

diff --git a/model/otelids.go → storage_v2/v1adapter/otelids.go b/model/otelids.go → storage_v2/v1adapter/otelids.go
@@ -2,43 +2,48 @@
 // Copyright (c) 2018 Uber Technologies, Inc.
 // SPDX-License-Identifier: Apache-2.0
 
-package model
+package v1adapter
 
 import (
 	"encoding/binary"
 
 	"go.opentelemetry.io/collector/pdata/pcommon"
+
+	"github.com/jaegertracing/jaeger-idl/model/v1"
 )
 
-// ToOTELTraceID converts the TraceID to OTEL's representation of a trace identitfier.
+// FromV1TraceID converts the TraceID to OTEL's representation of a trace identitfier.
 // This was taken from
 // https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/main/internal/coreinternal/idutils/big_endian_converter.go.
-func ToOTELTraceID(t TraceID) pcommon.TraceID {
+func FromV1TraceID(t model.TraceID) pcommon.TraceID {
 	traceID := [16]byte{}
 	binary.BigEndian.PutUint64(traceID[:8], t.High)
 	binary.BigEndian.PutUint64(traceID[8:], t.Low)
 	return traceID
 }
 
-func TraceIDFromOTEL(traceID pcommon.TraceID) TraceID {
-	return TraceID{
+func ToV1TraceID(traceID pcommon.TraceID) model.TraceID {
+	// traceIDShortBytesLen indicates length of 64bit traceID when represented as list of bytes
+	const traceIDShortBytesLen = 8
+
+	return model.TraceID{
 		High: binary.BigEndian.Uint64(traceID[:traceIDShortBytesLen]),
 		Low:  binary.BigEndian.Uint64(traceID[traceIDShortBytesLen:]),
 	}
 }
 
-// ToOTELSpanID converts the SpanID to OTEL's representation of a span identitfier.
+// FromV1SpanID converts the SpanID to OTEL's representation of a span identitfier.
 // This was taken from
 // https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/main/internal/coreinternal/idutils/big_endian_converter.go.
-func ToOTELSpanID(s SpanID) pcommon.SpanID {
+func FromV1SpanID(s model.SpanID) pcommon.SpanID {
 	spanID := [8]byte{}
 	binary.BigEndian.PutUint64(spanID[:], uint64(s))
 	return pcommon.SpanID(spanID)
 }
 
-// ToOTELSpanID converts OTEL's SpanID to the model representation of a span identitfier.
+// ToV1SpanID converts OTEL's SpanID to the model representation of a span identitfier.
 // This was taken from
 // https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/main/internal/coreinternal/idutils/big_endian_converter.go.
-func SpanIDFromOTEL(spanID pcommon.SpanID) SpanID {
-	return SpanID(binary.BigEndian.Uint64(spanID[:]))
+func ToV1SpanID(spanID pcommon.SpanID) model.SpanID {
+	return model.SpanID(binary.BigEndian.Uint64(spanID[:]))
 }