Update opentelemetry-proto to v0.18.0 (grafana#1754)

* Checkout opentelemetry-proto to v0.18.0 * Re-generate opentelemetry-proto .go files * Update modules * Update Tempo to new proto model * More fixes * Changelog * More explicit changelog
dimitris4000 · Sep 27, 2022 · 7bfeff3 · 7bfeff3
1 parent 8cb38b2
commit 7bfeff3
Show file tree

Hide file tree

Showing 42 changed files with 3,950 additions and 646 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -35,6 +35,8 @@ query_frontend:
 ```
 * [BUGFIX] Honor caching and buffering settings when finding traces by id [#1697](https://github.com/grafana/tempo/pull/1697) (@joe-elliott)
 * [BUGFIX] Correctly propagate errors from the iterator layer up through the queriers [#1723](https://github.com/grafana/tempo/pull/1723) (@joe-elliott)
+* [ENHANCEMENT] Upgrade opentelemetry-proto submodule to v0.18.0 [#1754](https://github.com/grafana/tempo/pull/1754) (@mapno)
+Internal types are updated to use `scope` instead of `instrumentation_library`. This is a breaking change in trace by ID queries if JSON is requested.
 
 ## v1.5.0 / 2022-08-17
 

diff --git a/Makefile b/Makefile
@@ -171,7 +171,7 @@ gen-proto:
 	find $(PROTO_INTERMEDIATE_DIR) -name "*.proto" | xargs -L 1 sed -i $(SED_OPTS) 's+ opentelemetry.proto+ tempopb+g'
 
 	@# Update go_package
-	find $(PROTO_INTERMEDIATE_DIR) -name "*.proto" | xargs -L 1 sed -i $(SED_OPTS) 's+github.com/open-telemetry/opentelemetry-proto/gen/go+github.com/grafana/tempo/pkg/tempopb+g'
+	find $(PROTO_INTERMEDIATE_DIR) -name "*.proto" | xargs -L 1 sed -i $(SED_OPTS) 's+go.opentelemetry.io/proto/otlp+github.com/grafana/tempo/pkg/tempopb+g'
 
 	@# Update import paths
 	find $(PROTO_INTERMEDIATE_DIR) -name "*.proto" | xargs -L 1 sed -i $(SED_OPTS) 's+import "opentelemetry/proto/+import "+g'

diff --git a/cmd/tempo-cli/cmd-list-block.go b/cmd/tempo-cli/cmd-list-block.go
@@ -8,15 +8,13 @@ import (
 	"os"
 	"os/signal"
 	"sort"
-	"strconv"
 	"syscall"
 	"time"
 
 	"github.com/dustin/go-humanize"
 	"github.com/google/uuid"
 	"github.com/grafana/tempo/pkg/model"
 	"github.com/grafana/tempo/pkg/tempopb"
-	v1 "github.com/grafana/tempo/pkg/tempopb/common/v1"
 	"github.com/grafana/tempo/pkg/util"
 	tempodb_backend "github.com/grafana/tempo/tempodb/backend"
 	"github.com/grafana/tempo/tempodb/encoding/common"
@@ -206,12 +204,12 @@ func extractKVPairs(t *tempopb.Trace) kvPairs {
 	kvp := kvPairs{}
 	for _, b := range t.Batches {
 		spanCount := 0
-		for _, ils := range b.InstrumentationLibrarySpans {
+		for _, ils := range b.ScopeSpans {
 			for _, s := range ils.Spans {
 				spanCount++
 				for _, a := range s.Attributes {
-					val, yay := stringVal(a.Value)
-					if !yay {
+					val := util.StringifyAnyValue(a.Value)
+					if val == "" {
 						continue
 					}
 					addKey(kvp, a.Key, 1)
@@ -220,8 +218,8 @@ func extractKVPairs(t *tempopb.Trace) kvPairs {
 			}
 		}
 		for _, a := range b.Resource.Attributes {
-			val, yay := stringVal(a.Value)
-			if !yay {
+			val := util.StringifyAnyValue(a.Value)
+			if val == "" {
 				continue
 			}
 			addKey(kvp, a.Key, spanCount)
@@ -253,29 +251,3 @@ func addVal(kvp kvPairs, key string, val string, count int) {
 	v.all[val] = stats
 	kvp[key] = v
 }
-func stringVal(v *v1.AnyValue) (string, bool) {
-	if sVal, ok := v.Value.(*v1.AnyValue_StringValue); ok {
-		return sVal.StringValue, true
-	}
-	if nVal, ok := v.Value.(*v1.AnyValue_IntValue); ok {
-		return strconv.FormatInt(nVal.IntValue, 10), true
-	}
-	if dVal, ok := v.Value.(*v1.AnyValue_DoubleValue); ok {
-		return fmt.Sprintf("%f", dVal.DoubleValue), true
-		// strconv.FormatFloat()
-	}
-	if bVal, ok := v.Value.(*v1.AnyValue_BoolValue); ok {
-		if bVal.BoolValue {
-			return "true", true
-		}
-		return "false", true
-	}
-	// todo? add support for these?
-	// if kVal, ok := v.Value.(*v1.AnyValue_KvlistValue); ok {
-	//  return fmt.Sprintf("kvval %v", kVal.KvlistValue) // better way?
-	// }
-	// if aVal, ok := v.Value.(*v1.AnyValue_ArrayValue); ok {
-	//  return fmt.Sprintf("arrayval %v", aVal.ArrayValue) // better way?
-	// }
-	return "", false
-}
diff --git a/cmd/tempo-vulture/main.go b/cmd/tempo-vulture/main.go
@@ -428,8 +428,8 @@ func hasMissingSpans(t *tempopb.Trace) bool {
 	linkedSpanIDs := make([][]byte, 0)
 
 	for _, b := range t.Batches {
-		for _, ils := range b.InstrumentationLibrarySpans {
-			for _, s := range ils.Spans {
+		for _, ss := range b.ScopeSpans {
+			for _, s := range ss.Spans {
 				if len(s.ParentSpanId) > 0 {
 					linkedSpanIDs = append(linkedSpanIDs, s.ParentSpanId)
 				}
@@ -442,8 +442,8 @@ func hasMissingSpans(t *tempopb.Trace) bool {
 
 	B:
 		for _, b := range t.Batches {
-			for _, ils := range b.InstrumentationLibrarySpans {
-				for _, s := range ils.Spans {
+			for _, ss := range b.ScopeSpans {
+				for _, s := range ss.Spans {
 					if bytes.Equal(s.SpanId, id) {
 						found = true
 						break B

diff --git a/cmd/tempo-vulture/main_test.go b/cmd/tempo-vulture/main_test.go
@@ -26,7 +26,7 @@ func TestHasMissingSpans(t *testing.T) {
 			&tempopb.Trace{
 				Batches: []*v1.ResourceSpans{
 					{
-						InstrumentationLibrarySpans: []*v1.InstrumentationLibrarySpans{
+						ScopeSpans: []*v1.ScopeSpans{
 							{
 								Spans: []*v1.Span{
 									{
@@ -44,7 +44,7 @@ func TestHasMissingSpans(t *testing.T) {
 			&tempopb.Trace{
 				Batches: []*v1.ResourceSpans{
 					{
-						InstrumentationLibrarySpans: []*v1.InstrumentationLibrarySpans{
+						ScopeSpans: []*v1.ScopeSpans{
 							{
 								Spans: []*v1.Span{
 									{

diff --git a/cmd/tempo-vulture/testdata/trace.json b/cmd/tempo-vulture/testdata/trace.json
diff --git a/integration/e2e/e2e_test.go b/integration/e2e/e2e_test.go
@@ -508,7 +508,7 @@ func equalTraces(a, b *tempopb.Trace) bool {
 func spanCount(a *tempopb.Trace) float64 {
 	count := 0
 	for _, batch := range a.Batches {
-		for _, spans := range batch.InstrumentationLibrarySpans {
+		for _, spans := range batch.ScopeSpans {
 			count += len(spans.Spans)
 		}
 	}

diff --git a/modules/compactor/compactor.go b/modules/compactor/compactor.go
@@ -305,7 +305,7 @@ func countSpans(dataEncoding string, objs ...[]byte) int {
 		}
 
 		for _, b := range t.Batches {
-			for _, ilm := range b.InstrumentationLibrarySpans {
+			for _, ilm := range b.ScopeSpans {
 				spans += len(ilm.Spans)
 			}
 		}

diff --git a/modules/distributor/distributor.go b/modules/distributor/distributor.go
@@ -297,7 +297,7 @@ func (d *Distributor) PushBatches(ctx context.Context, batches []*v1.ResourceSpa
 	spanCount := 0
 	for _, b := range batches {
 		size += b.Size()
-		for _, ils := range b.InstrumentationLibrarySpans {
+		for _, ils := range b.ScopeSpans {
 			spanCount += len(ils.Spans)
 		}
 	}
@@ -453,9 +453,9 @@ func requestsByTraceID(batches []*v1.ResourceSpans, userID string, spanCount int
 	tracesByID := make(map[uint32]*rebatchedTrace, tracesPerBatch)
 
 	for _, b := range batches {
-		spansByILS := make(map[uint32]*v1.InstrumentationLibrarySpans)
+		spansByILS := make(map[uint32]*v1.ScopeSpans)
 
-		for _, ils := range b.InstrumentationLibrarySpans {
+		for _, ils := range b.ScopeSpans {
 			for _, span := range ils.Spans {
 				traceID := span.TraceId
 				if !validation.ValidTraceID(traceID) {
@@ -464,16 +464,16 @@ func requestsByTraceID(batches []*v1.ResourceSpans, userID string, spanCount int
 
 				traceKey := util.TokenFor(userID, traceID)
 				ilsKey := traceKey
-				if ils.InstrumentationLibrary != nil {
-					ilsKey = fnv1a.AddString32(ilsKey, ils.InstrumentationLibrary.Name)
-					ilsKey = fnv1a.AddString32(ilsKey, ils.InstrumentationLibrary.Version)
+				if ils.Scope != nil {
+					ilsKey = fnv1a.AddString32(ilsKey, ils.Scope.Name)
+					ilsKey = fnv1a.AddString32(ilsKey, ils.Scope.Version)
 				}
 
 				existingILS, ilsAdded := spansByILS[ilsKey]
 				if !ilsAdded {
-					existingILS = &v1.InstrumentationLibrarySpans{
-						InstrumentationLibrary: ils.InstrumentationLibrary,
-						Spans:                  make([]*v1.Span, 0, spanCount/tracesPerBatch),
+					existingILS = &v1.ScopeSpans{
+						Scope: ils.Scope,
+						Spans: make([]*v1.Span, 0, spanCount/tracesPerBatch),
 					}
 					spansByILS[ilsKey] = existingILS
 				}
@@ -503,8 +503,8 @@ func requestsByTraceID(batches []*v1.ResourceSpans, userID string, spanCount int
 				}
 				if !ilsAdded {
 					existingTrace.trace.Batches = append(existingTrace.trace.Batches, &v1.ResourceSpans{
-						Resource:                    b.Resource,
-						InstrumentationLibrarySpans: []*v1.InstrumentationLibrarySpans{existingILS},
+						Resource:   b.Resource,
+						ScopeSpans: []*v1.ScopeSpans{existingILS},
 					})
 				}
 			}
@@ -542,7 +542,7 @@ func recordDiscaredSpans(err error, userID string, spanCount int) {
 
 func logSpans(batches []*v1.ResourceSpans, filterByStatusError bool, logger log.Logger) {
 	for _, b := range batches {
-		for _, ils := range b.InstrumentationLibrarySpans {
+		for _, ils := range b.ScopeSpans {
 			for _, s := range ils.Spans {
 				if filterByStatusError && s.Status.Code != v1.Status_STATUS_CODE_ERROR {
 					continue
@@ -567,7 +567,7 @@ func logSpansInResourceWithAllAttributes(rs *v1.ResourceSpans, filterByStatusErr
 			tempo_util.StringifyAnyValue(a.GetValue()))
 	}
 
-	for _, ils := range rs.InstrumentationLibrarySpans {
+	for _, ils := range rs.ScopeSpans {
 		for _, s := range ils.Spans {
 			if filterByStatusError && s.Status.Code != v1.Status_STATUS_CODE_ERROR {
 				continue