[processor/resourcedetection] Add support for dynamic refresh resource attributes

[paulojmdias]

Description

This PR adds support for a configurable refresh_interval to the resourcedetectionprocessor.
When set, resource detection will re-run periodically in the background, allowing resources (e.g., cloud metadata, container tags) to be updated without restarting the collector.

Since refresh is disabled by default, the current behavior is maintained.

Link to tracking issue

Fixes #42663

Testing

I tested in AWS, updating tags while the processor is running.

Also added tests to support this feature.

Documentation

Updated README.md with this parameter.

[atoulme]

This seems like a neat enhancement.

[github-actions]

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

[paulojmdias]

/label -stale

[mx-psi]

@dashpole @Aneurysm9 this is waiting for your review

[paulojmdias]

/label -stale

[paulojmdias]

Tests are failing due to #43625

[Aneurysm9]

This documentation could use a clarification pass. What is "some time"? How frequent is too frequent to refresh detected resource attributes? Should there be a lower bound on the refresh_interval?

[paulojmdias]

Good points! I've clarified the documentation with:

• Specific timing: "up to refresh_interval duration"
• Performance guidance: Values below 5 minutes can increase resource usage; below 1 minute is strongly discouraged
• No enforced minimum, but clear recommendations provided
• Added context about metric cardinality impact

[dashpole]

should we initialize res as pcommon.NewResource() so we don't need this check?

[paulojmdias]

I've refactored this to initialize the return values upfront with their defaults, so we don't need the nil check anymore.

[dashpole]

Is this the current behavior? It seems like we should fail if any of the detectors failed...

[paulojmdias]

I've rephrased the comment to be clearer about the partial success behavior.

[dashpole]

If we return the previous error, won't that cause refreshLoop to log the previous error? That seems very confusing if the error differs from our current error.

[paulojmdias]

Yeah, you're right. Thanks for pointing me out. Since prev.err is guaranteed to be nil here (from the hadPrevSuccess check), I've changed it to explicitly return nil to make the intent clear and avoid any confusion in the logs.

[dashpole]

Should we move the initial attempt at detection to Start so that we don't need the initOnce logic in the resource provider?

[paulojmdias]

I've moved the initial detection to Start(), which now calls Refresh() directly, removed the initOnce field entirely, and made Get() truly read-only with no initialization logic. Tests have been updated accordingly to call Refresh() first for initialization. Please let me know what you think about this change 🙏

[dashpole]

I think a big part of why the code is so confusing is that you are caching the latest resource here and in internal/resourcedetection.go. Please only store the current value in one place, and concentrate the refresh goroutine logic and handling of errors there.

[paulojmdias]

I've refactored the code to store the resource in a single location (the provider's detectedResource). I removed the current atomic.Pointer from the processor and moved all refresh logic into the provider with StartRefreshing() and StopRefreshing() methods. Now the processor just calls provider.Get() to retrieve the cached resource, and the provider handles all the refresh goroutine logic and error handling internally. This eliminates the duplicate state and makes the architecture much cleaner. Let me know if it makes sense to you 🙏