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

routingprocessor: remove broken debug log fields #6373

Conversation

pmalek-sumo
Copy link
Contributor

Description: Remove broken debug log fields in routingprocessor

The logger fields that were added as part of the last change introduced the following unwanted log output:

2021-11-18T14:51:59.175Z    debug    routingprocessor@v0.38.0/router.go:400    Registering exporters for routes    {"kind": "processor", "name": "routing", "exportersError": "json: unsupported type: componenthelper.StartFunc"}
2021-11-18T14:51:59.175Z    debug    routingprocessor@v0.38.0/router.go:446    Registering exporter for route    {"kind": "processor", "name": "routing", "route": "/prometheus.metrics.apiserver", "availableError": "json: unsupported type: componenthelper.StartFunc", "requested": ["sumologic/apiserver"]}
2021-11-18T14:51:59.175Z    debug    routingprocessor@v0.38.0/router.go:446    Registering exporter for route    {"kind": "processor", "name": "routing", "route": "/prometheus.metrics.container", "availableError": "json: unsupported type: componenthelper.StartFunc", "requested": ["sumologic/kubelet"]}

So let's remote them.

@pmalek-sumo pmalek-sumo requested a review from a team November 18, 2021 15:00
@@ -394,7 +394,7 @@ func (r *router) registerTracesExporters(hostTracesExporters map[config.Componen
// registerExportersForRoutes registers exporters according to the configuring
// routing table, taking into account the provided map of available exporters.
func (r *router) registerExportersForRoutes(available ExporterMap) error {
r.logger.Debug("Registering exporters for routes", zap.Any("exporters", available))
Copy link
Member

Choose a reason for hiding this comment

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

Can we get a list of exporter names out of this map, instead of just removing it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We could but I'd need to iterate over the whole map and save it in a slice (conditionally on a log level? Can this be easily done?). Would that be ok?

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 it's worth it -- it would certainly be helpful to know the available exporters and the requested for the route, but perhaps the extra code wouldn't be worth it...

Copy link
Member

@jpkrohling jpkrohling left a comment

Choose a reason for hiding this comment

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

I'm approving, but feel free to send an update to this PR if you can find a cheap and reasonable way to list the exporters. If you are OK with merging as is, let me know.

@pmalek-sumo
Copy link
Contributor Author

I'm approving, but feel free to send an update to this PR if you can find a cheap and reasonable way to list the exporters. If you are OK with merging as is, let me know.

Let's leave it as is for now. As far as the "cheap and reasonable way": this is just happening on the registration when processor initializes itself so we shouldn't be that concerned about it being cheap/not cheap :)

@jpkrohling jpkrohling merged commit fe31633 into open-telemetry:main Nov 22, 2021
@sumo-drosiek sumo-drosiek deleted the routingprocessor-remove-broken-debug-log-field branch March 1, 2024 14:06
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.

5 participants