[Exporter.Prometheus] Sanitize metric unit

[nabutabu]

Fixes #6187

Changes

The Prometheus exporter was not sanitizing metric unit strings before appending
them to the metric name. This caused malformed metric names when units contained
characters invalid in Prometheus (e.g. # RU from Azure Cosmos DB), which
Prometheus silently drops because # denotes a comment line in the text format.

A second related bug was also fixed: unit strings with multiple / characters
(e.g. req/s/extra) only had the first / converted to _per_, leaving
subsequent slashes in the final name, which also violates the Prometheus naming
rules ([a-zA-Z_][a-zA-Z0-9_]*).

It mets OTel spec criteria; https://github.com/open-telemetry/opentelemetry-specification/blob/b2f923fb1650dde1f061507908b834035506a796/specification/compatibility/prometheus_and_openmetrics.md#metric-metadata-1

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)

[codecov]

Codecov Report

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

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #7033      +/-   ##
==========================================
+ Coverage   88.56%   88.58%   +0.02%     
==========================================
  Files         270      270              
  Lines       12894    12909      +15     
==========================================
+ Hits        11419    11435      +16     
+ Misses       1475     1474       -1     

Flag	Coverage Δ
unittests-Project-Experimental	88.47% <100.00%> (-0.04%)	⬇️
unittests-Project-Stable	88.39% <100.00%> (-0.10%)	⬇️
unittests-UnstableCoreLibraries-Experimental	41.19% <100.00%> (-0.48%)	⬇️

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

Files with missing lines	Coverage Δ
...ometheus.HttpListener/Internal/PrometheusMetric.cs	99.37% <100.00%> (+0.05%)	⬆️

... and 5 files with indirect coverage changes

[Copilot]

Pull request overview

This PR fixes malformed Prometheus metric names by sanitizing metric unit strings before appending them to metric names, addressing cases like Azure Cosmos DB units containing invalid characters (e.g., # RU) and handling multiple / characters in units.

Changes:

• Added unit sanitization logic and applied it when computing the unit suffix in PrometheusMetric.
• Updated/added unit-related unit tests, including multi-slash and special-character unit scenarios.
• Documented the fix in the Prometheus HttpListener and AspNetCore exporter changelogs.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.

File	Description
test/OpenTelemetry.Exporter.Prometheus.HttpListener.Tests/PrometheusMetricTests.cs	Updates expected name for multi-slash units and adds focused tests for unit sanitization behavior.
src/OpenTelemetry.Exporter.Prometheus.HttpListener/Internal/PrometheusMetric.cs	Introduces SanitizeMetricUnit and ensures units are sanitized before being appended to metric names.
src/OpenTelemetry.Exporter.Prometheus.HttpListener/CHANGELOG.md	Records the HttpListener exporter fix for invalid unit characters.
src/OpenTelemetry.Exporter.Prometheus.AspNetCore/CHANGELOG.md	Records the AspNetCore exporter fix for invalid unit characters.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[martincostello]

@nabutabu Please address the review comments from Copilot.

[Kielek]

@nabutabu, could you please check this comment occuring on the similar implementation?

Thank you for pointing this out. I wasn't aware of PR #7033 when I submitted this. After reviewing it, both PRs take the same
approach of sanitizing the unit string in GetUnit() before it's returned.

I'm happy to close this PR in favor of #7033 since it was submitted first and already has maintainer review.
I do notice one difference: my implementation also strips trailing underscores (e.g., "##/RU!" → "RU" vs "RU_" in #7033), which might be worth incorporating there.

I'll keep a closer eye on open PRs in the future. Happy to contribute on other open issues!

Originally posted by @Nik-Reddy in #7063 (comment)

[Nik-Reddy]

Hi @nabutabu, nice work on this PR! I took a closer look (I had independently worked on the same issue in #7063 before realizing yours was first) and have a couple of observations:

1. Trailing underscores not stripped: SanitizeMetricUnit strips the leading _ but not trailing ones. For example, "##/RU!" currently produces "RU_" instead of "RU". The original issue description by @tillig specifically expects "#_R.U." → "RU" (no trailing underscore). A simple TrimEnd('_') on the result would fix this.

2. StringBuilder allocation on valid input: As the Copilot review noted, the current implementation creates a StringBuilder even when the unit is already valid (e.g., "seconds"). The fast-path return sb?.ToString() ?? metricUnit only works if sb stays null for valid inputs, which it does for fully-valid strings, but worth double-checking the edge cases.

Happy to help with a follow-up commit if needed!

[Nik-Reddy]

I reviewed the SanitizeMetricUnit implementation and identified two concrete issues that need fixing before merge:

1. Trailing underscores not stripped

The method strips leading _ but not trailing ones:

// Current: only strips leading
return result.Length > 0 && result[0] == '_'
    ? result.Substring(1)
    : result;

This means:

• ""##/RU!"" → ""RU_"" (should be ""RU"")
• ""req!"" → ""req_"" (should be ""req"")

Per @tillig's original issue description: ""#_R.U."" → ""RU"" (no trailing underscore).

Fix — replace the return block with:

var result = sb?.ToString() ?? metricUnit;
return result.Trim('_');

This handles both leading AND trailing underscores in one call, and is simpler than the current leading-only check.

2. Test expectations need updating

Two test cases assert the wrong expected values (with trailing underscores):

• SanitizeMetricUnit_RemoveMultipleUnsupportedCharacters: ""##/RU!"" should expect ""RU"" not ""RU_""
• Name_UnitWithSpecialChars_Sanitized: ""req!"" should expect ""metric_req"" not ""metric_req_""

3. StringBuilder allocation (minor)

The Copilot review flagged that sb allocates even for valid input, but tracing through the logic: for fully-valid strings (e.g., ""seconds""), sb stays null and the fast-path return sb?.ToString() ?? metricUnit works correctly. So this is actually fine for the common case — no change needed here.

Happy to submit a follow-up PR with these fixes found to be helpful.

[Kielek]

@Nik-Reddy, if you are fine, you could provide a diff here to be included by us in this PR.
If there will be no issues on GH side, you should be mentioned as Co-authored by tags on the commit merge

[github-actions]

Thank you for your contribution @nabutabu! 🎉 We would like to hear from you about your experience contributing to OpenTelemetry by taking a few minutes to fill out this survey.