Skip to content

[OpenTelemetry] Reduce redaction allocations - #8102

Merged
martincostello merged 2 commits into
open-telemetry:mainfrom
martincostello:self-diagnostics-redaction-perf
Nov 10, 2026
Merged

martincostello merged 2 commits into
open-telemetry:mainfrom
martincostello:self-diagnostics-redaction-perf

Conversation

@martincostello

@martincostello martincostello commented Nov 9, 2026 •

Copy link
Copy Markdown
Member

Changes

Refactor redaction of environment variables to avoid allocations by using spans.

I had a GitHub Copilot cloud agent do a performance optimisation sweep, and it found this. Not on the hot-path and opt-in, but big enough to consider taking.

Benchmarks

BenchmarkDotNet v0.16.0-preview.2, Linux Ubuntu 24.04.3 LTS, Intel Xeon Platinum 8370C CPU 2.80GHz, .NET 11.0.0 (11.0.0-rc.3.26522.148), DefaultJob:

Method AttributeCount Mean Error StdDev Ratio Gen0 Gen1 Allocated Alloc Ratio
SplitBased 3 477.1 ns 18.36 ns 52.69 ns 1.00 0.0396 - 1,000 B 1.00
SpanBased 3 189.2 ns 5.87 ns 17.04 ns 0.40 0.0076 - 192 B 0.19
SplitBased 12 2,234.7 ns 75.96 ns 216.72 ns 1.00 0.1984 - 5,048 B 1.00
SpanBased 12 1,129.3 ns 32.71 ns 94.37 ns 0.51 0.0629 - 1,616 B 0.32
SplitBased 40 7,839.8 ns 297.71 ns 863.70 ns 1.00 0.6866 0.0153 17,368 B 1.00
SpanBased 40 3,018.1 ns 60.33 ns 128.57 ns 0.39 0.2136 - 5,424 B 0.31
Benchmarks code
using System.Collections.Frozen;
using BenchmarkDotNet.Attributes;
using OpenTelemetry.Internal;

namespace Benchmarks.Diagnostics;

[MemoryDiagnoser]
public class ResourceAttributeRedactionBenchmarks
{
    private static readonly FrozenSet<string> SafeKeys = new[]
    {
        "deployment.environment",
        "deployment.environment.name",
        "service.instance.id",
        "service.name",
        "service.namespace",
        "service.version",
    }.ToFrozenSet(StringComparer.OrdinalIgnoreCase);

    private static readonly string[] SafePrefixes =
    [
        "cloud.",
        "container.",
        "host.",
        "k8s.",
        "os.",
        "process.runtime.",
        "telemetry.distro.",
        "telemetry.sdk.",
    ];

    private string value = string.Empty;

    [Params(3, 12, 40)]
    public int AttributeCount { get; set; }

    [GlobalSetup]
    public void Setup()
    {
        var attributes = new string[this.AttributeCount];

        for (var i = 0; i < attributes.Length; i++)
        {
            attributes[i] = i switch
            {
                0 => "service.name=checkout",
                1 => "deployment.environment=production",
                2 => "k8s.namespace.name=payments",
                _ => $"tenant.attribute{i}=sensitive-value-{i}",
            };
        }

        this.value = string.Join(',', attributes);
    }

    [Benchmark(Baseline = true)]
    public string SplitBased()
    {
        var pairs = this.value.Split(',');
        var builder = StringBuilderCache.Acquire(this.value.Length);

        for (var i = 0; i < pairs.Length; i++)
        {
            if (i > 0)
            {
                builder.Append(',');
            }

            var pair = pairs[i];
            if (pair.Length == 0)
            {
                continue;
            }

            var parts = pair.Split(['='], 2);
            if (parts.Length != 2)
            {
                builder.Append(SensitiveDataHelper.RedactedValue);
                continue;
            }

            builder.Append(parts[0]).Append('=');
            builder.Append(IsSafeResourceAttributeKey(parts[0].Trim()) ? parts[1] : SensitiveDataHelper.RedactedValue);
        }

        return StringBuilderCache.GetStringAndRelease(builder);
    }

    [Benchmark]
    public string SpanBased()
    {
        var builder = StringBuilderCache.Acquire(this.value.Length);
        var remaining = this.value.AsSpan();
        var first = true;

        while (true)
        {
            var separator = remaining.IndexOf(',');
            var pair = separator < 0 ? remaining : remaining[..separator];

            if (!first)
            {
                builder.Append(',');
            }

            if (!pair.IsEmpty)
            {
                var equals = pair.IndexOf('=');
                if (equals < 0)
                {
                    builder.Append(SensitiveDataHelper.RedactedValue);
                }
                else
                {
                    var key = pair[..equals];
                    builder.Append(key).Append('=');
                    builder.Append(IsSafeResourceAttributeKey(key.Trim()) ? pair[(equals + 1)..] : SensitiveDataHelper.RedactedValue);
                }
            }

            if (separator < 0)
            {
                break;
            }

            remaining = remaining[(separator + 1)..];
            first = false;
        }

        return StringBuilderCache.GetStringAndRelease(builder);
    }

