Skip to content

[receiver/hostmetrics] Add cputicks reader behind feature gate#47972

Open
salvatore-campagna wants to merge 28 commits into
open-telemetry:mainfrom
salvatore-campagna:fix/hostmetrics-cputicks-package
Open

[receiver/hostmetrics] Add cputicks reader behind feature gate#47972
salvatore-campagna wants to merge 28 commits into
open-telemetry:mainfrom
salvatore-campagna:fix/hostmetrics-cputicks-package

Conversation

@salvatore-campagna

@salvatore-campagna salvatore-campagna commented Apr 28, 2026

Copy link
Copy Markdown
Contributor

Summary

Follow-up to #46153 (precision improvements for memory, disk, and filesystem scrapers) and the approach discussed in #46177. The CPU scraper was excluded from #46153 because gopsutil converts raw integer jiffies from /proc/stat to float64 seconds before the scraper sees them. This shrinks operand magnitudes so that precision.Ratio rounds too aggressively; a 1-CPU/10s-interval machine gets only 1 significant digit, rounding 3% utilization to 0%.

As discussed in #46177, a gopsutil upstream fix wouldn't help here since the consumer still computes delta_user / delta_total as float64 / float64, producing 15-17 digits again. The fix needs to keep the data as uint64 throughout the pipeline. The new path is behind an alpha feature gate for opt-in testing before becoming the default.

This PR adds a Linux-only cputicks package and integrates it into the CPU scraper, combining the infrastructure and wiring proposed in #46177 into a single PR. The package is scoped to cpuscraper/internal/cputicks/ since it is only used by the CPU scraper. Linux-only is pragmatic: the vast majority of OTel collectors in production run on Linux, and macOS/Windows native readers can follow in separate PRs if needed.

The cputicks.Reader parses /proc/stat directly using bufio.Scanner + strconv.ParseUint, returning []Stat with raw uint64 fields. It handles variable column counts across kernel versions and supports rootPath for container environments where the host procfs is mounted at a non-default location.

To wire the new reader in without breaking existing behavior, the scraper's direct times + ucal fields are replaced with a single emitCPUMetrics function field that encapsulates the full read+record cycle. newGopsutilEmitter wraps the existing gopsutil+ucal logic unchanged; newCputicksEmitter uses cputicks.Reader with precision.Scale for tick-to-seconds conversion and precision.Ratio for utilization, preserving the true precision of the source data instead of producing 15-17 digit float64 artifacts.

The receiver.hostmetricsreceiver.UseCPUTicks alpha feature gate selects between the two emitters at construction time. When disabled (default) or on non-Linux platforms, behavior is unchanged and newGopsutilEmitter is used. Non-Linux platforms continue using gopsutil as noted in #46177 since reconstructing integer ticks from gopsutil's float64 output would be a lossy round-trip.

