On Sat, 23 Nov 2024 08:02:42 GMT, Alan Bateman <al...@openjdk.org> wrote:
>> Tim Prinzing has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Added more tests for socket connect events.
>>
>> - SocketAdapter connect
>> - SocketAdapter connect with exception
>> - Socket connect with exception
>> - SocketChannel connect with exception
>> - SocketChannel non-blocking connect
>> - SocketChannel non-blocking connect with exception
>
> src/jdk.jfr/share/classes/jdk/jfr/events/SocketConnectEvent.java line 38:
>
>> 36: @Label("Socket Connect")
>> 37: @Category("Java Application")
>> 38: @Description("Connecting a socket")
>
> I wonder if we can improve on this description. The event is recorded when a
> connection is established or cannot be established so it's more like "Socket
> Connection".
As I understand it, the event succeeds if exceptionMessage is null.
Perhaps this should be made more explicit with a failed field. Alternatively,
there could be two events: one for success and one for failure. What is the
typical duration of a failed event? If it is above 10-20 ms, two events might
not be as useful since all failures will be recorded anyway.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/21528#discussion_r1855573527
