Skip to content

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

Merged
rajkumar-rangaraj merged 3 commits intoopen-telemetry:mainfrom
Kielek:otlp-disk-retry
Apr 17, 2026
Merged

[Exporter.OTLP] Require path when disk-retry is configured#7106
rajkumar-rangaraj merged 3 commits intoopen-telemetry:mainfrom
Kielek:otlp-disk-retry

Conversation

@Kielek
Copy link
Copy Markdown
Member

@Kielek Kielek commented Apr 17, 2026

Changes

  • 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.

Merge requirement checklist

  • CONTRIBUTING guidelines followed (license requirements, nullable enabled, static analysis, etc.)
  • Unit tests added/updated
  • Appropriate CHANGELOG.md files updated for non-trivial changes
  • [ ] Changes in public API reviewed (if applicable)

@Kielek Kielek requested a review from a team as a code owner April 17, 2026 05:44
@github-actions github-actions bot added the pkg:OpenTelemetry.Exporter.OpenTelemetryProtocol Issues related to OpenTelemetry.Exporter.OpenTelemetryProtocol NuGet package label Apr 17, 2026
@codecov
Copy link
Copy Markdown

codecov bot commented Apr 17, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 88.85%. Comparing base (6fc51c6) to head (ef2b3b5).
⚠️ Report is 1 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #7106      +/-   ##
==========================================
+ Coverage   88.79%   88.85%   +0.06%     
==========================================
  Files         271      271              
  Lines       12970    12971       +1     
==========================================
+ Hits        11517    11526       +9     
+ Misses       1453     1445       -8     
Flag Coverage Δ
unittests-Project-Experimental 88.79% <100.00%> (+0.05%) ⬆️
unittests-Project-Stable 88.69% <100.00%> (-0.10%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...etryProtocol/Implementation/ExperimentalOptions.cs 95.00% <100.00%> (+0.26%) ⬆️

... and 4 files with indirect coverage changes

@rajkumar-rangaraj
Copy link
Copy Markdown
Member

We've been moving toward enabling disk retry by default, and requiring an explicit env var to use it at all works against that direction. Thoughts?

@Kielek
Copy link
Copy Markdown
Member Author

Kielek commented Apr 17, 2026

@rajkumar-rangaraj, I think that this plan can be against existing common behavior in OTel.
Short check with Collector/Go-lang/Java - all of them utilize in-memory option (with some differences in the implementations).

EDIT: Similar scenario for rust, nodejs and python. If retry available it is in-memory by default.

@martincostello
Copy link
Copy Markdown
Member

Maybe we could help improve the DX for the breaking change by adding something like this to the CHANGELOG entry?

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))

@martincostello
Copy link
Copy Markdown
Member

We could also potentially do something like noted in https://github.com/dotnet/core/pull/10376 and introduce a feature switch that enables the old behaviour in code:

else if (AppContext.TryGetSwitch("OpenTelemetry.Exporter.OpenTelemetryProtocol.DangerousUseTempPathForDiskRetry", out var isEnabled) && isEnabled)
{
    this.DiskRetryDirectoryPath = Path.GetTempPath();
}
else
{
    throw new NotSupportedException(
        $"Retry Policy '{retryPolicy}' requires '{OtlpDiskRetryDirectoryPathEnvVar}' to be configured.");
}

Users could then enable that with:

AppContext.SetSwitch("OpenTelemetry.Exporter.OpenTelemetryProtocol.DangerousUseTempPathForDiskRetry", true);

The disadvantage to this is that you have to change the code - you can't enable the context switch through configuration.

@Kielek
Copy link
Copy Markdown
Member Author

Kielek commented Apr 17, 2026

I do not think that we should add this kind of switch to experimental feature. I think that setting env. var. is equally hard as setting application switch.

@rajkumar-rangaraj rajkumar-rangaraj added this pull request to the merge queue Apr 17, 2026
Merged via the queue into open-telemetry:main with commit 78dffdc Apr 17, 2026
102 of 104 checks passed
@Kielek Kielek deleted the otlp-disk-retry branch April 18, 2026 04:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pkg:OpenTelemetry.Exporter.OpenTelemetryProtocol Issues related to OpenTelemetry.Exporter.OpenTelemetryProtocol NuGet package

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants