From ed814d1d6d9b9c9bda7aebca5b7a09fcadaa1ec8 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Robert=20Paj=C4=85k?= <rpajak@splunk.com>
Date: Tue, 12 Oct 2021 18:59:19 +0200
Subject: [PATCH] Throw an exception when parsing an OTEL_BSP_* env var fails
 (#2395)

---
 .../BatchExportActivityProcessorOptions.cs    | 21 +++++++++++--------
 ...BatchExportActivityProcessorOptionsTest.cs |  4 +---
 2 files changed, 13 insertions(+), 12 deletions(-)

diff --git a/src/OpenTelemetry/Trace/BatchExportActivityProcessorOptions.cs b/src/OpenTelemetry/Trace/BatchExportActivityProcessorOptions.cs
index a150cc74d..d0dca246e 100644
--- a/src/OpenTelemetry/Trace/BatchExportActivityProcessorOptions.cs
+++ b/src/OpenTelemetry/Trace/BatchExportActivityProcessorOptions.cs
@@ -21,6 +21,11 @@ using OpenTelemetry.Internal;
 
 namespace OpenTelemetry.Trace
 {
+    /// <summary>
+    /// Batch span processor options.
+    /// OTEL_BSP_MAX_QUEUE_SIZE, OTEL_BSP_MAX_EXPORT_BATCH_SIZE, OTEL_BSP_EXPORT_TIMEOUT, OTEL_BSP_SCHEDULE_DELAY
+    /// environment variables are parsed during object construction.
+    /// </summary>
     public class BatchExportActivityProcessorOptions : BatchExportProcessorOptions<Activity>
     {
         internal const string MaxQueueSizeEnvVarKey = "OTEL_BSP_MAX_QUEUE_SIZE";
@@ -35,28 +40,28 @@ namespace OpenTelemetry.Trace
         {
             int value;
 
-            if (TryLoadEnvVarInt(ExporterTimeoutEnvVarKey, out value))
+            if (LoadEnvVarInt(ExporterTimeoutEnvVarKey, out value))
             {
                 this.ExporterTimeoutMilliseconds = value;
             }
 
-            if (TryLoadEnvVarInt(MaxExportBatchSizeEnvVarKey, out value))
+            if (LoadEnvVarInt(MaxExportBatchSizeEnvVarKey, out value))
             {
                 this.MaxExportBatchSize = value;
             }
 
-            if (TryLoadEnvVarInt(MaxQueueSizeEnvVarKey, out value))
+            if (LoadEnvVarInt(MaxQueueSizeEnvVarKey, out value))
             {
                 this.MaxQueueSize = value;
             }
 
-            if (TryLoadEnvVarInt(ScheduledDelayEnvVarKey, out value))
+            if (LoadEnvVarInt(ScheduledDelayEnvVarKey, out value))
             {
                 this.ScheduledDelayMilliseconds = value;
             }
         }
 
-        private static bool TryLoadEnvVarInt(string envVarKey, out int result)
+        private static bool LoadEnvVarInt(string envVarKey, out int result)
         {
             result = 0;
 
@@ -78,13 +83,11 @@ namespace OpenTelemetry.Trace
                 return false;
             }
 
-            if (!int.TryParse(value, out var parsedValue))
+            if (!int.TryParse(value, out result))
             {
-                OpenTelemetrySdkEventSource.Log.FailedToParseEnvironmentVariable(envVarKey, value);
-                return false;
+                throw new ArgumentException($"{envVarKey} environment variable has an invalid value: '${value}'");
             }
 
-            result = parsedValue;
             return true;
         }
     }
diff --git a/test/OpenTelemetry.Tests/Trace/BatchExportActivityProcessorOptionsTest.cs b/test/OpenTelemetry.Tests/Trace/BatchExportActivityProcessorOptionsTest.cs
index 327752fa3..cd62ae57c 100644
--- a/test/OpenTelemetry.Tests/Trace/BatchExportActivityProcessorOptionsTest.cs
+++ b/test/OpenTelemetry.Tests/Trace/BatchExportActivityProcessorOptionsTest.cs
@@ -63,9 +63,7 @@ namespace OpenTelemetry.Trace.Tests
         {
             Environment.SetEnvironmentVariable(BatchExportActivityProcessorOptions.ExporterTimeoutEnvVarKey, "invalid");
 
-            var options = new BatchExportActivityProcessorOptions();
-
-            Assert.Equal(30000, options.ExporterTimeoutMilliseconds); // use default
+            Assert.Throws<ArgumentException>(() => new BatchExportActivityProcessorOptions());
         }
 
         [Fact]