Addressed comments

Signed-off-by: Alok Kumar Singh <[email protected]>
open-telemetry · Nov 10, 2024 · 063e396 · 063e396
1 parent b4c3aa9
commit 063e396
Show file tree

Hide file tree

Showing 2 changed files with 31 additions and 36 deletions.
diff --git a/samplers/jaegerremote/sampler_remote_options.go b/samplers/jaegerremote/sampler_remote_options.go
@@ -45,34 +45,36 @@ func getEnvOptions() ([]Option, []error) {
 	var options []Option
 	// list of errors which will be logged once logger is set by the user
 	var errs []error
-	if os.Getenv("OTEL_TRACES_SAMPLER") == "jaeger_remote" {
-		args := strings.Split(os.Getenv("OTEL_TRACES_SAMPLER_ARG"), ",")
-		for _, arg := range args {
-			keyValue := strings.Split(arg, "=")
-			if len(keyValue) != 2 {
-				errs = append(errs, fmt.Errorf("argument %s is not of type '<key>=<value>'", arg))
+
+	args := strings.Split(os.Getenv("OTEL_TRACES_SAMPLER_ARG"), ",")
+	for _, arg := range args {
+		keyValue := strings.Split(arg, "=")
+		if len(keyValue) != 2 {
+			errs = append(errs, fmt.Errorf("argument %s is not of type '<key>=<value>'", arg))
+			continue
+		}
+		keyValue[0] = strings.Trim(keyValue[0], " ")
+		keyValue[1] = strings.Trim(keyValue[1], " ")
+
+		switch keyValue[0] {
+		case "endpoint":
+			options = append(options, WithSamplingServerURL(keyValue[1]))
+		case "pollingIntervalMs":
+			intervalMs, err := strconv.ParseUint(keyValue[1], 10, 0)
+			if err != nil {
+				errs = append(errs, fmt.Errorf("%s parsing failed with :%w", keyValue[0], err))
 				continue
 			}
-			switch keyValue[0] {
-			case "endpoint":
-				options = append(options, WithSamplingServerURL(keyValue[1]))
-			case "pollingIntervalMs":
-				intervalMs, err := strconv.ParseUint(keyValue[1], 10, 0)
-				if err != nil {
-					errs = append(errs, err)
-					continue
-				}
-				options = append(options, WithSamplingRefreshInterval(time.Duration(intervalMs)*time.Millisecond))
-			case "initialSamplingRate":
-				samplingRate, err := strconv.ParseFloat(keyValue[1], 64)
-				if err != nil {
-					errs = append(errs, err)
-					continue
-				}
-				options = append(options, WithInitialSampler(trace.TraceIDRatioBased(samplingRate)))
-			default:
-				errs = append(errs, fmt.Errorf("invalid argument %s in OTEL_TRACE_SAMPLER_ARG", keyValue[0]))
+			options = append(options, WithSamplingRefreshInterval(time.Duration(intervalMs)*time.Millisecond))
+		case "initialSamplingRate":
+			samplingRate, err := strconv.ParseFloat(keyValue[1], 64)
+			if err != nil {
+				errs = append(errs, fmt.Errorf("%s parsing failed with :%w", keyValue[0], err))
+				continue
 			}
+			options = append(options, WithInitialSampler(trace.TraceIDRatioBased(samplingRate)))
+		default:
+			errs = append(errs, fmt.Errorf("invalid argument %s in OTEL_TRACE_SAMPLER_ARG", keyValue[0]))
 		}
 	}
 	return options, errs

diff --git a/samplers/jaegerremote/sampler_remote_test.go b/samplers/jaegerremote/sampler_remote_test.go
@@ -603,32 +603,26 @@ func TestEnvVarSettingForNewTracer(t *testing.T) {
 	}
 
 	tests := []struct {
-		otelTraceSampler     string
 		otelTraceSamplerArgs string
 		expErrs              []string
 		codeOptions          []Option
 		expConfig            testConfig
 	}{
 		{
-			otelTraceSampler:     "jaeger_remote",
 			otelTraceSamplerArgs: "endpoint=http://localhost:14250,pollingIntervalMs=5000,initialSamplingRate=0.25",
 			expErrs:              []string{},
 		},
 		{
-			otelTraceSampler:     "jaeger_remote",
-			otelTraceSamplerArgs: "endpointhttp://localhost:14250,pollingIntervalMs=5000,initialSamplingRate=0.25,invalidKey=invalidValue",
+			otelTraceSamplerArgs: "endpointhttp://localhost:14250,pollingIntervalMs=5x000,initialSamplingRate=0.xyz25,invalidKey=invalidValue",
 			expErrs: []string{
 				"argument endpointhttp://localhost:14250 is not of type '<key>=<value>'",
+				"pollingIntervalMs parsing failed",
+				"initialSamplingRate parsing failed",
 				"invalid argument invalidKey in OTEL_TRACE_SAMPLER_ARG",
 			},
 		},
-		{
-			otelTraceSampler: "some_sampler",
-			expErrs:          []string{},
-		},
 		{
 			// Make sure we don't override values provided in code
-			otelTraceSampler:     "jaeger_remote",
 			otelTraceSamplerArgs: "endpoint=http://localhost:14250,pollingIntervalMs=5000,initialSamplingRate=0.25",
 			expErrs:              []string{},
 			codeOptions: []Option{
@@ -643,14 +637,13 @@ func TestEnvVarSettingForNewTracer(t *testing.T) {
 
 	for _, test := range tests {
 		t.Run("", func(t *testing.T) {
-			t.Setenv("OTEL_TRACES_SAMPLER", test.otelTraceSampler)
 			t.Setenv("OTEL_TRACES_SAMPLER_ARG", test.otelTraceSamplerArgs)
 
 			_, errs := getEnvOptions()
 			require.Equal(t, len(test.expErrs), len(errs))
 
 			for i := range len(errs) {
-				require.EqualError(t, errs[i], test.expErrs[i])
+				require.ErrorContains(t, errs[i], test.expErrs[i])
 			}
 
 			if test.codeOptions != nil {