Stat.Total() excludes Guest and GuestNice because the Linux kernel already accounts them inside User and Nice respectively (#48024).

Testing

cd receiver/hostmetricsreceiver
go test ./internal/scraper/cpuscraper/...

Benchmarks

Reading /proc/stat directly does not introduce a performance penalty compared to gopsutil:

BenchmarkReadAll-8            147184    8216 ns/op    7992 B/op    33 allocs/op
BenchmarkReadAll_Gopsutil-8   130821    9200 ns/op    8737 B/op    46 allocs/op
cd receiver/hostmetricsreceiver
go test ./internal/scraper/cpuscraper/internal/cputicks/... -bench=. -benchmem

Ref #46177

Add a Linux-only cputicks package that reads raw uint64 tick counts
directly from /proc/stat, bypassing gopsutil's float64 conversion that
loses integer precision. Replace the scraper's direct times+ucal fields
with a pluggable emitCPUMetrics emitter, and wire the new cputicks path
behind the receiver.hostmetricsreceiver.UseCPUTicks alpha feature gate.

When enabled, CPU time and utilization metrics use precision.Scale and
precision.Ratio to preserve the true precision of the source data.
Non-Linux platforms continue using gopsutil unchanged.

Ref open-telemetry#46177
@salvatore-campagna salvatore-campagna force-pushed the fix/hostmetrics-cputicks-package branch from 533ca96 to 8fc2d67 Compare April 28, 2026 12:11
@salvatore-campagna salvatore-campagna marked this pull request as ready for review April 28, 2026 18:49
@salvatore-campagna

salvatore-campagna commented Apr 28, 2026

Copy link
Copy Markdown
Contributor Author

I think the supervisor-test is flaky.

@rogercoll rogercoll 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.

Nice work!

// Config relating to CPU Metric Scraper.
type Config struct {
metadata.MetricsBuilderConfig `mapstructure:",squash"`
rootPath string

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.

I don't have a strong opinion as it seems the filesystem scraper has a similar approach to use the rootPath, but an alternative would be extracting it from the context: https://github.com/salvatore-campagna/opentelemetry-collector-contrib/blob/fix/hostmetrics-cputicks-package/receiver/hostmetricsreceiver/internal/scraper/processscraper/process.go#L132

Any preference @braydonk @dmitryax ?

Comment thread receiver/hostmetricsreceiver/internal/scraper/cpuscraper/cpu_scraper.go Outdated
Comment on lines +182 to +192
findMetric := func(name string) pmetric.Metric {
for _, m := range metrics.All() {
if m.Name() == name {
return m
}
}
t.Fatalf("metric not found: %s", name)
return pmetric.Metric{}
}

findDP := func(metric pmetric.Metric, cpuName, state string) float64 {

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.

Are you familiar with the pkg/golden and pmetrictest packages? I think we can use it here to avoid this lookup functions (already implemented in pmetrictest)

See an example:
https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/fix/hostmetrics-cputicks-package/receiver/k8sclusterreceiver/internal/deployment/deployments_test.go#L62-L65

And this helper function to write the metrics on the first run: golden.WriteMetrics(t, expectedFile, md)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks for pointing me to golden and pmetrictest, I wasn't aware of them.

@rogercoll rogercoll 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.

LGTM! It would be great having a storage analysis of this improvement before moving the feature gate to beta 🎉

Comment thread receiver/hostmetricsreceiver/internal/scraper/cpuscraper/cpu_scraper_linux.go Outdated
@rogercoll

Copy link
Copy Markdown
Contributor

@dmitryax @braydonk Any thoughts on these changes? (I am primarily in favor as removes the float64 arithmetic noise which ends up reducing the storage costs while maintaining the precision)

@github-actions

Copy link
Copy Markdown
Contributor

This PR was marked stale due to lack of activity. It will be closed in 14 days.

@github-actions github-actions Bot added the Stale label May 29, 2026
@dmitryax

dmitryax commented May 29, 2026

Copy link
Copy Markdown
Member

We touched on this in the System Semantic Conventions SIG yesterday.

I have to push back on this. I should have flagged concerns on #46153 before approving it, and this PR builds further on the same approach while adding much more complexity. I don't think this complexity can be justified.

From my point of view, rounding a float's mantissa is meaningless by design as it's encoded as binary fractions, not as decimals. The PRs call the additional digits "arithmetic noise", but we cannot remove that noise we just push it further away behind more zeroes, e.g. 3.3333333333...4 becomes something like 3.330000000000...03232434

If users want values rounded this way, the transform processor is a better place, where it can be implement an OTTL function. Receivers should always optimize for accuracy of the emitted values not for their representation.

I'd even suggest we also revisit #46153. @braydonk WDYT?

@github-actions github-actions Bot removed the Stale label May 30, 2026
@salvatore-campagna

salvatore-campagna commented Jun 3, 2026

Copy link
Copy Markdown
Contributor Author

We touched on this in the System Semantic Conventions SIG yesterday.

I have to push back on this. I should have flagged concerns on #46153 before approving it, and this PR builds further on the same approach while adding much more complexity. I don't think this complexity can be justified.

From my point of view, rounding a float's mantissa is meaningless by design as it's encoded as binary fractions, not as decimals. The PRs call the additional digits "arithmetic noise", but we cannot remove that noise we just push it further away behind more zeroes, e.g. 3.3333333333...4 becomes something like 3.330000000000...03232434

If users want values rounded this way, the transform processor is a better place, where it can be implement an OTTL function. Receivers should always optimize for accuracy of the emitted values not for their representation.

I'd even suggest we also revisit #46153. @braydonk WDYT?

Thanks for raising this and for the SIG discussion context.

I think the numbers help separate the two phenomena here.

IEEE 754 representation (unavoidable). Absolutely right that some decimal values cannot be represented exactly in binary floating point. No code can fix that.

Early conversion of source counters in the processing pipeline (avoidable). The Linux kernel exposes CPU tick counts as exact integers in /proc/stat. gopsutil's parseStatLine calls strconv.ParseFloat on these integer values, divides by ClocksPerSec(100), and stores the result in TimesStat fields, which are float64. There is no raw integer API. This converts the counters to scaled float64 values before the delta computation even starts:

/proc/stat: "987654321" (exact uint64 from kernel)
  gopsutil: strconv.ParseFloat   ==> float64
  gopsutil: / ClocksPerSec(100)  ==> float64 9876543.21
  ucal:     float64 - float64    ==> delta
  ucal:     float64 / float64    ==> utilization

This PR reads /proc/stat directly and preserves the source counters through the delta computation:

/proc/stat: "987654321" (same exact uint64 from kernel)
  cputicks: strconv.ParseUint    ==> uint64
  delta:    uint64 - uint64      ==> 333
  ratio:    float64(333) / float64(3333)

The source data in this case is a set of integer tick counters. This change keeps those counters as integers through the delta computation and only converts to float64 when the utilization ratio is computed.

Reproducible evidence. The following is generated by a Go program that simulates both paths with the same tick delta (333/3333) at five consecutive absolute positions:

Scrape 1: gopsutil=0.09990999162122860
Scrape 2: gopsutil=0.09990999001238611
Scrape 3: gopsutil=0.09990999090654519
Scrape 4: gopsutil=0.09990999072706951
Scrape 5: gopsutil=0.09990999162122860

Direct integer-delta computation:
float64(333)/float64(3333) = 0.09990999099909990

Two observations from this data:

  1. The gopsutil path produces different utilization values for the same tick delta depending on the absolute counter position.
  2. Computing the ratio directly from the integer deltas produces the same value for all tested positions.

The important point here is not the final decimal representation. The important point is that the two computation paths produce different results because one computes the delta from scaled float64 values and the other computes the delta from the original integer counters.

Regarding downstream processing.

A transform processor operates on the emitted utilization value. It can round or otherwise transform that value, but it does not have access to the original integer counters or integer deltas.

Once the receiver emits:

cpu.utilization = 0.09990999090654519

the information that the ratio originated from:

333 / 3333

is no longer available downstream.

For that reason, computing utilization from integer deltas versus scaled float64 values is a receiver-side concern. A downstream processor can change the representation of the emitted value, but it cannot reconstruct the original integer-delta computation after the fact.

precision.Ratio is a separate layer on top of this. Even if precision.Ratio were removed entirely, the difference between the two computation paths described above would still exist.

Problem vs. solution.

The data above shows a measurable difference between the current computation path and one that preserves integer counters through the delta computation. The approach was discussed in #46177 and this PR implements it behind an alpha feature gate.

If there are concerns about code structure, package layout, or maintenance cost, I am happy to discuss those.

Before discussing implementation details, do we agree on the following observations?

  1. The two computation paths produce different utilization values for the same tick deltas.
  2. The difference is observed in the path where counters are first converted to scaled float64 values before delta computation.
  3. In the simulation above, computing utilization directly from integer deltas produces the same result across all tested counter positions.

If any of those observations are not supported by the data, I am happy to revisit the analysis.

@salvatore-campagna

Copy link
Copy Markdown
Contributor Author

For context on why this is not handled upstream: the only gopsutil fix that would work is a new API exposing raw integer ticks, with per-platform implementations across all supported OSes. That was discussed in #46177. gopsutil does not expose the raw integers and there is no indication that it will. The cputicks reader exists because of that constraint.

@rogercoll

Copy link
Copy Markdown
Contributor

During the System SIG on 04/06/2026 we discussed whether https://github.com/open-telemetry/opentelemetrycollector-contrib/pull/46153 could be moved into a processor or OTTL function. The core challenge is that the PR does not apply fixed-decimal truncation, it derives the correct number of significant digits from the magnitude of the original integer inputs, so the rounding is context-aware and cannot be reconstructed once the float64 is emitted.

Two cases:

  • precision.Ratio: used for utilization ratios, it could in principle become an OTTL function, but it would require the receiver to expose raw usage integers, so the function has both operands available. This means every utilization metric would need explicit pipeline configuration (usage values) to compute and store the result into a new utilization metric, making default setups verbose and leaving users who don't opt in with the values with false precision.
  • precision.Scale: cannot be moved downstream. The rounding corrects noise introduced by the receiver's own time-unit conversion (e.g. ticks → seconds). That information is lost once the float64 is emitted, a processor has no way to recover it. The only alternative would be for the receiver to emit raw tick counts instead of the actual metric type, which changes the receiver's contract with semantic conventions (metric type).

@github-actions

Copy link
Copy Markdown
Contributor

This PR was marked stale due to lack of activity. It will be closed in 14 days.

@github-actions github-actions Bot added the Stale label Jun 20, 2026
@rogercoll

Copy link
Copy Markdown
Contributor

Not stale. During System WG SIG on 04/06/2026 it has discussed if there was any gopsutil feature to retrieve the raw cputicks using the package. It seems that now at the moment, but it could be extended using the Ex structs strategy, see https://github.com/shirou/gopsutil#ex-struct-from-v4245 As this PR is under a feature flag, the cpu ticks could be added into an Ex gopsutil struct in a follow-up PR

@rogercoll rogercoll removed the Stale label Jun 22, 2026
salvatore-campagna and others added 3 commits June 22, 2026 10:40
Resolve conflicts in:
- receiver/hostmetricsreceiver/go.mod: bump to v0.154.0 / collector v1.60.1,
  keep tklauser/go-sysconf and pkg/golden direct deps
- receiver/hostmetricsreceiver/metadata.yaml: keep UseCPUTicks feature gate
  alongside upstream skip_strict_validation addition
salvatore-campagna and others added 6 commits July 9, 2026 11:07
Add skip_strict_validation: true to the receiver.hostmetricsreceiver.UseCPUTicks
feature gate to match how the other two pre-existing gates in this file
(hostmetrics.process.bootTimeCache and receiver.hostmetricsreceiver.UseLinuxMemAvailable)
bypass the new mdatagen prefix check. The required prefix receiver.host_metrics.
cannot be used as the ID because the underscore character is not allowed in
feature gate IDs.
…change

PR open-telemetry#49161 made the cpu attribute opt-in for system.cpu.time and
system.cpu.utilization and enabled system.cpu.logical.count by default.
The cputicks tests need to explicitly opt back in to per-CPU attributes
(since that is the core behavior being tested) and disable the logical
count metric to keep metric count assertions correct.
Apply gci grouping (stdlib before third-party) to generated_package_test.go
and generated_logs_test.go, which mdatagen produced with mixed import groups.
Required for the porto-and-gci CI shard.
@rogercoll

Copy link
Copy Markdown
Contributor

Friendly reminder for other codeowners @braydonk and @dmitryax

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants