Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

DATAGO-73953: use slf4j instead of apache commons logging #330

Merged
merged 1 commit into from
Aug 28, 2024
Merged

DATAGO-73953: use slf4j instead of apache commons logging #330

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 4 additions & 6 deletions ...rc/main/java/com/solace/spring/cloud/stream/binder/health/base/SolaceHealthIndicator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
package com.solace.spring.cloud.stream.binder.health.base;

import org.apache.commons.lang3.StringUtils;
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.boot.actuate.health.Health;
import org.springframework.boot.actuate.health.HealthIndicator;
import org.springframework.boot.actuate.health.Status;
Expand All @@ -16,12 +16,10 @@ public class SolaceHealthIndicator implements HealthIndicator {
private static final String INFO = "info";
private static final String RESPONSE_CODE = "responseCode";
private volatile Health health;
private static final Log logger = LogFactory.getLog(SolaceHealthIndicator.class);
private static final Logger LOGGER = LoggerFactory.getLogger(SolaceHealthIndicator.class);

private static void logDebugStatus(String status) {
if (logger.isDebugEnabled()) {
logger.debug(String.format("Solace connection/flow status is %s", status));
}
LOGGER.debug("Solace connection/flow status is {}", status);
}
protected void healthUp() {
health = Health.up().build();
Expand Down
10 changes: 4 additions & 6 deletions ...java/com/solace/spring/cloud/stream/binder/health/handlers/SolaceSessionEventHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,13 @@
import com.solacesystems.jcsmp.JCSMPProperties;
import com.solacesystems.jcsmp.SessionEventArgs;
import com.solacesystems.jcsmp.SolaceSessionOAuth2TokenProvider;
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.lang.Nullable;

public class SolaceSessionEventHandler extends DefaultSolaceOAuth2SessionEventHandler {
private final SessionHealthIndicator sessionHealthIndicator;
private static final Log logger = LogFactory.getLog(SolaceSessionEventHandler.class);
private static final Logger LOGGER = LoggerFactory.getLogger(SolaceSessionEventHandler.class);

public SolaceSessionEventHandler(JCSMPProperties jcsmpProperties,
@Nullable SolaceSessionOAuth2TokenProvider solaceSessionOAuth2TokenProvider,
Expand All @@ -22,9 +22,7 @@ public SolaceSessionEventHandler(JCSMPProperties jcsmpProperties,

@Override
public void handleEvent(SessionEventArgs eventArgs) {
if (logger.isDebugEnabled()) {
logger.debug(String.format("Received Solace JCSMP Session event [%s]", eventArgs));
}
LOGGER.debug("Received Solace JCSMP Session event [{}]", eventArgs);
super.handleEvent(eventArgs);
switch (eventArgs.getEvent()) {
case RECONNECTED -> this.sessionHealthIndicator.up();
Expand Down
20 changes: 7 additions & 13 deletions .../java/com/solace/spring/cloud/stream/binder/health/indicators/SessionHealthIndicator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@
import com.solace.spring.cloud.stream.binder.health.base.SolaceHealthIndicator;
import com.solace.spring.cloud.stream.binder.properties.SolaceSessionHealthProperties;
import com.solacesystems.jcsmp.SessionEventArgs;
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.lang.Nullable;

import java.util.Optional;
Expand All @@ -15,7 +15,7 @@ public class SessionHealthIndicator extends SolaceHealthIndicator {
private final AtomicInteger reconnectCount = new AtomicInteger(0);
private final SolaceSessionHealthProperties solaceHealthSessionProperties;
private final ReentrantLock writeLock = new ReentrantLock();
private static final Log logger = LogFactory.getLog(SessionHealthIndicator.class);
private static final Logger LOGGER = LoggerFactory.getLogger(SessionHealthIndicator.class);

public SessionHealthIndicator(SolaceSessionHealthProperties solaceHealthSessionProperties) {
this.solaceHealthSessionProperties = solaceHealthSessionProperties;
Expand All @@ -24,9 +24,7 @@ public SessionHealthIndicator(SolaceSessionHealthProperties solaceHealthSessionP
public void up() {
writeLock.lock();
try {
if (logger.isTraceEnabled()) {
logger.trace("Reset reconnect count");
}
LOGGER.trace("Reset reconnect count");
this.reconnectCount.set(0);
super.healthUp();
} finally {
Expand All @@ -42,10 +40,8 @@ public void reconnecting(@Nullable SessionEventArgs eventArgs) {
.filter(maxReconnectAttempts -> maxReconnectAttempts > 0)
.filter(maxReconnectAttempts -> reconnectAttempt > maxReconnectAttempts)
.isPresent()) {
if (logger.isDebugEnabled()) {
logger.debug(String.format("Solace connection reconnect attempt %s > %s, changing state to down",
reconnectAttempt, solaceHealthSessionProperties.getReconnectAttemptsUntilDown()));
}
LOGGER.debug("Solace connection reconnect attempt {} > {}, changing state to down",
reconnectAttempt, solaceHealthSessionProperties.getReconnectAttemptsUntilDown());
this.down(eventArgs, false);
return;
}
Expand All @@ -64,9 +60,7 @@ public void down(@Nullable SessionEventArgs eventArgs, boolean resetReconnectCou
writeLock.lock();
try {
if (resetReconnectCount) {
if (logger.isTraceEnabled()) {
logger.trace("Reset reconnect count");
}
LOGGER.trace("Reset reconnect count");
this.reconnectCount.set(0);
}
super.healthDown(eventArgs);
Expand Down
10 changes: 5 additions & 5 deletions ...in/java/com/solace/spring/cloud/stream/binder/inbound/BasicInboundXMLMessageListener.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@
import com.solace.spring.cloud.stream.binder.properties.SolaceConsumerProperties;
import com.solace.spring.cloud.stream.binder.util.FlowReceiverContainer;
import com.solace.spring.cloud.stream.binder.util.SolaceAcknowledgmentException;
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.cloud.stream.binder.ExtendedConsumerProperties;
import org.springframework.cloud.stream.provisioning.ConsumerDestination;
import org.springframework.core.AttributeAccessor;
Expand All @@ -24,7 +24,7 @@
public class BasicInboundXMLMessageListener extends InboundXMLMessageListener {
private final BiFunction<Message<?>, RuntimeException, Boolean> errorHandlerFunction;

private static final Log logger = LogFactory.getLog(BasicInboundXMLMessageListener.class);
private static final Logger LOGGER = LoggerFactory.getLogger(BasicInboundXMLMessageListener.class);

BasicInboundXMLMessageListener(FlowReceiverContainer flowReceiverContainer,
ConsumerDestination consumerDestination,
Expand Down Expand Up @@ -63,9 +63,9 @@ void handleMessage(Supplier<Message<?>> messageSupplier, Consumer<Message<?>> se
if (processedByErrorHandler) {
AckUtils.autoAck(acknowledgmentCallback);
} else {
logger.warn(String.format("Failed to map %s to a Spring Message and no error channel " +
LOGGER.warn("Failed to map {} to a Spring Message and no error channel " +
"was configured. Message will be rejected.", isBatched ? "a batch of XMLMessages" :
"an XMLMessage"), e);
"an XMLMessage", e);
if (!SolaceAckUtil.republishToErrorQueue(acknowledgmentCallback)) {
AckUtils.requeue(acknowledgmentCallback);
}
Expand Down
54 changes: 23 additions & 31 deletions ...rc/main/java/com/solace/spring/cloud/stream/binder/inbound/InboundXMLMessageListener.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,9 @@
import com.solacesystems.jcsmp.StaleSessionException;
import com.solacesystems.jcsmp.XMLMessage;
import org.apache.commons.lang3.exception.ExceptionUtils;
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.slf4j.event.Level;
import org.springframework.cloud.stream.binder.ExtendedConsumerProperties;
import org.springframework.cloud.stream.binder.RequeueCurrentMessageException;
import org.springframework.cloud.stream.provisioning.ConsumerDestination;
Expand Down Expand Up @@ -53,7 +54,7 @@ abstract class InboundXMLMessageListener implements Runnable {
private final AtomicBoolean stopFlag = new AtomicBoolean(false);
private final Supplier<Boolean> remoteStopFlag;

private static final Log logger = LogFactory.getLog(InboundXMLMessageListener.class);
private static final Logger LOGGER = LoggerFactory.getLogger(InboundXMLMessageListener.class);

InboundXMLMessageListener(FlowReceiverContainer flowReceiverContainer,
ConsumerDestination consumerDestination,
Expand Down Expand Up @@ -91,18 +92,18 @@ public void run() {
try {
receive();
} catch (RuntimeException | UnboundFlowReceiverContainerException e) {
logger.warn(String.format("Exception received while consuming messages from destination %s",
consumerDestination.getName()), e);
LOGGER.warn("Exception received while consuming messages from destination {}",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this not going to log as expected: e not logged

/**
 * Log a message at the WARN level according to the specified format
 * and arguments.
 * 
 * <p>This form avoids superfluous object creation when the logger
 * is disabled for the WARN level. 
 *
 * @param format the format string
 * @param arg1   the first argument
 * @param arg2   the second argument
 */
public void warn(String format, Object arg1, Object arg2);

Copy link
Collaborator Author

@Nephery Nephery Aug 28, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can use parameterized logging with a throwable as the last parameter. See https://www.slf4j.org/faq.html#paramException (Spring Boot 3.3.3 uses SLF4J 2.0.16).

For example, when I do:

LOGGER.warn("foo {}", "bar", new RuntimeException("test error", new RuntimeException("cause")));

I get the output:

2024-08-28T14:34:34.396-04:00  WARN   --- [           main] c.s.s.c.s.b.util.XMLMessageMapperTest    : foo bar

java.lang.RuntimeException: test error
Caused by: java.lang.RuntimeException: cause

consumerDestination.getName(), e);
}
}
} catch (StaleSessionException e) {
logger.error("Session has lost connection", e);
LOGGER.error("Session has lost connection", e);
} catch (Throwable t) {
logger.error(String.format("Received unexpected error while consuming from destination %s",
consumerDestination.getName()), t);
LOGGER.error("Received unexpected error while consuming from destination {}",
consumerDestination.getName(), t);
throw t;
} finally {
logger.info(String.format("Closing flow receiver to destination %s", consumerDestination.getName()));
LOGGER.info("Closing flow receiver to destination {}", consumerDestination.getName());
flowReceiverContainer.unbind();
}
}
Expand All @@ -123,13 +124,11 @@ private void receive() throws UnboundFlowReceiverContainerException, StaleSessio
} catch (StaleSessionException e) {
throw e;
} catch (JCSMPException e) {
String msg = String.format("Received error while trying to read message from endpoint %s",
flowReceiverContainer.getEndpointName());
if ((e instanceof JCSMPTransportException || e instanceof ClosedFacilityException) && !keepPolling()) {
logger.debug(msg, e);
} else {
logger.warn(msg, e);
}
LOGGER.atLevel((e instanceof JCSMPTransportException || e instanceof ClosedFacilityException) &&
!keepPolling() ? Level.DEBUG : Level.WARN)
.setCause(e)
.log("Received error while trying to read message from endpoint {}",
flowReceiverContainer.getEndpointName());
return;
}

Expand Down Expand Up @@ -165,22 +164,19 @@ private void processMessage(MessageContainer messageContainer) {
} catch (Exception e) {
try {
if (ExceptionUtils.indexOfType(e, RequeueCurrentMessageException.class) > -1) {
logger.warn(String.format(
"Exception thrown while processing XMLMessage %s. Message will be requeued.",
bytesXMLMessage.getMessageId()), e);
LOGGER.warn("Exception thrown while processing XMLMessage {}. Message will be requeued.",
bytesXMLMessage.getMessageId(), e);
AckUtils.requeue(acknowledgmentCallback);
} else {
logger.warn(String.format(
"Exception thrown while processing XMLMessage %s. Message will be requeued.",
bytesXMLMessage.getMessageId()), e);
LOGGER.warn("Exception thrown while processing XMLMessage {}. Message will be requeued.",
bytesXMLMessage.getMessageId(), e);
if (!SolaceAckUtil.republishToErrorQueue(acknowledgmentCallback)) {
AckUtils.requeue(acknowledgmentCallback);
}
}
} catch (SolaceAcknowledgmentException e1) {
e1.addSuppressed(e);
logger.warn(String.format("Exception thrown while re-queuing XMLMessage %s.",
bytesXMLMessage.getMessageId()), e1);
LOGGER.warn("Exception thrown while re-queuing XMLMessage {}.", bytesXMLMessage.getMessageId(), e1);
throw e1;
}
}
Expand Down Expand Up @@ -212,21 +208,17 @@ private void processBatchIfAvailable() {
} catch (Exception e) {
try {
if (ExceptionUtils.indexOfType(e, RequeueCurrentMessageException.class) > -1) {
if (logger.isWarnEnabled()) {
logger.warn("Exception thrown while processing batch. Batch's message will be requeued.", e);
}
LOGGER.warn("Exception thrown while processing batch. Batch's message will be requeued.", e);
AckUtils.requeue(acknowledgmentCallback);
} else {
if (logger.isWarnEnabled()) {
logger.warn("Exception thrown while processing batch. Batch's messages will be requeued.", e);
}
LOGGER.warn("Exception thrown while processing batch. Batch's messages will be requeued.", e);
if (!SolaceAckUtil.republishToErrorQueue(acknowledgmentCallback)) {
AckUtils.requeue(acknowledgmentCallback);
}
}
} catch (SolaceAcknowledgmentException e1) {
e1.addSuppressed(e);
logger.warn("Exception thrown while re-queuing batch.", e1);
LOGGER.warn("Exception thrown while re-queuing batch.", e1);
throw e1;
}
} finally {
Expand Down
Loading