Skip to content

Commit 7bf783b

Browse files
chrjohnchrjohn
authored
Merge pull request #746 from quickfix-j/use-session-log
Changed logging to use session-specific log where applicable
2 parents 89acf0c + dad51d7 commit 7bf783b

File tree

10 files changed

+143
-39
lines changed

10 files changed

+143
-39
lines changed

‎quickfixj-core/src/main/java/quickfix/Log.java‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,10 +51,20 @@ public interface Log {
5151
void onEvent(String text);
5252

5353
/**
54-
* Logs an session error event.
54+
* Logs a session error event.
5555
*
5656
* @param text the event description
5757
*/
5858
void onErrorEvent(String text);
5959

60+
/**
61+
* Logs a session warning event.
62+
* Logs a session error event if not implemented.
63+
*
64+
* @param text the event description
65+
*/
66+
default void onWarnEvent(String text) {
67+
onErrorEvent(text);
68+
}
69+
6070
}

‎quickfixj-core/src/main/java/quickfix/LogUtil.java‎

Lines changed: 58 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -39,15 +39,22 @@ public class LogUtil {
3939
* @param t the exception to log
4040
*/
4141
public static void logThrowable(Log log, String message, Throwable t) {
42+
String throwableString = constructThrowableString(message, t);
43+
log.onErrorEvent(throwableString);
44+
}
45+
46+
private static String constructThrowableString(String message, Throwable t) {
4247
final StringWriter stringWriter = new StringWriter();
4348
final PrintWriter printWriter = new PrintWriter(stringWriter);
4449
printWriter.println(message);
45-
t.printStackTrace(printWriter);
46-
if (t.getCause() != null) {
47-
printWriter.println("Cause: " + t.getCause().getMessage());
48-
t.getCause().printStackTrace(printWriter);
50+
if (t != null) {
51+
t.printStackTrace(printWriter);
52+
if (t.getCause() != null) {
53+
printWriter.println("Cause: " + t.getCause().getMessage());
54+
t.getCause().printStackTrace(printWriter);
55+
}
4956
}
50-
log.onErrorEvent(stringWriter.toString());
57+
return stringWriter.toString();
5158
}
5259

5360
/**
@@ -68,5 +75,51 @@ public static void logThrowable(SessionID sessionID, String message, Throwable t
6875
log.error(message, t);
6976
}
7077
}
78+
79+
/**
80+
* Logs a throwable including the stack trace as a session warning event.
81+
* If session cannot be found, the general log is used.
82+
*
83+
* @param sessionID sessionID of Session to lookup
84+
* @param message the message to log
85+
* @param throwable throwable to log
86+
*/
87+
public static void logWarning(SessionID sessionID, String message, Throwable throwable) {
88+
String throwableString = constructThrowableString(message, throwable);
89+
logWarning(sessionID, throwableString);
90+
}
91+
92+
/**
93+
* Logs a warning as a session event if the session is registered, otherwise
94+
* the general log is used.
95+
*
96+
* @param sessionID sessionID of Session to lookup
97+
* @param message the message to log
98+
*/
99+
public static void logWarning(SessionID sessionID, String message) {
100+
final Session session = Session.lookupSession(sessionID);
101+
final String messageToLog;
102+
if (session != null) {
103+
messageToLog = message;
104+
} else {
105+
messageToLog = message + " sessionID=" + sessionID;
106+
}
107+
logWarning(session, messageToLog);
108+
}
109+
110+
/**
111+
* Logs a warning as a session event if the session is not NULL, otherwise
112+
* the general log is used.
113+
*
114+
* @param session the session to use
115+
* @param message the message to log
116+
*/
117+
static void logWarning(final Session session, String message) {
118+
if (session != null) {
119+
session.getLog().onWarnEvent(message);
120+
} else {
121+
log.warn(message);
122+
}
123+
}
71124

72125
}

‎quickfixj-core/src/main/java/quickfix/Session.java‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1066,31 +1066,31 @@ private void next(Message message, boolean isProcessingQueuedMessages) throws Fi
10661066
if (rejectInvalidMessage) {
10671067
throw e;
10681068
} else {
1069-
getLog().onErrorEvent("Warn: incoming message with " + e + ": " + getMessageToLog(message));
1069+
getLog().onWarnEvent("incoming message with " + e + ": " + getMessageToLog(message));
10701070
}
10711071
} catch (final FieldException e) {
10721072
if (message.isSetField(e.getField())) {
10731073
if (rejectInvalidMessage) {
10741074
throw e;
10751075
} else {
1076-
getLog().onErrorEvent(
1077-
"Warn: incoming message with incorrect field: "
1076+
getLog().onWarnEvent(
1077+
"incoming message with incorrect field: "
10781078
+ message.getField(e.getField()) + ": " + getMessageToLog(message));
10791079
}
10801080
} else {
10811081
if (rejectInvalidMessage) {
10821082
throw e;
10831083
} else {
1084-
getLog().onErrorEvent(
1085-
"Warn: incoming message with missing field: " + e.getField()
1084+
getLog().onWarnEvent(
1085+
"incoming message with missing field: " + e.getField()
10861086
+ ": " + e.getMessage() + ": " + getMessageToLog(message));
10871087
}
10881088
}
10891089
} catch (final FieldNotFound e) {
10901090
if (rejectInvalidMessage) {
10911091
throw e;
10921092
} else {
1093-
getLog().onErrorEvent("Warn: incoming " + e + ": " + getMessageToLog(message));
1093+
getLog().onWarnEvent("incoming " + e + ": " + getMessageToLog(message));
10941094
}
10951095
}
10961096
}
@@ -2017,7 +2017,7 @@ public void next() throws IOException {
20172017
disconnect("Timed out waiting for heartbeat", true);
20182018
stateListener.onHeartBeatTimeout(sessionID);
20192019
} else {
2020-
LOG.warn("Heartbeat failure detected but deactivated");
2020+
getLog().onWarnEvent("Heartbeat failure detected but deactivated");
20212021
}
20222022
} else {
20232023
if (state.isTestRequestNeeded()) {

‎quickfixj-core/src/main/java/quickfix/SessionSettings.java‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -410,7 +410,21 @@ public boolean getBool(String key) throws ConfigError, FieldConvertError {
410410
public boolean getBool(SessionID sessionID, String key) throws FieldConvertError, ConfigError {
411411
return BooleanConverter.convert(getString(sessionID, key));
412412
}
413+
414+
/**
415+
* Get a boolean setting from the default section if present or use default value.
416+
*/
417+
public boolean getBoolOrDefault(String key, boolean defaultValue) throws FieldConvertError, ConfigError {
418+
return isSetting(key) ? getBool(key) : defaultValue;
419+
}
413420

421+
/**
422+
* Get a boolean setting if present or use default value.
423+
*/
424+
public boolean getBoolOrDefault(SessionID sessionID, String key, boolean defaultValue) throws FieldConvertError, ConfigError {
425+
return isSetting(sessionID, key) ? getBool(sessionID, key) : defaultValue;
426+
}
427+
414428
/**
415429
* Sets a string-valued session setting.
416430
*

‎quickfixj-core/src/main/java/quickfix/mina/IoSessionResponder.java‎

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,7 @@
2121

2222
import org.apache.mina.core.future.WriteFuture;
2323
import org.apache.mina.core.session.IoSession;
24-
import org.slf4j.Logger;
25-
import org.slf4j.LoggerFactory;
24+
import quickfix.LogUtil;
2625
import quickfix.Responder;
2726
import quickfix.Session;
2827

@@ -34,7 +33,6 @@
3433
* the MINA networking code.
3534
*/
3635
public class IoSessionResponder implements Responder {
37-
private final Logger log = LoggerFactory.getLogger(getClass());
3836
private final IoSession ioSession;
3937
private final boolean synchronousWrites;
4038
private final long synchronousWriteTimeout;
@@ -51,9 +49,8 @@ public IoSessionResponder(IoSession session, boolean synchronousWrites, long syn
5149
public boolean send(String data) {
5250
// Check for and disconnect slow consumers.
5351
if (maxScheduledWriteRequests > 0 && ioSession.getScheduledWriteMessages() >= maxScheduledWriteRequests) {
54-
Session qfjSession = (Session) ioSession.getAttribute(SessionConnector.QF_SESSION);
5552
try {
56-
qfjSession.disconnect("Slow consumer", true);
53+
getQFJSession().disconnect("Slow consumer", true);
5754
} catch (IOException e) {
5855
}
5956
return false;
@@ -64,11 +61,11 @@ public boolean send(String data) {
6461
if (synchronousWrites) {
6562
try {
6663
if (!future.awaitUninterruptibly(synchronousWriteTimeout)) {
67-
log.error("Synchronous write timed out after {}ms", synchronousWriteTimeout);
64+
getQFJSession().getLog().onErrorEvent("Synchronous write timed out after " + synchronousWriteTimeout + "ms.");
6865
return false;
6966
}
7067
} catch (RuntimeException e) {
71-
log.error("Synchronous write failed: {}", e.getMessage());
68+
LogUtil.logThrowable(getQFJSession().getSessionID(), "Synchronous write failed.", e);
7269
return false;
7370
}
7471
}
@@ -81,7 +78,7 @@ public void disconnect() {
8178
// by the following call. We are using a minimal
8279
// threading model and calling join will prevent the
8380
// close event from being processed by this thread (if
84-
// this thread is the MINA IO processor thread.
81+
// this thread is the MINA IO processor thread).
8582
ioSession.closeOnFlush();
8683
ioSession.setAttribute(SessionConnector.QFJ_RESET_IO_CONNECTOR, Boolean.TRUE);
8784
}
@@ -98,4 +95,9 @@ public String getRemoteAddress() {
9895
IoSession getIoSession() {
9996
return ioSession;
10097
}
98+
99+
private Session getQFJSession() {
100+
return (Session) ioSession.getAttribute(SessionConnector.QF_SESSION);
101+
}
102+
101103
}

‎quickfixj-core/src/main/java/quickfix/mina/SessionConnector.java‎

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -464,15 +464,12 @@ public static void closeManagedSessionsAndDispose(IoService ioService, boolean a
464464
}
465465

466466
protected boolean isContinueInitOnError() {
467-
boolean continueInitOnError = false;
468-
if (settings.isSetting(SessionFactory.SETTING_CONTINUE_INIT_ON_ERROR)) {
469-
try {
470-
continueInitOnError = settings.getBool(SessionFactory.SETTING_CONTINUE_INIT_ON_ERROR);
471-
} catch (ConfigError | FieldConvertError ex) {
472-
// ignore and return default
473-
}
467+
try {
468+
return settings.getBoolOrDefault(SessionFactory.SETTING_CONTINUE_INIT_ON_ERROR, false);
469+
} catch (FieldConvertError | ConfigError ex) {
470+
// ignore and return default
474471
}
475-
return continueInitOnError;
472+
return false;
476473
}
477474

478475
}

‎quickfixj-core/src/main/java/quickfix/mina/acceptor/AbstractSocketAcceptor.java‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
import quickfix.DefaultSessionFactory;
3131
import quickfix.FieldConvertError;
3232
import quickfix.LogFactory;
33+
import quickfix.LogUtil;
3334
import quickfix.MessageFactory;
3435
import quickfix.MessageStoreFactory;
3536
import quickfix.RuntimeError;
@@ -117,6 +118,7 @@ protected synchronized void startAcceptingConnections() throws ConfigError {
117118
ioAcceptor.bind(socketDescriptor.getAddress());
118119
log.info("Listening for connections at {} for session(s) {}", address, socketDescriptor.getAcceptedSessions().keySet());
119120
} catch (IOException | GeneralSecurityException | ConfigError e) {
121+
// we cannot log for a specific session here
120122
if (continueInitOnError) {
121123
log.warn("error during session initialization for session(s) {}, continuing...", socketDescriptor.getAcceptedSessions().keySet(), e);
122124
} else {
@@ -183,8 +185,6 @@ && getSettings().getBool(sessionID, SSLSupport.SETTING_USE_SSL)) {
183185
if (acceptTransportType == ProtocolFactory.SOCKET) {
184186
useSSL = true;
185187
sslConfig = SSLSupport.getSslConfig(getSettings(), sessionID);
186-
} else {
187-
log.warn("SSL will not be enabled for transport type={}, session={}", acceptTransportType, sessionID);
188188
}
189189
}
190190

@@ -214,6 +214,11 @@ && getSettings().getBool(sessionID, SSLSupport.SETTING_USE_SSL)) {
214214
descriptor.acceptSession(session);
215215
allSessions.put(sessionID, session);
216216
}
217+
218+
if (acceptTransportType != ProtocolFactory.SOCKET
219+
&& getSettings().getBoolOrDefault(sessionID, SSLSupport.SETTING_USE_SSL, false)) {
220+
LogUtil.logWarning(sessionID, "SSL is only supported for transport type SOCKET and will not be enabled for transport type=" + acceptTransportType);
221+
}
217222
}
218223

219224
private boolean equals(Object object1, Object object2) {
@@ -245,7 +250,7 @@ private void createSessions(SessionSettings settings, boolean continueInitOnErro
245250
}
246251
} catch (Throwable t) {
247252
if (continueInitOnError) {
248-
log.warn("error during session initialization for {}, continuing...", sessionID, t);
253+
LogUtil.logWarning(sessionID, "error during session initialization, continuing...", t);
249254
} else {
250255
throw t instanceof ConfigError ? (ConfigError) t : new ConfigError(
251256
"error during session initialization", t);
@@ -329,6 +334,7 @@ public StaticAcceptorSessionProvider(final Map<SessionID, Session> acceptorSessi
329334
this.acceptorSessions = acceptorSessions;
330335
}
331336

337+
@Override
332338
public Session getSession(SessionID sessionID, SessionConnector connector) {
333339
return acceptorSessions.get(sessionID);
334340
}
@@ -349,6 +355,7 @@ public DefaultAcceptorSessionProvider(Map<SessionID, Session> acceptorSessions)
349355
this.acceptorSessions = acceptorSessions;
350356
}
351357

358+
@Override
352359
public Session getSession(SessionID sessionID, SessionConnector ignored) {
353360
Session session = acceptorSessions.get(sessionID);
354361
if (session == null) {

‎quickfixj-core/src/main/java/quickfix/mina/initiator/AbstractSocketInitiator.java‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
import quickfix.FieldConvertError;
3030
import quickfix.Initiator;
3131
import quickfix.LogFactory;
32+
import quickfix.LogUtil;
3233
import quickfix.MessageFactory;
3334
import quickfix.MessageStoreFactory;
3435
import quickfix.Session;
@@ -123,7 +124,7 @@ private void createInitiator(final Session session, final boolean continueInitOn
123124
throw new ConfigError("Must specify at least one socket address");
124125
}
125126

126-
SocketAddress localAddress = getLocalAddress(settings, sessionID);
127+
SocketAddress localAddress = getLocalAddress(settings, session);
127128

128129
final NetworkingOptions networkingOptions = new NetworkingOptions(getSettings()
129130
.getSessionProperties(sessionID, true));
@@ -182,18 +183,19 @@ && getSettings().isSetting(sessionID, Initiator.SETTING_PROXY_DOMAIN)) {
182183
initiators.add(ioSessionInitiator);
183184
} catch (ConfigError e) {
184185
if (continueInitOnError) {
185-
log.warn("error during session initialization for {}, continuing...", sessionID, e);
186+
LogUtil.logWarning(sessionID, "error during session initialization, continuing... ", e);
186187
} else {
187188
throw e;
188189
}
189190
}
190191
}
191192

192193
// QFJ-482
193-
private SocketAddress getLocalAddress(SessionSettings settings, final SessionID sessionID)
194+
private SocketAddress getLocalAddress(SessionSettings settings, final Session session)
194195
throws ConfigError, FieldConvertError {
195196
// Check if use of socket local/bind address
196197
SocketAddress localAddress = null;
198+
SessionID sessionID = session.getSessionID();
197199
if (settings.isSetting(sessionID, Initiator.SETTING_SOCKET_LOCAL_HOST)) {
198200
String host = settings.getString(sessionID, Initiator.SETTING_SOCKET_LOCAL_HOST);
199201
if ("localhost".equals(host)) {
@@ -204,7 +206,7 @@ private SocketAddress getLocalAddress(SessionSettings settings, final SessionID
204206
port = (int) settings.getLong(sessionID, Initiator.SETTING_SOCKET_LOCAL_PORT);
205207
}
206208
localAddress = ProtocolFactory.createSocketAddress(ProtocolFactory.SOCKET, host, port);
207-
log.info("Using initiator local host: {}", localAddress);
209+
session.getLog().onEvent("Using initiator local host: " + localAddress);
208210
}
209211
return localAddress;
210212
}
@@ -222,7 +224,7 @@ private void createSessions(boolean continueInitOnError) throws ConfigError, Fie
222224
}
223225
} catch (final Throwable e) {
224226
if (continueInitOnError) {
225-
log.warn("error during session initialization for {}, continuing...", sessionID, e);
227+
LogUtil.logWarning(sessionID, "error during session initialization, continuing...", e);
226228
} else {
227229
throw e instanceof ConfigError ? (ConfigError) e : new ConfigError(
228230
"error during session initialization", e);

‎quickfixj-core/src/test/java/quickfix/SessionSettingsTest.java‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,11 @@ public void testSettings() throws Exception {
119119
assertFalse("wrong setting", settings.getBool(sessionID1, "TestBoolFalse"));
120120
settings.setBool(sessionID3, "TestBool", true);
121121
assertTrue("wrong settings", settings.getBool(sessionID3, "TestBool"));
122-
122+
assertFalse(settings.getBoolOrDefault(sessionID3, "unknownSetting", false));
123+
assertTrue(settings.getBoolOrDefault(sessionID3, "unknownSetting", true));
124+
assertTrue(settings.getBoolOrDefault(sessionID3, "TestBool", false));
125+
assertTrue(settings.getBoolOrDefault(sessionID3, "TestBool", true));
126+
123127
settings.setString(sessionID3, "TestString", "foo");
124128
assertEquals("wrong setting", "foo", settings.getString(sessionID3, "TestString"));
125129

0 commit comments

Comments
 (0)