clustermesh: silence misleading logs about service resolution

[giorio94]

9f5a82a ("clustermesh: use custom dialer for service resolution") configured a custom dialer to perform service resolution when the etcd address refers to a local service (mainly in the kvstoremesh case).

Yet, the custom dialer function outputs quite verbose log messages, which can be misleading when the address does not point to a local service (as it is correct that the address cannot be parsed). One example being:

level=error msg="Unable to parse etcd service URL" error="parse \"172.19.0.2:2379\": first path segment in URL cannot contain colon"

Hence, let's silence them, as they do more harm than good in this specific situation. We still print (at debug level) the outcome of the translation, so that we know to whom we are actually connecting.

Silence misleading log messages about service resolution in clustermesh 

[giorio94]

/test

[squeed]

Is there any chance that this will hide useful error messages as well? Should we re-enable logging if debug mode is on?

[giorio94]

Is there any chance that this will hide useful error messages as well? Should we re-enable logging if debug mode is on?

I'm a bit doubtful as well. That function currently emits four log messages depending on the conditions:

• Unable to parse etcd service URL (error level): this is definitely misleading and should never be emitted in this context, given that for instance IP addresses are valid target addresses.
• Unable to parse etcd service URL into a service ID (debug level): could not convert the address into a service name (it should never happen unless the address has no dots).
• Service not found in the service IP getter (debug level): could not find the given service (this can again happen in many legitimate cases if the address looked like a service name but it was not);
• custom dialer based on k8s service backend is dialing to %q (debug level): shows the actual address which is being dialed.

An alternative could be to add a flag to silence the first three types of messages and keep only the last, to know the actual target and spot possible issues. WDYT?

[squeed]

It would be ideal to log - somewhere - the actual IP to which we're connecting, especially since there is some amount of magic going on. But I wouldn't consider that a blocker -- your call.

[giorio94]

It would be ideal to log - somewhere - the actual IP to which we're connecting, especially since there is some amount of magic going on. But I wouldn't consider that a blocker -- your call.

I updated that function to allow silencing the error messages, while preserving the one which shows the target address (at debug level). While being there, I also updated the custom dialer to propagate the given context. @squeed PTAL.

[giorio94]

/test

[asauber]

I'd like to propose an alternative solution. We should be using valid URLs in any cases where this error message is popping up. Adding a scheme (http://, https://) to the string will allow these to parse as expected, and this is the type of value which is used in the etcd docs.

Go Playground example: https://go.dev/play/p/OJB_oM5PQGO

[giorio94]

I'd like to propose an alternative solution. We should be using valid URLs in any cases where this error message is popping up.

Yeah, the dialer is passed the address without the scheme, although it was originally present. We could probably prepend a fake scheme while doing the translation to silence it (although it wouldn't change anything in terms of the final result). I'm still personally in favor of this change, as also the other log messages pointing out failures (although at debug level) might be misleading (as also possibly repeated multiple times). WDYT?

[asauber]

@giorio94 considering the service IP lookup, it makes sense to me