Repository navigation
Fix container.cpu.time on Linux: report it as a counter in seconds - #7840
Open
evgenyfedorov2 wants to merge 3 commits into
Open
evgenyfedorov2 wants to merge 3 commits into
evgenyfedorov2 wants to merge 3 commits into
Conversation
With UseLinuxCalculationV2, container.cpu.time was an ObservableGauge with unit "1" that reported the host's total /proc/stat time as system time and the number of cores currently in use as user time. Computing the latter also consumed the state shared with the container.cpu.limit.utilization and container.cpu.request.utilization gauges. It is now an ObservableCounter in seconds, as on Windows, reporting the container's cumulative user and system CPU time from the cgroup v2 cpu.stat file (user_usec and system_usec), falling back to the host's /proc/stat values when cpu.stat doesn't exist. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Cumulative CPU-time conversion can overflow signed nanoseconds and produce non-monotonic counter values.
1 open finding
What changed in this PR
Aligns Linux cgroup v2 container.cpu.time with the documented cross-platform metric contract.
Changes:
- Reports cumulative user/system CPU seconds from
cpu.stat. - Uses an
ObservableCounter<double>with units. - Adds parser, provider, isolation, and acceptance coverage.
| File | Description |
|---|---|
src/.../ILinuxUtilizationParser.cs |
Adds the user/system CPU-time parser contract. |
src/.../LinuxUtilizationParserCgroupV2.cs |
Parses cgroup and host CPU times. |
src/.../LinuxUtilizationParserCgroupV1.cs |
Marks the v2 operation unsupported. |
src/.../LinuxUtilizationProvider.cs |
Emits cumulative CPU seconds as a counter. |
test/.../DummyLinuxUtilizationParser.cs |
Implements the new test contract. |
test/.../LinuxUtilizationParserCgroupV2Tests.cs |
Covers parsing, fallback, and malformed input. |
test/.../LinuxUtilizationParserCgroupV1Tests.cs |
Verifies unsupported behavior. |
test/.../LinuxUtilizationProviderTests.cs |
Verifies metadata, values, and callback isolation. |
test/.../AcceptanceTest.cs |
Updates end-to-end cgroup v2 expectations. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+655
to
+657
| // In cgroup v2, the Units are microseconds for user_usec and system_usec. | ||
| // We multiply by 1000 to convert to nanoseconds. | ||
| return (userMicroseconds * Thousand, systemMicroseconds * Thousand); |
evgenyfedorov2
marked this pull request as draft
October 14, 2026 10:30
With UseLinuxCalculationV2, container.cpu.time was an ObservableGauge with unit "1", and cpu.mode=user reported the number of cores in use (CpuUtilizationV2), which also changed the state used by the container.cpu.limit/request.utilization gauges. Restore the design from dotnet#5806: an ObservableCounter with unit "s", where cpu.mode=user is the container's cumulative CPU time from the cgroup cpu.stat. cpu.mode=system is unchanged. This replaces the user_usec/system_usec approach of the previous commit. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
evgenyfedorov2
changed the title
Collaborator
🎉 Good job! The coverage increased 🎉
Full code coverage report: https://dev.azure.com/dnceng-public/public/_build/results?buildId=1634858&view=codecoverage-tab |
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
evgenyfedorov2
marked this pull request as ready for review
October 14, 2026 11:53
Collaborator
🎉 Good job! The coverage increased 🎉
Full code coverage report: https://dev.azure.com/dnceng-public/public/_build/results?buildId=1635002&view=codecoverage-tab |
amadeuszl
reviewed
Oct 15, 2026
| unit: "1"); | ||
|
|
||
| _ = meter.CreateObservableGauge( | ||
| _ = meter.CreateObservableCounter( |
Contributor
There was a problem hiding this comment.
It depends on backend but its a breaking a change, are we good as its supposed to be released in .net 11?
Member
Author
There was a problem hiding this comment.
do you remember or understand how we could have missed that? I personally don't remember and the incorrect instrument type looks like a really silly mistake. I couldn't find any previous discussions about instrument types here either.
I don't have a strong preference as to when to merge and release it. Can very well wait and release with 11.1, for instance.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Changes to
container.cpu.timeinLinuxUtilizationProvider:ObservableCounterwith unitsinstead ofObservableGaugewith unit1. Cumulative CPU time only increases, so it's a counter in seconds. That's how OpenTelemetry definescontainer.cpu.timeand how Windows registers it (same unit and description).cpu.mode=useris CPU time in seconds instead of CPU cores in use. The value came fromCpuUtilizationV2(), which returns the number of CPU cores in use: a rate, not a time. Now it'susage_usecfrom the process's cgroupcpu.stat(the container's cumulative CPU time), converted to seconds.Microsoft Reviewers: Open in CodeFlow