[routingprocessor] Make exporters registration more generic. #13529
Conversation
kovrus
force-pushed
the
issue-13158-refactoring
branch
from
b629885 to
712240e
Compare
|
cc: @gouthamve |
| logger: logger, | ||
| router: newRouter(*oCfg, logger), | ||
| logger: logger, | ||
| metricsRouter: newRouter[component.MetricsExporter](*oCfg, logger), |
There was a problem hiding this comment.
This might be the first usage of generics in this code base. Are you able to find other uses? If this is the first, we might want to give a heads up to other maintainers.
There was a problem hiding this comment.
I couldn't find any other usage of generics in contrib repository. Is there an easy way to give heads up to maintainers?
There was a problem hiding this comment.
@open-telemetry/collector-contrib-maintainer , heads up: we are about to introduce the first usage of generics in the code base. Does anyone have anything against this?
There was a problem hiding this comment.
The TQL already has some due to the participle package requiring them. It is about to have some more via #13320.
| @@ -344,7 +340,7 @@ func (r *router[E]) registerExporters(exporters map[config.ComponentID]component | |||
| for id, exp := range exporters { | |||
| exporter, ok := exp.(E) | |||
| if !ok { | |||
| return fmt.Errorf("the exporter %q isn't a ... exporter", id.String()) | |||
| return fmt.Errorf("the exporter %q isn't a %T exporter", id.String(), new(E)) | |||
There was a problem hiding this comment.
It will print smth like the exporter "otlp" isn't a *component.TracesExporter exporter
kovrus
force-pushed
the
issue-13158-refactoring
branch
from
1d3f848 to
b1e9942
Compare
kovrus
force-pushed
the
issue-13158-refactoring
branch
from
b1e9942 to
7a5710e
Compare
| @@ -34,69 +34,71 @@ import ( | |||
| // structure (plog.Logs, pmetric.Metrics and ptrace.Traces respectively) in order | |||
| // to not cause higher CPU usage in the exporters when exproting data (it's always | |||
| // better to batch before exporting). | |||
| type router struct { | |||
| type router[E component.Exporter] struct { | |||
There was a problem hiding this comment.
I wanted to introduce a type constraint for exporters to limit type params only to component.TracesExporter,
component.MetricsExporter, and component.LogsExporter but it seems that currently it is not possible to union types with behavioural interfaces.
|
Looks good. I don't think we need a changelog entry for this, as there should not be any behavior change. I'll label this accordingly if that's your understanding as well. |
Yes, we do not change the semantic here, so it is fine to skip the change log entry. |
Description:
Make exporters registration more generic. It should simplify using tql in routing table PoC for #13158
Link to tracking Issue:
Testing:
Documentation: