Add an otelcol.processor.resourcedetection component #5764

ptodev · 2023-11-14T09:55:58Z

This is a draft PR to port over the resource detection processor. There are a few things which we need to agree on before the PR is taken out of "draft" stage. I will open comments on the specific lines where we need consensus.

docs/sources/flow/reference/components/otelcol.processor.resourcedetection.md

ptodev · 2023-11-14T10:49:30Z

component/otelcol/processor/resourcedetection/resourcedetection.go

+	// HTTP client settings for the detector
+	// Timeout default is 5s
+	// Client otelcol.HTTPClientArguments `river:",squash"`
+	Timeout time.Duration `river:"timeout,attr,optional"`
+	//TODO: Uncomment this later? Can we just get away with a timeout, or do we need all the http client settings?
+	//      It seems that HTTP client settings are only used in the ec2 detection via ClientFromContext.


The HTTP settings in the Collector expose HTTP client settings for two reasons:

The timeout is used to cancel resource detections if they take too long. This use case has nothing to do with HTTP at all.

The ec2 detection uses the HTTP settings if they are provided. This feature is not documented in the OTel docs.

For now I would like to expose just the timeout parameter:

This is because it would be super confusing to document timeout as both a timeout for HTTP and for ec2.

I'll leave a TODO to implement it in the future.

I wish the OTel Collector worked differently:

There should be a non-HTTP timeout settings for all detectors

There should be HTTP settings specific to the ec2 detector. That way ec2 won't use context, and it will be possible to have different HTTP settings for different detectors.

Maybe it'll be work raising these issues with the upstream maintainers.

docs/sources/flow/reference/components/otelcol.processor.resourcedetection.md

.../processor/resourcedetection/internal/resource_attribute_config/resource_attribute_config.go

github-actions · 2023-12-22T00:10:16Z

This PR has not had any activity in the past 30 days, so the needs-attention label has been added to it.
If you do not have enough time to follow up on this PR or you think it's no longer relevant, consider closing it.
The needs-attention label signals to maintainers that something has fallen through the cracks. No action is needed by you; your PR will be kept open and you do not have to respond to this comment. The label will be removed the next time this job runs if there is new activity.
Thank you for your contributions!

docs/sources/flow/reference/components/otelcol.processor.resourcedetection.md

ptodev · 2024-01-10T21:13:29Z

docs/sources/flow/reference/components/otelcol.processor.resourcedetection.md

