Remove unnecessary duplicate code and allocations for reading enums i…

…n JSON (#5928)

Signed-off-by: Bogdan <[email protected]>

Signed-off-by: Bogdan <[email protected]>

CHANGELOG.md

@@ -37,6 +37,7 @@
 - Add support to unmarshalls bytes into pmetric.Metrics with `jsoniter` in jsonUnmarshaler(#5433)
 - Add httpprovider to allow loading config files stored in HTTP (#5810)
 - Added `service.telemetry.traces.propagators` configuration to set propagators for collector's internal spans. (#5572)
+- Remove unnecessary duplicate code and allocations for reading enums in JSON. (#5928)
 
 ### 🧰 Bug fixes 🧰
 

pdata/internal/json/enum.go

@@ -0,0 +1,40 @@
+// Copyright The OpenTelemetry 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 json // import "go.opentelemetry.io/collector/pdata/internal/json"
+
+import (
+	jsoniter "github.com/json-iterator/go"
+)
+
+// ReadEnumValue returns the enum integer value representation. Accepts both enum names and enum integer values.
+// See https://developers.google.com/protocol-buffers/docs/proto3#json.
+func ReadEnumValue(iter *jsoniter.Iterator, valueMap map[string]int32) int32 {
+	switch iter.WhatIsNext() {
+	case jsoniter.NumberValue:
+		return iter.ReadInt32()
+	case jsoniter.StringValue:
+		val, ok := valueMap[iter.ReadString()]
+		// Same behavior with official protbuf JSON decoder,
+		// see https://github.com/open-telemetry/opentelemetry-proto-go/pull/81
+		if !ok {
+			iter.ReportError("ReadEnumValue", "unknown string value")
+			return 0
+		}
+		return val
+	default:
+		iter.ReportError("ReadEnumValue", "unsupported value type")
+		return 0
+	}
+}

pdata/internal/json/enum_test.go

@@ -0,0 +1,85 @@
+// Copyright The OpenTelemetry 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 json
+
+import (
+	"testing"
+
+	jsoniter "github.com/json-iterator/go"
+	"github.com/stretchr/testify/assert"
+)
+
+func TestReadEnumValue(t *testing.T) {
+	valueMap := map[string]int32{
+		"undefined": 0,
+		"foo":       1,
+		"bar":       2,
+	}
+	tests := []struct {
+		name    string
+		jsonStr string
+		want    int32
+		wantErr bool
+	}{
+		{
+			name:    "foo string",
+			jsonStr: "\"foo\"\n",
+			want:    1,
+		},
+		{
+			name:    "foo number",
+			jsonStr: "1\n",
+			want:    1,
+		},
+		{
+			name:    "unknown number",
+			jsonStr: "5\n",
+			want:    5,
+		},
+		{
+			name:    "bar string",
+			jsonStr: "\"bar\"\n",
+			want:    2,
+		},
+		{
+			name:    "bar number",
+			jsonStr: "2\n",
+			want:    2,
+		},
+		{
+			name:    "unknown string",
+			jsonStr: "\"baz\"\n",
+			wantErr: true,
+		},
+		{
+			name:    "wrong type",
+			jsonStr: "true",
+			wantErr: true,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			iter := jsoniter.ConfigFastest.BorrowIterator([]byte(tt.jsonStr))
+			defer jsoniter.ConfigFastest.ReturnIterator(iter)
+			val := ReadEnumValue(iter, valueMap)
+			if tt.wantErr {
+				assert.Error(t, iter.Error)
+				return
+			}
+			assert.NoError(t, iter.Error)
+			assert.Equal(t, tt.want, val)
+		})
+	}
+}

pdata/pmetric/json.go

@@ -527,10 +527,5 @@ func (d *jsonUnmarshaler) readQuantileValue(iter *jsoniter.Iterator) *otlpmetric
 }
 
 func (d *jsonUnmarshaler) readAggregationTemporality(iter *jsoniter.Iterator) otlpmetrics.AggregationTemporality {
-	value := iter.ReadAny()
-	if v := value.ToInt(); v > 0 {
-		return otlpmetrics.AggregationTemporality(v)
-	}
-	v := value.ToString()
-	return otlpmetrics.AggregationTemporality(otlpmetrics.AggregationTemporality_value[v])
+	return otlpmetrics.AggregationTemporality(json.ReadEnumValue(iter, otlpmetrics.AggregationTemporality_value))
 }

pdata/pmetric/json_test.go

@@ -15,7 +15,6 @@
 package pmetric
 
 import (
-	"fmt"
 	"testing"
 	"time"
 
@@ -334,49 +333,6 @@ func Test_jsonUnmarshaler_UnmarshalMetrics(t *testing.T) {
 	}
 }
 
-func TestReadAggregationTemporality(t *testing.T) {
-	tests := []struct {
-		name    string
-		jsonStr string
-		want    otlpmetrics.AggregationTemporality
-	}{
-		{
-			name: "string",
-			jsonStr: fmt.Sprintf(`"%s"`,
-				otlpmetrics.AggregationTemporality_AGGREGATION_TEMPORALITY_CUMULATIVE.String()),
-			want: otlpmetrics.AggregationTemporality_AGGREGATION_TEMPORALITY_CUMULATIVE,
-		},
-		{
-			name: "string",
-			jsonStr: fmt.Sprintf(`"%s"`,
-				otlpmetrics.AggregationTemporality_AGGREGATION_TEMPORALITY_DELTA.String()),
-			want: otlpmetrics.AggregationTemporality_AGGREGATION_TEMPORALITY_DELTA,
-		},
-		{
-			name: "int",
-			jsonStr: fmt.Sprintf("%d",
-				otlpmetrics.AggregationTemporality_AGGREGATION_TEMPORALITY_CUMULATIVE),
-			want: otlpmetrics.AggregationTemporality_AGGREGATION_TEMPORALITY_CUMULATIVE,
-		},
-		{
-			name: "int",
-			jsonStr: fmt.Sprintf("%d",
-				otlpmetrics.AggregationTemporality_AGGREGATION_TEMPORALITY_DELTA),
-			want: otlpmetrics.AggregationTemporality_AGGREGATION_TEMPORALITY_DELTA,
-		},
-	}
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			iter := jsoniter.ConfigFastest.BorrowIterator([]byte(tt.jsonStr))
-			defer jsoniter.ConfigFastest.ReturnIterator(iter)
-			unmarshaler := &jsonUnmarshaler{}
-			if got := unmarshaler.readAggregationTemporality(iter); got != tt.want {
-				t.Errorf("readSpanKind() = %v, want %v", got, tt.want)
-			}
-		})
-	}
-}
-
 func TestReadMetricsDataUnknownField(t *testing.T) {
 	jsonStr := `{"extra":""}`
 	iter := jsoniter.ConfigFastest.BorrowIterator([]byte(jsonStr))

pdata/ptrace/json.go

@@ -158,7 +158,7 @@ func readSpan(iter *jsoniter.Iterator) *otlptrace.Span {
 		case "name":
 			sp.Name = iter.ReadString()
 		case "kind":
-			sp.Kind = readSpanKind(iter)
+			sp.Kind = otlptrace.Span_SpanKind(json.ReadEnumValue(iter, otlptrace.Span_SpanKind_value))
 		case "startTimeUnixNano", "start_time_unix_nano":
 			sp.StartTimeUnixNano = json.ReadUint64(iter)
 		case "endTimeUnixNano", "end_time_unix_nano":
@@ -190,7 +190,7 @@ func readSpan(iter *jsoniter.Iterator) *otlptrace.Span {
 				case "message":
 					sp.Status.Message = iter.ReadString()
 				case "code":
-					sp.Status.Code = readStatusCode(iter)
+					sp.Status.Code = otlptrace.Status_StatusCode(json.ReadEnumValue(iter, otlptrace.Status_StatusCode_value))
 				default:
 					iter.ReportError("readSpan.status", fmt.Sprintf("unknown field:%v", f))
 				}
@@ -257,21 +257,3 @@ func readSpanEvent(iter *jsoniter.Iterator) *otlptrace.Span_Event {
 	})
 	return event
 }
