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.

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

Issue #2504 - expose more WebSocket details in jetty server dump #7058

Merged
merged 1 commit into from
Dec 2, 2021

Conversation

lachlan-roberts
Copy link
Contributor

closes #2504

Signed-off-by: Lachlan Roberts <lachlan@webtide.com>
Copy link
Contributor

@joakime joakime left a comment

Choose a reason for hiding this comment

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

Please fix the format type.
Do we have an existing unit test that can trigger the new code in this PR?

public String toString()
{
return String.format("%s@%x{idleTimeout=%s, writeTimeout=%s, autoFragment=%s, maxFrameSize=%s, " +
"inputBufferSize=%s, outputBufferSize=%s, maxBinaryMessageSize=%s, maxTextMessageSize=%s, maxOutgoingFrames=%s}",
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are these numbers formatted with %s and not %d ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think there's any major difference between %d and %s for integers unless you want some special formatting.

These are also class types and not the primitive ones, so I thought %s might be better, they are Duration, Boolean, Long, Integer and the values are null if no configuration is set for them.

@lachlan-roberts
Copy link
Contributor Author

Do we have an existing unit test that can trigger the new code in this PR?

@joakime I don't think there are currently any websocket tests for the output of the dump, I could write some if you want.

@lachlan-roberts lachlan-roberts merged commit 33c9536 into jetty-10.0.x Dec 2, 2021
@lachlan-roberts lachlan-roberts deleted the jetty-10.0.x-2504-WebSocketDump branch December 2, 2021 08:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Expose more WebSocket details in JMX and Server Dump
2 participants