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

[resourcedetection] cpuinfo ignores the configured timeout in the resourcedetection processor #33771

Closed
cwegener opened this issue Jun 26, 2024 · 5 comments · Fixed by #33774
Closed
Labels
bug Something isn't working os:windows priority:p2 Medium processor/resourcedetection Resource detection processor

Comments

@cwegener
Copy link
Contributor

Component(s)

processor/resourcedetection, processor/resourcedetection/internal/system

What happened?

Description

The host cpuinfo attributes introduced in #26533 use the cpu.Info() call from gopsutil.

cpu.Info() ignores any timeouts that a user configures in the resourcedetection processor, as it is simply calling cpu.InfoWithContext(context.Background) 1

func Info() ([]InfoStat, error) {
	return InfoWithContext(context.Background())
}

Steps to Reproduce

N/A - A repro would require a really elaborate set up with a reliable way of slowing down COM calls in windows. Outside of production systems, I don't know how to artificially slow down COM calls ...

Expected Result

cpuinfo honors the configured timeout as configured by the user.

Actual Result

cpuinfo disregards the configured timeout as configured by the user.

Collector version

v0.103.1

Environment information

Environment

OS: Windows (10, 11, 2019, 2022)

OpenTelemetry Collector configuration

receivers:
  otlp:
    protocols:
      grpc:
      http:
processors:
  resourcedetection:
    timeout: 60s
    detectors:
      - system
      - env
    system:
      resource_attributes:
        host.id:
          enabled: true
exporters:
  logging:
service:
  pipelines:
    traces:
      receivers:
        - otlp
      processors:
        - resourcedetection
      exporters:
        - logging

Log output

No response

Additional context

No response

Footnotes

  1. https://github.com/shirou/gopsutil/blob/master/cpu/cpu_windows.go#L92-L94

@cwegener cwegener added bug Something isn't working needs triage New item requiring triage labels Jun 26, 2024
Copy link
Contributor

Pinging code owners:

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

@cwegener
Copy link
Contributor Author

Quick fix:

diff --git a/processor/resourcedetectionprocessor/internal/system/system.go b/processor/resourcedetectionprocessor/internal/system/system.go
index f892087a48..54ace04266 100644
--- a/processor/resourcedetectionprocessor/internal/system/system.go
+++ b/processor/resourcedetectionprocessor/internal/system/system.go
@@ -130,7 +130,7 @@ func (d *Detector) Detect(ctx context.Context) (resource pcommon.Resource, schem
 		return pcommon.NewResource(), "", fmt.Errorf("failed getting OS description: %w", err)
 	}
 
-	cpuInfo, err := cpu.Info()
+	cpuInfo, err := cpu.InfoWithContext(ctx)
 	if err != nil {
 		return pcommon.NewResource(), "", fmt.Errorf("failed getting host cpuinfo: %w", err)
 	}

@cwegener
Copy link
Contributor Author

Forgot to mention that the default timeout value in gopsutil is 3 seconds 1, which is nowhere near enough for COM/WMI instantiation.

Footnotes

  1. https://github.com/shirou/gopsutil/blob/v3.24.5/internal/common/common.go#L32

@mx-psi
Copy link
Member

mx-psi commented Jun 26, 2024

@cwegener Your fix looks reasonable to me, woluld you be open to file a PR to fix this?

@mx-psi
Copy link
Member

mx-psi commented Jun 26, 2024

@cwegener Actually I think we can tackle this on #33774 so no need for a separate PR :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working os:windows priority:p2 Medium processor/resourcedetection Resource detection processor
Projects
None yet
2 participants