-
-func readSpanKind(iter *jsoniter.Iterator) otlptrace.Span_SpanKind {
-	any := iter.ReadAny()
-	if v := any.ToInt(); v > 0 {
-		return otlptrace.Span_SpanKind(v)
-	}
-	v := any.ToString()
-	return otlptrace.Span_SpanKind(otlptrace.Span_SpanKind_value[v])
-}
-
-func readStatusCode(iter *jsoniter.Iterator) otlptrace.Status_StatusCode {
-	any := iter.ReadAny()
-	if v := any.ToInt(); v > 0 {
-		return otlptrace.Status_StatusCode(v)
-	}
-	v := any.ToString()
-	return otlptrace.Status_StatusCode(otlptrace.Status_StatusCode_value[v])
-}

pdata/ptrace/json_test.go

@@ -15,15 +15,13 @@
 package ptrace
 
 import (
-	"fmt"
 	"testing"
 	"time"
 
 	jsoniter "github.com/json-iterator/go"
 	"github.com/stretchr/testify/assert"
 
 	"go.opentelemetry.io/collector/pdata/internal"
-	otlptrace "go.opentelemetry.io/collector/pdata/internal/data/protogen/trace/v1"
 )
 
 var tracesOTLP = func() Traces {
@@ -285,59 +283,3 @@ func TestReadSpanEventUnknownField(t *testing.T) {
 		assert.Contains(t, iter.Error.Error(), "unknown field")
 	}
 }
-
-func TestReadSpanKind(t *testing.T) {
-	tests := []struct {
-		name    string
-		jsonStr string
-		want    otlptrace.Span_SpanKind
-	}{
-		{
-			name:    "string",
-			jsonStr: fmt.Sprintf(`"%s"`, otlptrace.Span_SPAN_KIND_INTERNAL.String()),
-			want:    otlptrace.Span_SPAN_KIND_INTERNAL,
-		},
-		{
-			name:    "int",
-			jsonStr: fmt.Sprintf("%d", otlptrace.Span_SPAN_KIND_INTERNAL),
-			want:    otlptrace.Span_SPAN_KIND_INTERNAL,
-		},
-	}
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			iter := jsoniter.ConfigFastest.BorrowIterator([]byte(tt.jsonStr))
-			defer jsoniter.ConfigFastest.ReturnIterator(iter)
-			if got := readSpanKind(iter); got != tt.want {
-				t.Errorf("readSpanKind() = %v, want %v", got, tt.want)
-			}
-		})
-	}
-}
-
-func TestReadStatusCode(t *testing.T) {
-	tests := []struct {
-		name    string
-		jsonStr string
-		want    otlptrace.Status_StatusCode
-	}{
-		{
-			name:    "string",
-			jsonStr: fmt.Sprintf(`"%s"`, otlptrace.Status_STATUS_CODE_ERROR.String()),
-			want:    otlptrace.Status_STATUS_CODE_ERROR,
-		},
-		{
-			name:    "int",
-			jsonStr: fmt.Sprintf("%d", otlptrace.Status_STATUS_CODE_ERROR),
-			want:    otlptrace.Status_STATUS_CODE_ERROR,
-		},
-	}
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			iter := jsoniter.ConfigFastest.BorrowIterator([]byte(tt.jsonStr))
-			defer jsoniter.ConfigFastest.ReturnIterator(iter)
-			if got := readStatusCode(iter); got != tt.want {
-				t.Errorf("readStatusCode() = %v, want %v", got, tt.want)
-			}
-		})
-	}
-}