reverse_tunnel: Add some info logs for tracking downstream conns#45022
reverse_tunnel: Add some info logs for tracking downstream conns#45022aakugan wants to merge 2 commits into
Conversation
Signed-off-by: aakugan <aakashganapathy2@gmail.com>
aakugan
force-pushed
the
add-logs-downstream
branch
from
c47fced to
99c21c1
Compare
|
|
||
| ENVOY_LOG(debug, "reverse_tunnel: RAII IoHandle created with duplicated socket " | ||
| "and protection enabled."); | ||
| ENVOY_LOG(info, "reverse_tunnel: RAII IoHandle created with duplicated socket for node_id: {}" |
There was a problem hiding this comment.
This is a per-request log and we should keep it at debug to prevent log flooding. Per-request logs can have a level > debug only if they indicate an error condition.
For better verbosity while debugging, you can turn on debug logs during testing.
There was a problem hiding this comment.
@basundhara-c These are per tunnel logs. We should only see max connection_count * worker threads number of logs during envoy process life cycle right?
There was a problem hiding this comment.
It's listener_count * max connection_count * worker threads, right? Do we have an upper bound on the this (the total handshake request count)?
Also, cannot we expose this on stats, that should suffice for debugging?
There was a problem hiding this comment.
Hmm the thought process was to use info cuz these are infrequent per listener like ideally once every few odd minutes so should not be a bottleneck + stats imo are not great for this use case as we can cause explosion if we tag with the node id etc and have large number of listeners.
| RCConnectionWrapper* wrapper, bool closed) { | ||
| ENVOY_LOG(debug, "reverse_tunnel: Connection wrapper done - error: '{}', closed: {}", error, | ||
| closed); | ||
| ENVOY_LOG(info, "reverse_tunnel: Connection wrapper done - error: '{}', closed: {}, for node_id: {}", error, |
There was a problem hiding this comment.
Same as above, we cannot turn on info for per-request logs. If there are any other per-request logs we erronnously kept at info in the codebase, we should switch them to debug as well.
5 participants
Commit Message
reverse_tunnel: Add some info logs for tracking downstream conns
Additional Description
Added some info logs annotated with the node id which makes the lifecycle of the conns more apparent from the logs. More specifically: need to make a conn, making a conn, made a conn / failed to make a conn and the conn accepted by the listener.
Testing
Just logs so the existing unit and integ tests should suffice imo.