fix: align meter name and UCUM units with dotnet/runtime#124140 and OTel semconv#191
fix: align meter name and UCUM units with dotnet/runtime#124140 and OTel semconv#191jmathes wants to merge 1 commit intorjmurillo:mainfrom
Conversation
…Tel semconv - Fix meter name from 'Microsoft.Extensions.Caching.Memory.MemoryCache' to 'Microsoft.Extensions.Caching.Memory' per the API proposal spec - Fix UCUM unit annotations from plural to singular form per the pattern established by stable .NET runtime metrics - Add unit documentation to ApiReference.md - Add 'Combining Cache and Runtime Metrics' section to docs - Update all doc references to the old meter name Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
There was a problem hiding this comment.
Pull request overview
I have re-read .github/copilot-instructions.md in full.
This PR updates MeteredMemoryCache metrics metadata (meter name + UCUM units) to match the stated dotnet/runtime proposal and OTel .NET runtime semantic conventions, and refreshes documentation/examples accordingly.
Changes:
- Changed
MeteredMemoryCache.MeterNametoMicrosoft.Extensions.Caching.Memoryand updated docs/tests/examples to match. - Updated UCUM unit annotations for count instruments from plural to singular (
{request},{eviction},{entry}). - Expanded/standardized metrics documentation (units) and added guidance for subscribing to both cache and runtime meters.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/Unit/BclAdoptionCoverageTests.cs | Updates assertion for the new MeterName constant value. |
| src/CacheImplementations/MeteredMemoryCache.cs | Updates MeterName constant and UCUM unit strings for observable instruments. |
| README.md | Updates Quick Start .AddMeter(...) example to the new meter name. |
| docs/OpenTelemetryIntegration.md | Updates meter name and adds a section describing subscribing to System.Runtime alongside cache metrics. |
| docs/MeteredMemoryCache.md | Updates all snippets referencing the meter name (DI + manual meter creation + factory usage). |
| docs/DocumentationReview.md | Updates documented meter name reference. |
| docs/ContainerCacheSizing.md | Updates the OpenTelemetry .AddMeter(...) snippet to the new meter name. |
| docs/ApiReference.md | Adds explicit Unit lines for instruments and standardizes cache.estimated_size unit documentation. |
| #region 1a — MeterName constant value | ||
|
|
||
| [Fact] | ||
| public void MeterName_Constant_HasExpectedValue() | ||
| { | ||
| Assert.Equal("Microsoft.Extensions.Caching.Memory.MemoryCache", MeteredMemoryCache.MeterName); | ||
| Assert.Equal("Microsoft.Extensions.Caching.Memory", MeteredMemoryCache.MeterName); | ||
| } | ||
|
|
||
| #endregion |
There was a problem hiding this comment.
#region / #endregion directives are present in this test class, but this repository's policy prohibits introducing or keeping #region blocks in code. Please remove these directives (and the rest of the #region blocks in this file) and rely on file structure/comments or split into multiple files/classes if logical grouping is needed. (Ref: .github/copilot-instructions.md lines 1187-1194.)
| The `System.Runtime` meter emits metrics defined by the | ||
| [OTel .NET runtime semantic conventions](https://opentelemetry.io/docs/specs/semconv/runtime/dotnet-metrics/), | ||
| including `dotnet.gc.collections`, `dotnet.process.memory.working_set`, | ||
| `dotnet.thread_pool.queue.length`, and others. Available in .NET 9+ without additional packages. |
There was a problem hiding this comment.
The statement "Available in .NET 9+ without additional packages" is likely incorrect/misleading. The System.Runtime meter exists across multiple .NET versions, and this repo targets net10.0 (per Directory.Build.props). Consider removing the version claim or replacing it with an accurate minimum (e.g., "Available in modern .NET runtimes" / "net6.0+"), and if older TFMs require OpenTelemetry.Instrumentation.Runtime, mention that explicitly.
| `dotnet.thread_pool.queue.length`, and others. Available in .NET 9+ without additional packages. | |
| `dotnet.thread_pool.queue.length`, and others. It is available in modern .NET runtimes without additional packages; for older target frameworks you can use the `OpenTelemetry.Instrumentation.Runtime` package to collect similar metrics. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the OpenTelemetry meter name to Microsoft.Extensions.Caching.Memory across the codebase and documentation. It also standardizes metric units to singular forms (e.g., {request} instead of {requests}) and adds a new section to the OpenTelemetry integration guide regarding runtime metrics. Feedback was provided to correct the documentation to reflect that System.Runtime metrics are available starting from .NET 8 rather than .NET 9.
| The `System.Runtime` meter emits metrics defined by the | ||
| [OTel .NET runtime semantic conventions](https://opentelemetry.io/docs/specs/semconv/runtime/dotnet-metrics/), | ||
| including `dotnet.gc.collections`, `dotnet.process.memory.working_set`, | ||
| `dotnet.thread_pool.queue.length`, and others. Available in .NET 9+ without additional packages. |
There was a problem hiding this comment.
The System.Runtime meter and its associated metrics (like dotnet.gc.collections) were actually introduced in .NET 8. While .NET 9 expanded the set of available metrics, the basic observability described here is available to .NET 8 users as well. Consider updating the documentation to reflect .NET 8+ support to avoid excluding users on the previous LTS version.
| `dotnet.thread_pool.queue.length`, and others. Available in .NET 9+ without additional packages. | |
| including dotnet.gc.collections, dotnet.process.memory.working_set, | |
| dotnet.thread_pool.queue.length, and others. Available in .NET 8+ without additional packages. |
Summary
Aligns MeteredMemoryCache meter name and UCUM unit annotations with the dotnet/runtime#124140 API proposal and the OTel .NET runtime semantic conventions. Adds documentation for combining cache metrics with .NET runtime metrics.
Background
The library implements the cache metrics proposal from dotnet/runtime#124140. Two discrepancies existed between the implementation and the spec:
.MemoryCachesuffix not present in the proposal.Changes
1. Meter name:
Microsoft.Extensions.Caching.Memory.MemoryCache->Microsoft.Extensions.Caching.MemoryWhy: The dotnet/runtime#124140 API proposal specifies:
The implementation had
Microsoft.Extensions.Caching.Memory.MemoryCache, meaning consumers subscribing to the spec-correct meter name would receive zero metrics.Files changed:
src/CacheImplementations/MeteredMemoryCache.cs(line 305):MeterNameconstanttests/Unit/BclAdoptionCoverageTests.cs(line 24): assertion updated to matchREADME.md(line 133):.AddMeter()call in Quick Startdocs/ContainerCacheSizing.md(line 153):.AddMeter()calldocs/DocumentationReview.md(line 68): meter name referencedocs/MeteredMemoryCache.md(lines 55, 77, 99, 128, 403, 450, 458, 548): all.AddMeter(),new Meter(), andmeterFactory.Create()callsdocs/OpenTelemetryIntegration.md(line 262):.AddMeter()callTest and example files that reference
MeteredMemoryCache.MeterName(the constant) inherit the change automatically.2. UCUM units: plural -> singular
Why: The OTel .NET runtime semantic conventions use singular UCUM annotations consistently across all stable metrics:
dotnet.gc.collectionsuses{collection}, not{collections}dotnet.thread_pool.thread.countuses{thread}, not{threads}dotnet.jit.compiled_methodsuses{method}, not{methods}dotnet.monitor.lock_contentionsuses{contention}, not{contentions}dotnet.exceptionsuses{exception}, not{exceptions}File changed:
src/CacheImplementations/MeteredMemoryCache.cscache.requests(line 339){requests}{request}cache.evictions(line 344){evictions}{eviction}cache.entries(line 349){entries}{entry}3. Unit documentation added to ApiReference.md
Why: Three of four metric instruments lacked explicit unit documentation. The fourth (
cache.estimated_size) had its unit embedded in the description text rather than a separate field. Standardized all four to use a**Unit:**line.File changed:
docs/ApiReference.mdcache.requests: added**Unit:** \{request}``cache.evictions: added**Unit:** \{eviction}``cache.entries: added**Unit:** \{entry}``cache.estimated_size: extracted**Unit:** \By`` from description into its own line4. "Combining Cache and Runtime Metrics" documentation section
Why: The OTel .NET runtime semantic conventions define metrics (
dotnet.gc.*,dotnet.process.*,dotnet.thread_pool.*) that are valuable for diagnosing cache behavior (eviction spikes correlating with GC pressure, thread pool saturation during concurrent access). No existing documentation showed how to subscribe to both meters together.File changed:
docs/OpenTelemetryIntegration.md-- new section after "Periodic Export Configuration" with a code snippet showing.AddMeter("System.Runtime")alongside the cache meter.Testing
git grepthat no references to the old meter name remain (the two remainingMicrosoft.Extensions.Caching.Memory.MemoryCachehits inCacheStatistics.csare<see cref>references to theMemoryCacheStatisticsTYPE, not the meter).BclAdoptionCoverageTests.MeterName_Constant_HasExpectedValuetest assertion was updated to match.Additional Notes
Breaking change: Consumers who filter metrics by the old meter name (
Microsoft.Extensions.Caching.Memory.MemoryCache) will stop receiving cache metrics. This is intentional -- the old name did not match the spec, and aligning now prevents a harder migration when .NET ships native MemoryCache metrics per dotnet/runtime#124140.