+---------------------------------------------- | ------------------------------------------------- | --------
+[resource_attributes](#ec2--resource_attributes) | Configures which resource attributes to add.      | no
+
+##### ec2 > resource_attributes


These ... > resource_attributes headings have a small weight of #### so that they don't appear in the ToC. This makes the ToC much cleaner.

docs/sources/flow/reference/components/otelcol.processor.resourcedetection.md

component/otelcol/processor/resourcedetection/resourcedetection.go

rlankfo · 2024-01-10T23:00:02Z

Does this provide support for the env resource detector? e.g. setting resource attributes in OTEL_RESOURCE_ATTRIBUTES environment variable?

ptodev · 2024-01-11T14:10:42Z

Does this provide support for the env resource detector? e.g. setting resource attributes in OTEL_RESOURCE_ATTRIBUTES environment variable?

@rlankfo - Yes. It is mentioned in the docs and there is a unit test.

component/otelcol/processor/resourcedetection/internal/openshift/config.go

clayton-cornell

Some doc suggestions and questions

docs/sources/flow/reference/components/otelcol.processor.resourcedetection.md

tpaschalis

LGTM % some nits; I don't mean to block this PR as it's been a large effort, but I have a potential concern about the long-term maintainability of it and whether we could re-think how we configure these blocks and keep track of new options.

GIven that would be a big change in of itself, I'm fine with re-considering in a future PR.

component/otelcol/config_k8s.go

component/otelcol/processor/resourcedetection/internal/aws/ec2/config.go

component/otelcol/processor/resourcedetection/resourcedetection.go

.../processor/resourcedetection/internal/resource_attribute_config/resource_attribute_config.go

clayton-cornell

Docs look good to me

Co-authored-by: Clayton Cornell <[email protected]>

mar4uk · 2024-01-24T16:07:23Z

🎉

* Add a otelcol.processor.resourcedetection component * PR review suggestions * Make k8s API config authentication options const. * Fail validation if an incorrect detector is supplied. * Remove unnecessary Consul attributes. --------- Co-authored-by: Clayton Cornell <[email protected]>

ptodev linked an issue Nov 14, 2023 that may be closed by this pull request

OTEL: Resource Detection Processor #4183

Closed

ptodev commented Nov 14, 2023

View reviewed changes

docs/sources/flow/reference/components/otelcol.processor.resourcedetection.md Outdated Show resolved Hide resolved

ptodev commented Nov 14, 2023

View reviewed changes

docs/sources/flow/reference/components/otelcol.processor.resourcedetection.md Outdated Show resolved Hide resolved

ptodev commented Nov 14, 2023

View reviewed changes

.../processor/resourcedetection/internal/resource_attribute_config/resource_attribute_config.go Show resolved Hide resolved

ptodev mentioned this pull request Nov 20, 2023

Add metrics and resource attribute config options from vcentereceiver in docs and tests #5813

Merged

4 tasks

github-actions bot added the needs-attention An issue or PR has been sitting around and needs attention. label Dec 22, 2023

ptodev force-pushed the 4183-otel-resource-detection-processor branch from 4e708de to 4bb42f6 Compare January 10, 2024 21:08

ptodev removed the needs-attention An issue or PR has been sitting around and needs attention. label Jan 10, 2024

ptodev commented Jan 10, 2024

View reviewed changes

ptodev marked this pull request as ready for review January 10, 2024 21:16

rlankfo reviewed Jan 10, 2024

View reviewed changes

component/otelcol/processor/resourcedetection/resourcedetection.go Outdated Show resolved Hide resolved

ptodev force-pushed the 4183-otel-resource-detection-processor branch from 4d855af to 317b367 Compare January 11, 2024 14:08

mar4uk reviewed Jan 18, 2024

View reviewed changes

component/otelcol/processor/resourcedetection/internal/openshift/config.go Show resolved Hide resolved

ptodev force-pushed the 4183-otel-resource-detection-processor branch from ffefa15 to cf1f371 Compare January 18, 2024 12:26

clayton-cornell reviewed Jan 18, 2024

View reviewed changes

clayton-cornell added the type/docs Docs Squad label across all Grafana Labs repos label Jan 18, 2024

tpaschalis approved these changes Jan 19, 2024

View reviewed changes

clayton-cornell approved these changes Jan 19, 2024

View reviewed changes

ptodev added 8 commits January 23, 2024 11:41

Add a otelcol.processor.resourcedetection component

7db6642

Remove "block" from title

0a11ef1

Add common config and fill in attributes table

ffd96aa

Remove examples

98238ef

Write docs, add defaults

18db16d

Make detector blocks optional

bdc202f

Update changelog, add to all.go

54a22b0

make generate docs

4c922a1

ptodev and others added 8 commits January 23, 2024 11:41

Add docs aliases

44f63b6

Fix typo in comment

9e5bcff

Cleanup comments

7c896f5

Apply suggestions from code review

5c75846

Co-authored-by: Clayton Cornell <[email protected]>

PR review suggestions

0df95f0

Make k8s API config authentication options const.

9b89487

Fail validation if an incorrect detector is supplied.

c314342

Remove unnecessary Consul attributes.

1a3f73f

ptodev force-pushed the 4183-otel-resource-detection-processor branch from 68d5af3 to 1a3f73f Compare January 23, 2024 11:41

ptodev merged commit 74245fd into main Jan 23, 2024
10 checks passed

ptodev deleted the 4183-otel-resource-detection-processor branch January 23, 2024 19:39

github-actions bot added the frozen-due-to-age Locked due to a period of inactivity. Please open new issues or PRs if more discussion is needed. label Feb 24, 2024

github-actions bot locked as resolved and limited conversation to collaborators Feb 24, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add an otelcol.processor.resourcedetection component #5764

Add an otelcol.processor.resourcedetection component #5764

ptodev commented Nov 14, 2023

ptodev Nov 14, 2023

github-actions bot commented Dec 22, 2023

ptodev Jan 10, 2024

rlankfo commented Jan 10, 2024

ptodev commented Jan 11, 2024

clayton-cornell left a comment

tpaschalis left a comment

clayton-cornell left a comment

mar4uk commented Jan 24, 2024

Add an otelcol.processor.resourcedetection component #5764

Add an otelcol.processor.resourcedetection component #5764

Conversation

ptodev commented Nov 14, 2023

ptodev Nov 14, 2023

Choose a reason for hiding this comment

github-actions bot commented Dec 22, 2023

ptodev Jan 10, 2024

Choose a reason for hiding this comment

rlankfo commented Jan 10, 2024

ptodev commented Jan 11, 2024

clayton-cornell left a comment

Choose a reason for hiding this comment

tpaschalis left a comment

Choose a reason for hiding this comment

clayton-cornell left a comment

Choose a reason for hiding this comment

mar4uk commented Jan 24, 2024