On Tue, 3 Dec 2024 00:40:24 GMT, Tim Prinzing <[email protected]> wrote:
>> Adds a JFR event for socket connect operations.
>>
>> Existing tests TestSocketEvents and TestSocketChannelEvents modified to also
>> check for connect events.
>
> Tim Prinzing has updated the pull request incrementally with one additional
> commit since the last revision:
>
> split socket connect failure out to its own event.
src/java.base/share/classes/jdk/internal/event/SocketConnectFailedEvent.java
line 145:
> 143: if (shouldCommit(duration)) {
> 144: commit(start, duration, host, address.getHostAddress(),
> port, connectEx.getMessage());
> 145: }
Would it be better to pass `connectEx.toString()` here to capture the type of
the exception and avoid the awkward case where the message could be the empty
string or null? Same in the other offer() method above...
src/jdk.jfr/share/classes/jdk/jfr/events/SocketConnectFailedEvent.java line 52:
> 50: @Label("Connect Exception Message")
> 51: public String connectExceptionMessage;
> 52: }
There seem to be a missing newline at the end of this file
src/jdk.jfr/share/conf/jfr/default.jfc line 746:
> 744: <setting name="enabled">true</setting>
> 745: <setting name="stackTrace">true</setting>
> 746: <setting name="threshold" control="socket-threshold">20
> ms</setting>
Does this event needs a threshold?
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/21528#discussion_r1867888476
PR Review Comment: https://git.openjdk.org/jdk/pull/21528#discussion_r1867909881
PR Review Comment: https://git.openjdk.org/jdk/pull/21528#discussion_r1867912497
