[cmd/telemetrygen] remove reliance on JSON unmarshalling for status c…

…ode string (#26985)

Removes the use of JSON Unmarshalling when determining the provided
trace status code.

The existing implementation allowed both status code and status strings
to be set using the command line flag `--status-code` without issue
however it's not obvious that the default command line value must be a
JSON string as encountered in #25906

I've replaced the existing code with a switch-case statement which is
more explicit. Additionally, the code now checks the lowercase version
of the provided string, enabling support for mixed-case inputs such as
`--status-code unset` and `--status-code Unset` flags.

I'm undecided if this is actually better or required since we now
understand the original cause of the issue.

Example usage is now:

```
telemetrygen traces --otlp-insecure --traces 1 --status-code 0 # unset
telemetrygen traces --otlp-insecure --traces 1 --status-code 1 # error
telemetrygen traces --otlp-insecure --traces 1 --status-code 2 # ok 
telemetrygen traces --otlp-insecure --traces 1 --status-code unset
telemetrygen traces --otlp-insecure --traces 1 --status-code error
telemetrygen traces --otlp-insecure --traces 1 --status-code ok
```

Fixes #25906

Added additional test cases and implemented sub-tests as the tests were
quitting after the first falsified case.

---------

Co-authored-by: Ziqi Zhao <[email protected]>
Co-authored-by: Pablo Baeyens <[email protected]>

.chloggen/fix_telemetrygen-trace-status-code-parameter.yaml

@@ -0,0 +1,25 @@
+# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
+change_type: bug_fix
+
+# The name of the component, or a single word describing the area of concern, (e.g. filelogreceiver)
+component: telemetrygen
+
+# A brief description of the change.  Surround your text with quotes ("") if it needs to start with a backtick (`).
+note: remove need for JSON unmarshalling of trace status codes and unsupport mixed case input
+
+# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists.
+issues: [25906]
+
+# (Optional) One or more lines of additional information to render under the primary note.
+# These lines will be padded with 2 spaces and then inserted directly into the document.
+# Use pipe (|) for multiline entries.
+subtext:
+
+# If your change doesn't affect end users or the exported elements of any package,
+# you should instead start your pull request title with [chore] or use the "Skip Changelog" label.
+# Optional: The change log or logs in which this entry should be included.
+# e.g. '[user]' or '[user, api]'
+# Include 'user' if the change is relevant to end users.
+# Include 'api' if there is a change to a library API.
+# Default: '[user]'
+change_logs: [ user ]

cmd/telemetrygen/internal/traces/traces.go

@@ -6,6 +6,7 @@ package traces
 import (
 	"context"
 	"fmt"
+	"strings"
 	"sync"
 	"sync/atomic"
 	"time"
@@ -123,13 +124,18 @@ func Run(c *Config, logger *zap.Logger) error {
 	}
 
 	var statusCode codes.Code
-	if c.StatusCode == "" {
+
+	switch strings.ToLower(c.StatusCode) {
+	case "0", "unset", "":
 		statusCode = codes.Unset
-	} else {
-		if err := statusCode.UnmarshalJSON([]byte(c.StatusCode)); err != nil {
-			return fmt.Errorf("expected `status-code` to be one of (Unset, Error, Ok) or (0, 1, 2), got %q instead", c.StatusCode)
-		}
+	case "1", "error":
+		statusCode = codes.Error
+	case "2", "ok":
+		statusCode = codes.Ok
+	default:
+		return fmt.Errorf("expected `status-code` to be one of (Unset, Error, Ok) or (0, 1, 2), got %q instead", c.StatusCode)
 	}
+
 	wg := sync.WaitGroup{}
 
 	running := &atomic.Bool{}

cmd/telemetrygen/internal/traces/worker_test.go

@@ -131,46 +131,53 @@ func TestSpanStatuses(t *testing.T) {
 		spanStatus  codes.Code
 		validInput  bool
 	}{
-		{inputStatus: `"Unset"`, spanStatus: codes.Unset, validInput: true},
-		{inputStatus: `"Error"`, spanStatus: codes.Error, validInput: true},
-		{inputStatus: `"Ok"`, spanStatus: codes.Ok, validInput: true},
+		{inputStatus: `Unset`, spanStatus: codes.Unset, validInput: true},
+		{inputStatus: `Error`, spanStatus: codes.Error, validInput: true},
+		{inputStatus: `Ok`, spanStatus: codes.Ok, validInput: true},
+		{inputStatus: `unset`, spanStatus: codes.Unset, validInput: true},
+		{inputStatus: `error`, spanStatus: codes.Error, validInput: true},
+		{inputStatus: `ok`, spanStatus: codes.Ok, validInput: true},
+		{inputStatus: `UNSET`, spanStatus: codes.Unset, validInput: true},
+		{inputStatus: `ERROR`, spanStatus: codes.Error, validInput: true},
+		{inputStatus: `OK`, spanStatus: codes.Ok, validInput: true},
 		{inputStatus: `0`, spanStatus: codes.Unset, validInput: true},
 		{inputStatus: `1`, spanStatus: codes.Error, validInput: true},
 		{inputStatus: `2`, spanStatus: codes.Ok, validInput: true},
-		{inputStatus: `"Foo"`, spanStatus: codes.Unset, validInput: false},
-		{inputStatus: `"UNSET"`, spanStatus: codes.Unset, validInput: false},
+		{inputStatus: `Foo`, spanStatus: codes.Unset, validInput: false},
 		{inputStatus: `-1`, spanStatus: codes.Unset, validInput: false},
 		{inputStatus: `3`, spanStatus: codes.Unset, validInput: false},
+		{inputStatus: `Err`, spanStatus: codes.Unset, validInput: false},
 	}
 
 	for _, tt := range tests {
-		syncer := &mockSyncer{}
-
-		tracerProvider := sdktrace.NewTracerProvider()
-		sp := sdktrace.NewSimpleSpanProcessor(syncer)
-		tracerProvider.RegisterSpanProcessor(sp)
-		otel.SetTracerProvider(tracerProvider)
-
-		cfg := &Config{
-			Config: common.Config{
-				WorkerCount: 1,
-			},
-			NumTraces:  1,
-			StatusCode: tt.inputStatus,
-		}
-
-		// test the program given input, including erroneous inputs
-		if tt.validInput {
-			require.NoError(t, Run(cfg, zap.NewNop()))
-			// verify that the default the span status is set as expected
-			for _, span := range syncer.spans {
-				assert.Equal(t, span.Status().Code, tt.spanStatus, fmt.Sprintf("span status: %v and expected status %v", span.Status().Code, tt.spanStatus))
+		t.Run(fmt.Sprintf("inputStatus=%s", tt.inputStatus), func(t *testing.T) {
+			syncer := &mockSyncer{}
+
+			tracerProvider := sdktrace.NewTracerProvider()
+			sp := sdktrace.NewSimpleSpanProcessor(syncer)
+			tracerProvider.RegisterSpanProcessor(sp)
+			otel.SetTracerProvider(tracerProvider)
+
+			cfg := &Config{
+				Config: common.Config{
+					WorkerCount: 1,
+				},
+				NumTraces:  1,
+				StatusCode: tt.inputStatus,
 			}
-		} else {
-			require.Error(t, Run(cfg, zap.NewNop()))
-		}
-	}
 
+			// test the program given input, including erroneous inputs
+			if tt.validInput {
+				require.NoError(t, Run(cfg, zap.NewNop()))
+				// verify that the default the span status is set as expected
+				for _, span := range syncer.spans {
+					assert.Equal(t, span.Status().Code, tt.spanStatus, fmt.Sprintf("span status: %v and expected status %v", span.Status().Code, tt.spanStatus))
+				}
+			} else {
+				require.Error(t, Run(cfg, zap.NewNop()))
+			}
+		})
+	}
 }
 
 var _ sdktrace.SpanExporter = (*mockSyncer)(nil)