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

Fix #1417 Change socket mode ping/pong from debug to trace #1421

Merged
merged 1 commit into from
Jan 15, 2025
+22 −22
Merged

Fix #1417 Change socket mode ping/pong from debug to trace #1421

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
8 changes: 4 additions & 4 deletions slack-api-client/src/main/java/com/slack/api/socket_mode/SocketModeClient.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,15 +185,15 @@
final AtomicLong nextInvocationMillis = new AtomicLong(System.currentTimeMillis());
executorService.scheduleWithFixedDelay(() -> {
long startMillis = System.currentTimeMillis();
if (getLogger().isDebugEnabled()) {
getLogger().debug("Checking the current session...");
if (getLogger().isTraceEnabled()) {
getLogger().trace("Checking the current session...");

Check warning on line 189 in slack-api-client/src/main/java/com/slack/api/socket_mode/SocketModeClient.java

View check run for this annotation

Codecov / codecov/patch

slack-api-client/src/main/java/com/slack/api/socket_mode/SocketModeClient.java#L189 

Added line #L189 was not covered by tests
}
if (isAutoReconnectEnabled() && nextInvocationMillis.get() <= System.currentTimeMillis()) {
nextInvocationMillis.set(maintainCurrentSession());
}
if (getLogger().isDebugEnabled()) {
if (getLogger().isTraceEnabled()) {
long spentMillis = System.currentTimeMillis() - startMillis;
getLogger().debug("The session maintenance completed in {} milliseconds", spentMillis);
getLogger().trace("The session maintenance completed in {} milliseconds", spentMillis);

Check warning on line 196 in slack-api-client/src/main/java/com/slack/api/socket_mode/SocketModeClient.java

View check run for this annotation

Codecov / codecov/patch

slack-api-client/src/main/java/com/slack/api/socket_mode/SocketModeClient.java#L196 

Added line #L196 was not covered by tests
}
}, 5_000L, intervalMillis, TimeUnit.MILLISECONDS);
if (getLogger().isDebugEnabled()) {
Expand Down
12 changes: 6 additions & 6 deletions ...k-api-client/src/main/java/com/slack/api/socket_mode/impl/SocketModeClientJavaWSImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,15 +130,15 @@
public boolean verifyConnection() {
if (this.currentSession != null && this.currentSession.isOpen()) {
try {
if (getLogger().isDebugEnabled()) {
getLogger().debug("Sending a ping message");
if (getLogger().isTraceEnabled()) {
getLogger().trace("Sending a ping message");

Check warning on line 134 in slack-api-client/src/main/java/com/slack/api/socket_mode/impl/SocketModeClientJavaWSImpl.java

View check run for this annotation

Codecov / codecov/patch

slack-api-client/src/main/java/com/slack/api/socket_mode/impl/SocketModeClientJavaWSImpl.java#L134 

Added line #L134 was not covered by tests
}
this.currentSession.sendPing();
long waitMillis = 0L;
while (waitMillis <= 3_000L) {
if (this.currentSession.isPongReceived()) {
if (getLogger().isDebugEnabled()) {
getLogger().debug("Received a pong message");
if (getLogger().isTraceEnabled()) {
getLogger().trace("Received a pong message");

Check warning on line 141 in slack-api-client/src/main/java/com/slack/api/socket_mode/impl/SocketModeClientJavaWSImpl.java

View check run for this annotation

Codecov / codecov/patch

slack-api-client/src/main/java/com/slack/api/socket_mode/impl/SocketModeClientJavaWSImpl.java#L141 

Added line #L141 was not covered by tests
}
return true;
}
Expand All @@ -152,8 +152,8 @@
e.getMessage()
);
}
if (getLogger().isDebugEnabled()) {
getLogger().debug("Failed to receive a pong message");
if (getLogger().isTraceEnabled()) {
getLogger().trace("Failed to receive a pong message");

Check warning on line 156 in slack-api-client/src/main/java/com/slack/api/socket_mode/impl/SocketModeClientJavaWSImpl.java

View check run for this annotation

Codecov / codecov/patch

slack-api-client/src/main/java/com/slack/api/socket_mode/impl/SocketModeClientJavaWSImpl.java#L156 

Added line #L156 was not covered by tests
}
Comment on lines +155 to 157
Copy link

Choose a reason for hiding this comment

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

Consider making this a warning.

Suggested change
if (getLogger().isTraceEnabled()) {
getLogger().trace("Failed to receive a pong message");
}
getLogger().warn("Failed to receive a pong message");

Copy link
Member Author

Choose a reason for hiding this comment

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

When taking time until successfully connecting, having these at WARN level every 100 milliseconds can be noisy, so we won't change the log level here.

}
return false;
Expand Down
12 changes: 6 additions & 6 deletions slack-api-client/src/main/java/com/slack/api/socket_mode/impl/SocketModeClientTyrusImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -187,8 +187,8 @@
public boolean verifyConnection() {
if (this.currentSession != null && this.currentSession.isOpen()) {
String ping = "ping-pong_" + currentSession.getId();
if (getLogger().isDebugEnabled()) {
getLogger().debug("Sending a ping message: {}", ping);
if (getLogger().isTraceEnabled()) {
getLogger().trace("Sending a ping message: {}", ping);

Check warning on line 191 in slack-api-client/src/main/java/com/slack/api/socket_mode/impl/SocketModeClientTyrusImpl.java

View check run for this annotation

Codecov / codecov/patch

slack-api-client/src/main/java/com/slack/api/socket_mode/impl/SocketModeClientTyrusImpl.java#L191 

Added line #L191 was not covered by tests
}
ByteBuffer pingBytes = ByteBuffer.wrap(ping.getBytes());
try {
Expand All @@ -199,8 +199,8 @@
while (waitMillis <= 3_000L) {
String pong = latestPong.getAndSet(null);
if (pong != null && pong.equals(ping)) {
if (getLogger().isDebugEnabled()) {
getLogger().debug("Received a pong message: {}", ping);
if (getLogger().isTraceEnabled()) {
getLogger().trace("Received a pong message: {}", ping);

Check warning on line 203 in slack-api-client/src/main/java/com/slack/api/socket_mode/impl/SocketModeClientTyrusImpl.java

View check run for this annotation

Codecov / codecov/patch

slack-api-client/src/main/java/com/slack/api/socket_mode/impl/SocketModeClientTyrusImpl.java#L203 

Added line #L203 was not covered by tests
}
return true;
}
Expand All @@ -213,8 +213,8 @@
this.currentSession.getId(),
e.getMessage());
}
if (getLogger().isDebugEnabled()) {
getLogger().debug("Failed to receive a pong message: {}", ping);
if (getLogger().isTraceEnabled()) {
getLogger().trace("Failed to receive a pong message: {}", ping);

Check warning on line 217 in slack-api-client/src/main/java/com/slack/api/socket_mode/impl/SocketModeClientTyrusImpl.java

View check run for this annotation

Codecov / codecov/patch

slack-api-client/src/main/java/com/slack/api/socket_mode/impl/SocketModeClientTyrusImpl.java#L217 

Added line #L217 was not covered by tests
}
Comment on lines +216 to 218
Copy link

Choose a reason for hiding this comment

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

Consider making this a warning.

Suggested change
if (getLogger().isTraceEnabled()) {
getLogger().trace("Failed to receive a pong message: {}", ping);
}
getLogger().warn("Failed to receive a pong message: {}", ping);

}
return false;
Expand Down
12 changes: 6 additions & 6 deletions ...rc/main/java/com/slack/api/jakarta_socket_mode/impl/JakartaSocketModeClientTyrusImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -187,8 +187,8 @@
public boolean verifyConnection() {
if (this.currentSession != null && this.currentSession.isOpen()) {
String ping = "ping-pong_" + currentSession.getId();
if (getLogger().isDebugEnabled()) {
getLogger().debug("Sending a ping message: {}", ping);
if (getLogger().isTraceEnabled()) {
getLogger().trace("Sending a ping message: {}", ping);

Check warning on line 191 in slack-jakarta-socket-mode-client/src/main/java/com/slack/api/jakarta_socket_mode/impl/JakartaSocketModeClientTyrusImpl.java

View check run for this annotation

Codecov / codecov/patch

slack-jakarta-socket-mode-client/src/main/java/com/slack/api/jakarta_socket_mode/impl/JakartaSocketModeClientTyrusImpl.java#L191 

Added line #L191 was not covered by tests
}
ByteBuffer pingBytes = ByteBuffer.wrap(ping.getBytes());
try {
Expand All @@ -199,8 +199,8 @@
while (waitMillis <= 3_000L) {
String pong = latestPong.getAndSet(null);
if (pong != null && pong.equals(ping)) {
if (getLogger().isDebugEnabled()) {
getLogger().debug("Received a pong message: {}", ping);
if (getLogger().isTraceEnabled()) {
getLogger().trace("Received a pong message: {}", ping);

Check warning on line 203 in slack-jakarta-socket-mode-client/src/main/java/com/slack/api/jakarta_socket_mode/impl/JakartaSocketModeClientTyrusImpl.java

View check run for this annotation

Codecov / codecov/patch

slack-jakarta-socket-mode-client/src/main/java/com/slack/api/jakarta_socket_mode/impl/JakartaSocketModeClientTyrusImpl.java#L203 

Added line #L203 was not covered by tests
}
return true;
}
Expand All @@ -213,8 +213,8 @@
this.currentSession.getId(),
e.getMessage());
}
if (getLogger().isDebugEnabled()) {
getLogger().debug("Failed to receive a pong message: {}", ping);
if (getLogger().isTraceEnabled()) {
getLogger().trace("Failed to receive a pong message: {}", ping);

Check warning on line 217 in slack-jakarta-socket-mode-client/src/main/java/com/slack/api/jakarta_socket_mode/impl/JakartaSocketModeClientTyrusImpl.java

View check run for this annotation

Codecov / codecov/patch

slack-jakarta-socket-mode-client/src/main/java/com/slack/api/jakarta_socket_mode/impl/JakartaSocketModeClientTyrusImpl.java#L217 

Added line #L217 was not covered by tests
}
}
return false;
Expand Down
Loading