Extract builder for pipelines components into one struct #5542
Conversation
bogdandrutu
added
the
Skip Changelog
bogdandrutu
force-pushed
the
createpipelines
branch
from
a6f56e8 to
3720160
Compare
Codecov Report
@@ Coverage Diff @@
## main #5542 +/- ##
==========================================
+ Coverage 90.94% 90.96% +0.01%
==========================================
Files 191 192 +1
Lines 11375 11397 +22
==========================================
+ Hits 10345 10367 +22
Misses 807 807
Partials 223 223
Continue to review full report at Codecov.
|
bogdandrutu
force-pushed
the
createpipelines
branch
from
3720160 to
27820f5
Compare
|
|
||
| // StartAll starts all pipelines. | ||
| // | ||
| // Start with exporters, processors (in revers configured order), then receivers. |
| } | ||
|
|
||
| // Build builds all pipelines from config. | ||
| func Build(ctx context.Context, telemetry component.TelemetrySettings, buildInfo component.BuildInfo, cfg *config.Config, factories component.Factories) (*BuiltPipelines, error) { |
There was a problem hiding this comment.
It feels weird to pass both component.TelemetrySettings and service.Config as arguments, given that the config struct has service.ConfigServiceTelemetry within it. I definitely want to be able to customize the component.TelemetrySettings for my use case, and I think several other people have expressed their need for this, so not passing the telemetry structs in some way does not feel like an option. Some possibilities are:
- Move
service.ConfigServiceTelemetryout of its current location in the configuration. I think this is not a realistic option if it affects the YAML, since it will break too many things, but we could do it with a custom unmarshaling function if we want to go this way. - Do a 'telemetry provider' kind of thing. It makes this somewhat more clunky to use for e.g. my use case, but maybe it's the way to go?
- Do nothing and just document it; accept that we can't change it and that we have this weirdness in the API
There was a problem hiding this comment.
Will address this issue separately, I am planning to actually send only what is needed (no telemetry settings).
There was a problem hiding this comment.
👍 , yeah, thinking about this later, the Extensions field is also a problem, so probably we can just send exactly what we need
Signed-off-by: Bogdan Drutu <[email protected]>
bogdandrutu
force-pushed
the
createpipelines
branch
from
27820f5 to
930fc5c
Compare
Signed-off-by: Bogdan Drutu [email protected]