Do not print events in case custom event callback was specified

[nazar-pc]

Proposed change

Do not print messages like this:

2024-05-10T23:05:19.230236Z  INFO async_nats: event: connected

In case ConnectOptions.event_callback was specified.

Use case

Info level messages are often done by the app, printing them by the low-level library like NATS is annoying. For example I'd like to also print client ID for easier debugging, but this means similar message will be printed twice.

Contribution

I can send a PR if this is considered to be a welcome change

[Jarema]

Those are not print messages, but tracing messages.

Will only be showed if you configured the tracing_subscriber. Those are very useful if you're debugging your app/library and entirely opt-in.

If you have some other tracing you would like to subscribe to, while not seeing the library logs, you can disable them via EnvFilter: https://stackoverflow.com/questions/73247589/how-to-turn-off-tracing-events-emitted-by-other-crates

[nazar-pc]

Right, I know that are produced by tracing subscriber. My concern is about log level of such things. This is the only low-level library that unconditionally prints such messages. Usually libraries print WARN and ERROR messages and everything else is either DEBUG or TRACE level, not INFO.

[Jarema]

Hm, I get your point, however I'm not sure if I agree with such approach. If library can print ERROR, why not INFO if the log level is relevant to the log?

Interested what some other contributors thoughts are cc. @thomastaylor312 @paolobarbolini @caspervonb

[nazar-pc]

ERROR typically means something that is broken and must be fixed, WARN means something isn't quite as bad, but worth looking into. Both of those are important for developer and/or maintainer to know and printed in case normal error propagation to the code library user controls is not possible in a straightforward way.

INFO on the other hand is not so important. So if library user can hook into event_callback, they also can print the same (or different or none at all) message about connection state, this is likely why they used event_callback in the first place. In this case printing it unconditionally is redundant and even annoying sometimes.

[Jarema]

I mostly agree.

However, I do not like conditionaly outputitng or not some logs as a side effect. I would be suprised as a user, that if I asked for callback, that action would have a side effect of disabling some logs by default.

Especially that sometimes different teams are handling fault detection and app development. Hm, maybe option to disable Event logs would be a more straightfoward, even if a bit verbose solution to this problem?

[caspervonb]

Aye we should be logging what events we see. Three options as as I see it.

1. Add a span for it.
2. Bump log level
3. Keep as is.

[thomastaylor312]

I have noticed this as well but I kinda just gloss over this output at this point. I think this kind of logging feels like a DEBUG level log. Seeing which event I received would be something I'd want to see if I were debugging, but not necessarily under normal operation. Definitely agree that it shouldn't be a conditional thing that depends on whether or not a callback was specified