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

Allow passing a custom reporter to the controller #207

Merged
merged 9 commits into from
Oct 31, 2024

Conversation

dmathieu
Copy link
Member

Right now, the Collector receiver implementation (in #160) uses OTLPReceiver, which sends data over the wire to an OTLP server.

This is not the behavior we want for the collector. We need to use nextConsumer to pass the data down to the next stage, and have it respect the collector's config.
To achieve that, we need to be able to pass a custom reporter to the controller, so a CollectorReporter can build pdata from the traces, and pass it down to the next consumer.

@dmathieu dmathieu marked this pull request as ready for review October 25, 2024 09:34
@dmathieu dmathieu requested review from a team as code owners October 25, 2024 09:34
@dmathieu
Copy link
Member Author

dmathieu commented Oct 25, 2024

See also draft #208 (WIP) for the main custom collector we'll need to pass in the collector receiver.

internal/controller/reporter.go Outdated Show resolved Hide resolved
Comment on lines +23 to +25
// If the reporter needs to perform a long-running starting operation then it
// is recommended that Start() returns quickly and the long-running operation
// is performed in the background.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if we should have this here as in some cases (e.g. long-running operation that can fail) it's going to create problems (reporter that hasn't initialized properly with the rest of the agent being oblivious and executing as normal).

I'd reword to the following or remove altogether:

Suggested change
// If the reporter needs to perform a long-running starting operation then it
// is recommended that Start() returns quickly and the long-running operation
// is performed in the background.
// If this method needs to perform a long-running operation that can NOT fail, it is
// recommended that Start returns quickly and the long-running operation is
// performed in the background.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know. Running an HTTP server can fail, yet that's a long-running operation that should not run synchronously here.
If folks need something long-running that can report errors, they should setup a channel, or an error handler to be able to report the error upstream.

Note that this comment is heavily inspired by the one on collector component, which does the same thing.
https://github.com/open-telemetry/opentelemetry-collector/blob/main/component/component.go#L32

@christos68k christos68k merged commit a720d06 into open-telemetry:main Oct 31, 2024
23 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants