-
Notifications
You must be signed in to change notification settings - Fork 273
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
Setup collector receiver #160
base: main
Are you sure you want to change the base?
Conversation
cd467ae
to
a639d0a
Compare
ccdac1c
to
00c1c6b
Compare
00c1c6b
to
032d279
Compare
Co-authored-by: Tim Rühsen <[email protected]>
032d279
to
70647f1
Compare
collector/internal/helpers.go
Outdated
@@ -0,0 +1,236 @@ | |||
// Copyright The OpenTelemetry Authors |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can't we move opentelemetry-ebpf-profiler/helpers.go
into its own package that we can import here to avoid duplicating all this code?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done in #197
collector/internal/config.go
Outdated
) | ||
|
||
// Config represents the receiver config settings within the collector's config.yaml | ||
type Config struct { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe for another PR, but it'd be nice if we used a single configuration structure to cover both CLI and collector, that way the validation logic would also be shared.
|
||
// Start starts the receiver. | ||
func (c *Controller) Start(ctx context.Context, host component.Host) error { | ||
cfg := c.config |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since there's a lot of duplication with startup logic in main here, maybe we should factor out commonalities into a separate package (e.g. agent
or controller
) that we can reuse here and in main
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While your point definitely makes sense, this is far from being a trivial change in main.go
, as it mixes a lot of things together. So making that refactoring looks like it's going to take several PRs.
While I'm happy to look into that, I wonder if we shouldn't start without this refactoring, so we can move forward with the collector support.
Moving this back to draft. I realized this uses the OTLPReporter, which doesn't use the collector's |
This is an extraction from #87, with the intent to clean up and setup only the collector receiver, without an attached collector distribution.