open-telemetry / opentelemetry-collector-contrib

Contrib repository for the OpenTelemetry Collector
https://opentelemetry.io
Apache License 2.0
2.94k stars 2.29k forks source link

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

Closed cwegener closed 3 months ago

cwegener commented 3 months ago

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

github-actions[bot] commented 3 months ago

Pinging code owners:

cwegener commented 3 months ago

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 commented 3 months ago

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

mx-psi commented 3 months ago

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

mx-psi commented 3 months ago

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