    private static bool IsSafeResourceAttributeKey(string key)
    {
        if (SafeKeys.Contains(key))
        {
            return true;
        }

        foreach (var prefix in SafePrefixes)
        {
            if (key.StartsWith(prefix, StringComparison.OrdinalIgnoreCase))
            {
                return true;
            }
        }

        return false;
    }

    private static bool IsSafeResourceAttributeKey(ReadOnlySpan<char> key)
    {
        var alternateLookup = SafeKeys.GetAlternateLookup<ReadOnlySpan<char>>();
        if (alternateLookup.Contains(key))
        {
            return true;
        }

        foreach (var prefix in SafePrefixes)
        {
            if (key.StartsWith(prefix, StringComparison.OrdinalIgnoreCase))
            {
                return true;
            }
        }

        return false;
    }
}

Merge requirement checklist

  • CONTRIBUTING guidelines followed (license requirements, successful build/tests, etc.)
  • Tests added/updated
  • CHANGELOG.md file(s) updated for non-trivial changes
  • Changes in public API reviewed (if applicable)
  • AI usage disclosure:
    • No AI used
    • AI-assisted
    • Bulk AI-generated
  • I have the experience and knowledge necessary to understand, review, and validate all content in this PR1

Footnotes

  1. Yes, I can answer maintainer questions about the content of this PR without using AI. ↩

Refactor redaction of environment variables to avoid allocations by using spans.

Assisted-by: Copilot <223556219+Copilot@users.noreply.github.com>
@martincostello
martincostello requested a balanced review from Copilot November 9, 2026 17:48
@opentelemetry-pr-dashboard

opentelemetry-pr-dashboard Bot commented Nov 9, 2026 •

Copy link
Copy Markdown

Pull request dashboard status

Merged · refreshed 2026-11-10 01:06 UTC

Status above doesn't look right?
  • Anything look wrong? Report it with what you expected; it helps us improve the dashboard.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

No active review issues block the resource-attribute redaction allocation optimization.

No open issues 🎉

What changed in this PR

Reduces allocations when redacting resource attributes in the SDK's opt-in self-diagnostics output, while preserving the existing redaction policy.

Changes:

  • Replaces comma and equals-sign string splitting with span-based parsing.
  • Appends keys directly from the original string.
  • Uses span-based safe-key lookup on modern .NET, retaining a string fallback for older targets.
File Description
src/​OpenTelemetry/​Internal/​Diagnostics/​SelfDiagnosticsEnvironmentVariablePolicy.cs Refactors resource-attribute parsing and safe-key lookup to reduce temporary allocations.

Review effort: Balanced

Avoid using indexer so the code compiles for all TFMs.
@codecov

codecov Bot commented Nov 9, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.11%. Comparing base (1e88560) to head (25bc7c3).
⚠️ Report is 2 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #8102      +/-   ##
==========================================
+ Coverage   93.09%   93.11%   +0.01%     
==========================================
  Files         397      397              
  Lines       23335    23343       +8     
  Branches     4359     4361       +2     
==========================================
+ Hits        21724    21735      +11     
+ Misses       1103     1100       -3     
  Partials      508      508              
Flag Coverage Δ
unittests-Project-Experimental 93.20% <100.00%> (-0.01%) ⬇️
unittests-Project-Stable 93.17% <100.00%> (-0.05%) ⬇️

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

Files with missing lines Coverage Δ
...ostics/SelfDiagnosticsEnvironmentVariablePolicy.cs 98.64% <100.00%> (+0.05%) ⬆️

... and 1 file with indirect coverage changes

@martincostello
martincostello marked this pull request as ready for review November 9, 2026 18:21
@martincostello
martincostello requested a review from a team as a code owner November 9, 2026 18:21
var separator = remaining.IndexOf(',');
var pair = separator < 0 ? remaining : remaining.Slice(0, separator);

if (!first)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: this block adds the comma without checking whether the pair could be empty or malformed and would end up with a comma without its content.

Input                             Output
service.name=checkout,broken       service.name=checkout,[REDACTED]
service.name=checkout,             service.name=checkout,
service.name=checkout,,            service.name=checkout,,

Although this is the same behavior as before the change and not in scope of this PR.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@martincostello
martincostello added this pull request to the merge queue Nov 10, 2026
Merged via the queue into open-telemetry:main with commit f7eaaab Nov 10, 2026
53 checks passed
@martincostello
martincostello deleted the self-diagnostics-redaction-perf branch November 10, 2026 01:05
@martincostello martincostello added this to the v1.21.0 milestone Nov 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants