refactor(metered): align metrics with dotnet/runtime#124140 API-approved spec#173
refactor(metered): align metrics with dotnet/runtime#124140 API-approved spec#173
Conversation
…view The API review (bartonjs, 2026-03-03) approved the meter name as Microsoft.Extensions.Caching.Memory.MemoryCache to match the type name. Updated MeterName constant from 'Microsoft.Extensions.Caching.Memory' to 'Microsoft.Extensions.Caching.Memory.MemoryCache'. All test references use MeteredMemoryCache.MeterName constant so they auto-align with this change. Ref: dotnet/runtime#124140 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Documentation referenced stale metric names (cache_hits_total, cache_misses_total) from before the codebase aligned with dotnet/runtime#124140. Updated all references across 12 files: - Replaced separate cache_hits_total/cache_misses_total with unified cache.requests instrument (cache.request.type dimension) - Fixed meter name from CacheImplementations.MeteredMemoryCache to Microsoft.Extensions.Caching.Memory.MemoryCache - Updated all PromQL queries to use cache_requests_total with cache_request_type label selector - Updated Prometheus exposition format examples Files: README.md, docs/ApiReference.md, docs/BestPractices.md, docs/DocumentationReview.md, docs/FAQ.md, docs/MeteredMemoryCache.md, docs/MigrationGuide.md, docs/MultiCacheScenarios.md, docs/OpenTelemetryIntegration.md, docs/Troubleshooting.md, examples/AspNetCore/docs/README-HTTP-Tests.md, .github/copilot-instructions.md Ref: dotnet/runtime#124140 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request refactors the metrics emitted by Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![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 effectively refactors the metrics implementation to align with the dotnet/runtime#124140 specification. The changes, including updating the meter name and consolidating cache hit/miss metrics into a single cache.requests instrument, are applied consistently across both the source code and the extensive documentation. The documentation updates are thorough, correctly reflecting the new metric names, types, and tags. I've found one minor inconsistency in a PromQL query within the documentation, for which I've provided a suggestion to improve consistency across the project's guides.
|
LGTM |
jmathes
left a comment
jmathes
left a comment
There was a problem hiding this comment.
I think the missing test cases are worth adding back in if it's idiomatic to be that rigorous in these things. Otherwise I only have questions which might be comments but I'm not sure.
- Standardize all PromQL queries with by (cache_name) per BCL cache.name dimension - Improve test assertions in BestPractices.md and MigrationGuide.md to verify cache.request.type tag values (hit/miss) per dotnet/runtime#124140 spec - Add hit/miss diagnostic split example in Troubleshooting.md - Update FAQ.md PromQL queries for consistency Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
|
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
Confirmed: I re-read .github/copilot-instructions.md in full before proceeding.
Aligns MeteredMemoryCache with the dotnet/runtime#124140 OpenTelemetry semantic conventions by standardizing the meter name and updating documentation to reflect the current instrument set and naming.
Changes:
- Update
MeteredMemoryCache.MeterNametoMicrosoft.Extensions.Caching.Memory.MemoryCache. - Refresh README and docs to use
cache.requests/cache.evictions/cache.entries/cache.estimated_size(and corresponding Prometheus-exported names). - Update example guidance in docs and
.github/copilot-instructions.mdto reference the new instrument names.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| src/CacheImplementations/MeteredMemoryCache.cs | Updates MeterName constant to the BCL-aligned meter name. |
| README.md | Updates public-facing instrument list and AddMeter example. |
| examples/AspNetCore/docs/README-HTTP-Tests.md | Updates described metric names for HTTP tests. |
| docs/Troubleshooting.md | Updates troubleshooting sample to query cache.requests and split by cache.request.type. |
| docs/OpenTelemetryIntegration.md | Updates Prometheus sample, views, and PromQL examples for cache.requests. |
| docs/MultiCacheScenarios.md | Updates PromQL examples to use cache_requests_total patterns. |
| docs/MigrationGuide.md | Updates migration examples and validation snippets to use cache.requests. |
| docs/MeteredMemoryCache.md | Updates metric tables and eviction tracking explanation. |
| docs/FAQ.md | Updates FAQ metric descriptions and example queries. |
| docs/DocumentationReview.md | Declares metric naming standardization as resolved and documents meter name. |
| docs/BestPractices.md | Updates dashboard/alert queries and test snippets to new metric names. |
| docs/ApiReference.md | Updates API reference metric sections to new instrument names. |
| .github/copilot-instructions.md | Updates internal guidance examples to reference cache.requests / cache.evictions. |
You can also share your feedback on Copilot code review. Take the survey.
…e tag docs Address copilot-pull-request-reviewer findings: - MeteredMemoryCache.md: Clarify cache.evictions excludes Removed/Replaced - README.md: Fix cache.estimated_size condition (TrackStatistics, not SizeLimit) - OpenTelemetryIntegration.md: Same estimated_size condition fix - ApiReference.md: cache.name is always present (defaults to Default), cache.evictions excludes explicit removals, cache.estimated_size condition corrected Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Panel of 5 audit agents reviewed all documentation against MeteredMemoryCache.cs implementation. 37 inaccuracies found and fixed across 9 files. Critical fixes (10): - MeteredMemoryCache.md: 5x wrong AddMeter name 'MyApp.Cache' (metrics silently lost) - MeteredMemoryCache.md: phantom meterName parameter on 2 API signatures + example - ApiReference.md: Remove() falsely claimed cache.evictions emitted (excluded reason) - Troubleshooting.md: pseudocode disclaimer on non-compilable test harness High fixes (11): - Prometheus _bytes suffix on cache_estimated_size (OTel unit 'By' conversion) - Alert rules dropping cache_name label but annotations referencing it (3 files) - cache.estimated_size condition: TrackStatistics, not SizeLimit (4 files) - ApiReference: 2 missing IMeterFactory constructor docs, Name nullability - Eviction semantics: exclusion logic (not positive match) across all docs Medium/Low fixes (16): - cache.name tag: 'always present, defaults to Default' (was 'if specified') - HELP text alignment with code descriptions - Overhead range, allocation counts, thread-safety mechanism Test coverage (new file): - tests/Unit/BclAdoptionCoverageTests.cs: 12 new tests filling coverage gaps including MeterName constant, eviction reasons, entry commit semantics, estimated_size registration conditions, disposal, GetCurrentStatistics Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Summary
Aligns codebase metrics with the dotnet/runtime#124140 API-approved specification for
MemoryCachemetrics.Changes
Source (1 file)
Updated
MeterNameconstant to match the API review outcome (bartonjs, 2026-03-03):Microsoft.Extensions.Caching.MemoryMicrosoft.Extensions.Caching.Memory.MemoryCacheAll 14 test references use
MeteredMemoryCache.MeterNameconstant, so tests auto-aligned. No test changes required.Documentation (12 files)
Updated metric name references across all documentation:
cache_hits_total/cache_misses_totalcache.requestswithcache.request.typedimensionCacheImplementations.MeteredMemoryCacheMicrosoft.Extensions.Caching.Memory.MemoryCachecache_evictions_total(in OTel contexts)cache.evictionsAdditional fixes from multi-round review:
reasontag claims oncache.evictions(tag does not exist in implementation)ExplicitBucketHistogramConfigurationonObservableCounterwithMetricStreamConfigurationby (cache_name)on bothsum()callscache_estimated_sizeto Prometheus sample outputInstruments (unchanged, already aligned)
cache.requestscache.name,cache.request.type(hit/miss)cache.evictionscache.namecache.entriescache.namecache.estimated_sizecache.nameBreaking Change
Consumers subscribing to the old meter name via hardcoded string
"Microsoft.Extensions.Caching.Memory"will need to update to"Microsoft.Extensions.Caching.Memory.MemoryCache"or useMeteredMemoryCache.MeterName.Validation
dotnet format[PASS]dotnet build -c Release[PASS] (0 warnings)dotnet test -c Release[PASS] (243/243)References