[Exporter.OTLP] Require path when disk-retry is configured (#7106)

Co-authored-by: Rajkumar Rangaraj <rajrang@microsoft.com>

Author: Kielek

src/OpenTelemetry.Exporter.OpenTelemetryProtocol/CHANGELOG.md

@@ -7,6 +7,20 @@ Notes](../../RELEASENOTES.md).
 
 ## Unreleased
 
+* **Breaking change:** Fixed an insecure disk retry default. Disk retry now
+  requires `OTEL_DOTNET_EXPERIMENTAL_OTLP_DISK_RETRY_DIRECTORY_PATH` when
+  `OTEL_DOTNET_EXPERIMENTAL_OTLP_RETRY=disk` is configured. The exporter no
+  longer falls back to a shared temp directory by default.
+  To retain the previous behaviour, set the
+  `OTEL_DOTNET_EXPERIMENTAL_OTLP_DISK_RETRY_DIRECTORY_PATH` environment
+  variable to the value one of the following environment variables:
+
+  * `TMP`, `TMP`, or `USERPROFILE` ([Windows](https://learn.microsoft.com/windows/win32/api/fileapi/nf-fileapi-gettemppath2w#remarks))
+  * `TMPDIR` (or the literal value `/tmp/`) ([Linux](https://learn.microsoft.com/dotnet/api/system.io.path.gettemppath?tabs=linux),
+      [macOS](https://learn.microsoft.com/dotnet/api/system.io.path.gettemppath?tabs=macos)).
+
+  ([#7106](https://github.com/open-telemetry/opentelemetry-dotnet/pull/7106))
+
 * Fixed an issue in OTLP/gRPC retry handling where parsing gRPC status.
   ([#7064](https://github.com/open-telemetry/opentelemetry-dotnet/pull/7064))
 

src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/ExperimentalOptions.cs

@@ -42,8 +42,8 @@ public ExperimentalOptions(IConfiguration configuration)
                 }
                 else
                 {
-                    // Fallback to temp location.
-                    this.DiskRetryDirectoryPath = Path.GetTempPath();
+                    throw new NotSupportedException(
+                        $"Retry Policy '{retryPolicy}' requires '{OtlpDiskRetryDirectoryPathEnvVar}' to be configured.");
                 }
             }
             else

src/OpenTelemetry.Exporter.OpenTelemetryProtocol/README.md

@@ -652,12 +652,9 @@ want to solicit feedback from the community.
       Added in `1.8.0`.
 
     * When set to `disk`, it enables retries by storing telemetry on disk during
-    transient errors. The default path where the telemetry is stored is
-    obtained by calling
-    [Path.GetTempPath()](https://learn.microsoft.com/dotnet/api/system.io.path.gettemppath)
-    or can be customized by setting
-    `OTEL_DOTNET_EXPERIMENTAL_OTLP_DISK_RETRY_DIRECTORY_PATH` environment
-    variable.
+    transient errors. You MUST set
+    `OTEL_DOTNET_EXPERIMENTAL_OTLP_DISK_RETRY_DIRECTORY_PATH` to a dedicated
+    directory location.
 
       The OTLP exporter utilizes a forked version of the
       [OpenTelemetry.PersistentStorage.FileSystem](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/tree/main/src/OpenTelemetry.PersistentStorage.FileSystem)

test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpExporterOptionsExtensionsTests.cs

@@ -102,14 +102,11 @@ public void AppendPathIfNotPresent_TracesPath_AppendsCorrectly(string input, str
 #pragma warning disable CS0618 // Suppressing gRPC obsolete warning
     [InlineData(OtlpExportProtocol.Grpc, typeof(OtlpGrpcExportClient), false, 10000, null)]
     [InlineData(OtlpExportProtocol.Grpc, typeof(OtlpGrpcExportClient), false, 10000, "in_memory")]
-    [InlineData(OtlpExportProtocol.Grpc, typeof(OtlpGrpcExportClient), false, 10000, "disk")]
 #pragma warning restore CS0618 // Suppressing gRPC obsolete warning
     [InlineData(OtlpExportProtocol.HttpProtobuf, typeof(OtlpHttpExportClient), false, 10000, null)]
     [InlineData(OtlpExportProtocol.HttpProtobuf, typeof(OtlpHttpExportClient), true, 8000, null)]
     [InlineData(OtlpExportProtocol.HttpProtobuf, typeof(OtlpHttpExportClient), false, 10000, "in_memory")]
     [InlineData(OtlpExportProtocol.HttpProtobuf, typeof(OtlpHttpExportClient), true, 8000, "in_memory")]
-    [InlineData(OtlpExportProtocol.HttpProtobuf, typeof(OtlpHttpExportClient), false, 10000, "disk")]
-    [InlineData(OtlpExportProtocol.HttpProtobuf, typeof(OtlpHttpExportClient), true, 8000, "disk")]
     public void GetTransmissionHandler_InitializesCorrectHandlerExportClientAndTimeoutValue(OtlpExportProtocol protocol, Type exportClientType, bool customHttpClient, int expectedTimeoutMilliseconds, string? retryStrategy)
     {
         var exporterOptions = new OtlpExporterOptions() { Protocol = protocol };
@@ -129,6 +126,47 @@ public void GetTransmissionHandler_InitializesCorrectHandlerExportClientAndTimeo
         AssertTransmissionHandler(transmissionHandler, exportClientType, expectedTimeoutMilliseconds, retryStrategy);
     }
 
+    [Theory]
+#pragma warning disable CS0618 // Suppressing gRPC obsolete warning
+    [InlineData(OtlpExportProtocol.Grpc, typeof(OtlpGrpcExportClient), false, 10000)]
+#pragma warning restore CS0618 // Suppressing gRPC obsolete warning
+    [InlineData(OtlpExportProtocol.HttpProtobuf, typeof(OtlpHttpExportClient), false, 10000)]
+    [InlineData(OtlpExportProtocol.HttpProtobuf, typeof(OtlpHttpExportClient), true, 8000)]
+    public void GetTransmissionHandler_DiskRetryWithDirectory_InitializesCorrectHandlerExportClientAndTimeoutValue(OtlpExportProtocol protocol, Type exportClientType, bool customHttpClient, int expectedTimeoutMilliseconds)
+    {
+        var exporterOptions = new OtlpExporterOptions() { Protocol = protocol };
+        if (customHttpClient)
+        {
+            exporterOptions.HttpClientFactory = () =>
+            {
+                return new HttpClient { Timeout = TimeSpan.FromMilliseconds(expectedTimeoutMilliseconds) };
+            };
+        }
+
+        var configuration = new ConfigurationBuilder()
+            .AddInMemoryCollection(
+            new Dictionary<string, string?>
+            {
+                [ExperimentalOptions.OtlpRetryEnvVar] = "disk",
+                [ExperimentalOptions.OtlpDiskRetryDirectoryPathEnvVar] = Path.GetTempPath(),
+            })
+            .Build();
+
+        var transmissionHandler = exporterOptions.GetExportTransmissionHandler(new ExperimentalOptions(configuration), OtlpSignalType.Traces);
+        AssertTransmissionHandler(transmissionHandler, exportClientType, expectedTimeoutMilliseconds, "disk");
+    }
+
+    [Fact]
+    public void GetTransmissionHandler_DiskRetryWithoutDirectory_Throws()
+    {
+        var configuration = new ConfigurationBuilder()
+            .AddInMemoryCollection(new Dictionary<string, string?> { [ExperimentalOptions.OtlpRetryEnvVar] = "disk" })
+            .Build();
+
+        var exception = Assert.Throws<NotSupportedException>(() => new ExperimentalOptions(configuration));
+        Assert.Contains(ExperimentalOptions.OtlpDiskRetryDirectoryPathEnvVar, exception.Message, StringComparison.Ordinal);
+    }
+
     [Theory]
     [InlineData("Traces", "traces")]
     [InlineData("Logs", "logs")]