You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
The SocketAppenderBase uses an InetAddress to establish a socket connection. The object of the InetAddress is being created on setting the remoteHost string value. Since InetAddress does a host name lookup on initialization, it requires an internet connection, meaning even if the appender is being configured to be lazy initialized an application using such an appender would always need the device to be online in some way.
This can lead to different problems:
The application might hang for some time until the device is online, in case it blocks on logger initialization and some weird android connectivity stuff is going on.
Setting the address property might fail because of an non existing internet connection and cause the SocketAppender to never have an address, thus never being able to connect to a socket (silently returns on each connection attempt when address is null).
The same problem exists on the slightly different code on qos-ch/logback master. Their the problem can occur on starting the appender. I wonder what happens if an appender does not actually end up started after calling it's start method.
The easy solution for this problem would be to create the socket from the remoteHost and not use an InetAddress at all.
Probably there should also be an issue created on qos-ch/logback. @tony19 can you coordinate this? I think it might make sense to fix this just in one place and then merge it over.
The text was updated successfully, but these errors were encountered:
logback-android is mostly sync'ed up with logback, so as of v1.1.1-1, SocketAppenderBase is now AbstractSocketAppender. It looks like I accidentally removed the lazy flag from SocketAppender in that merge.
But speaking in terms of v1.0.10-2 (which i think you're still using): I agree there's a problem with SocketAppenderBase#getRemoteHost(), regarding the lazy flag. It seems the address field should've been populated in SocketAppenderBase#start(). I do like the idea of removing the address field, since we only need the hostname and port to create the socket (both of which we already have).
Yes, please create the pull request in logback. Thanks!
I gave this issue some more thought. Basically we want to do the following things:
not use INetAddress so we do not run in connection problems when we don't even try to create a connection yet
validate the host on start of the component
Both targets contradict. We can not make a host lookup without an internet connection. Meaning if we want to remove the dependency of an internet connection at start of a socket component it must not do a host lookup. We might just do a null/empty check and/or make the host lookup dependent on the lazy flag.
FYI: I can not see any lazy flags anymore in the socket components on your fork.
The
SocketAppenderBase
uses anInetAddress
to establish a socket connection. The object of theInetAddress
is being created on setting theremoteHost
string value. SinceInetAddress
does a host name lookup on initialization, it requires an internet connection, meaning even if the appender is being configured to be lazy initialized an application using such an appender would always need the device to be online in some way.This can lead to different problems:
address
property might fail because of an non existing internet connection and cause theSocketAppender
to never have anaddress
, thus never being able to connect to a socket (silently returns on each connection attempt whenaddress
is null).The same problem exists on the slightly different code on qos-ch/logback master. Their the problem can occur on starting the appender. I wonder what happens if an appender does not actually end up started after calling it's start method.
The easy solution for this problem would be to create the socket from the
remoteHost
and not use anInetAddress
at all.Probably there should also be an issue created on qos-ch/logback. @tony19 can you coordinate this? I think it might make sense to fix this just in one place and then merge it over.
The text was updated successfully, but these errors were encountered: