Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[processor/resourcedetection] failed to convert cpuinfo family to integer #29025

Closed
povilasv opened this issue Nov 8, 2023 · 5 comments · Fixed by #29152
Closed

[processor/resourcedetection] failed to convert cpuinfo family to integer #29025

povilasv opened this issue Nov 8, 2023 · 5 comments · Fixed by #29152
Labels
bug Something isn't working priority:p2 Medium processor/resourcedetection Resource detection processor

Comments

@povilasv
Copy link
Contributor

povilasv commented Nov 8, 2023

Component(s)

processor/resourcedetection

What happened?

Description

system resource detection fails with:

{"level":"warn","ts":1699364976.361069,"caller":"system/system.go:102","msg":"failed to get host cpuinfo","kind":"processor","name":"resourcedetection/env","pipeline":"metrics","error":"failed to convert cpuinfo family to integer: strconv.ParseInt: parsing \"POWER\": invalid syntax"}

The issue is that system detector expects the family to be int in:

family, err := strconv.ParseInt(cpuInfo.Family, 10, 64)

But the actual library returns POWER string in certain cases:

SEE: https://github.com/shirou/gopsutil/blob/e74324b6a726997ce756b8f79dbbd7a3a0999ba0/cpu/cpu_linux.go#L264

Also looking at this repo which has a lot of example of cpuinfo, there seems to be a lot of families that are not numbers:

https://github.com/search?q=repo%3Arandombit%2Fcpuinfo%20Family&type=code

Steps to Reproduce

Expected Result

No error

Actual Result

Collector version

v0.88.0

Environment information

No response

OpenTelemetry Collector configuration

No response

Log output

No response

Additional context

I believe we need to change host.cpu.family to be a string resource attribute instead of int and get rid of parsing.

@povilasv povilasv added bug Something isn't working needs triage New item requiring triage labels Nov 8, 2023
@github-actions github-actions bot added the processor/resourcedetection Resource detection processor label Nov 8, 2023
Copy link
Contributor

github-actions bot commented Nov 8, 2023

Pinging code owners:

See Adding Labels via Comments if you do not have permissions to add labels yourself.

@povilasv povilasv changed the title [processor/resourcedetection] fix failed to convert cpuinfo family to integer [processor/resourcedetection] failed to convert cpuinfo family to integer Nov 8, 2023
@mx-psi
Copy link
Member

mx-psi commented Nov 8, 2023

This is actually wrong at the spec level https://github.com/open-telemetry/semantic-conventions/blob/main/docs/resource/host.md cc @ChrsMark I think we should change the type to string

@mx-psi
Copy link
Member

mx-psi commented Nov 8, 2023

@mx-psi mx-psi added priority:p2 Medium and removed needs triage New item requiring triage labels Nov 8, 2023
@povilasv
Copy link
Contributor Author

povilasv commented Nov 9, 2023

Thank you!

@mx-psi
Copy link
Member

mx-psi commented Nov 13, 2023

@povilasv I filed #29152 for this, PTAL

mx-psi added a commit that referenced this issue Nov 13, 2023
…odel to string (#29152)

**Description:** 

Add feature gate to change family and CPU model to string, to adapt to
open-telemetry/semantic-conventions/issues/495.

**Link to tracking Issue:** Fixes #29025

**Testing:** Tested with the sample configuration on #26533.
RoryCrispin pushed a commit to ClickHouse/opentelemetry-collector-contrib that referenced this issue Nov 24, 2023
…odel to string (open-telemetry#29152)

**Description:** 

Add feature gate to change family and CPU model to string, to adapt to
open-telemetry/semantic-conventions/issues/495.

**Link to tracking Issue:** Fixes open-telemetry#29025

**Testing:** Tested with the sample configuration on open-telemetry#26533.
mx-psi added a commit that referenced this issue Nov 30, 2023
…ostCPUModelAndFamilyAsString` to beta (#29462)

**Description:** 

Follow up to #29152. The feature gate has been in alpha for two releases
(v0.89.0 and v0.90.0).

**Link to tracking Issue:** Relates to #29025 and to
open-telemetry/semantic-conventions/issues/495.

**Testing:** Tested with the sample configuration on #26533.
mx-psi added a commit that referenced this issue May 17, 2024
…PUModelAndFamilyAsString` feature gate to stable (#33077)

**Description:** <Describe what has changed.>
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue.
Ex. Adding a feature - Explain what this achieves.-->

Follows #29152 and #29462. The feature gate has been in beta since
v0.91.0.

**Link to tracking Issue:** Relates to
#29025
and to
open-telemetry/semantic-conventions#495
cparkins pushed a commit to AmadeusITGroup/opentelemetry-collector-contrib that referenced this issue Jul 11, 2024
…PUModelAndFamilyAsString` feature gate to stable (open-telemetry#33077)

**Description:** <Describe what has changed.>
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue.
Ex. Adding a feature - Explain what this achieves.-->

Follows open-telemetry#29152 and open-telemetry#29462. The feature gate has been in beta since
v0.91.0.

**Link to tracking Issue:** Relates to
open-telemetry#29025
and to
open-telemetry/semantic-conventions#495
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working priority:p2 Medium processor/resourcedetection Resource detection processor
Projects
None yet
2 participants