On Wed, 6 Nov 2024 00:15:44 GMT, Tim Prinzing <tprinz...@openjdk.org> 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:
>
> suggested changes
src/jdk.jfr/share/classes/jdk/jfr/internal/JDKEvents.java line 2:
> 1: /*
> 2: * Copyright (c) 2016, 2023, Oracle and/or its affiliates. All rights
> reserved.
I assume you didn't mean to change that.
src/jdk.jfr/share/conf/jfr/profile.jfc line 741:
> 739: <setting name="stackTrace">true</setting>
> 740: <setting name="threshold" control="socket-threshold">10
> ms</setting>
> 741: </event>
In default.jfr the threshold for the socket events is 20ms, but 10ms in
profile.jfc. Is that intentional?
test/jdk/jdk/jfr/event/io/TestSocketChannelEvents.java line 106:
> 104:
> 105: try (SocketChannel sc =
> SocketChannel.open(ssc.getLocalAddress())) {
> 106:
> addExpectedEvent(IOEvent.createSocketConnectEvent(sc.socket()));
This is SocketChannel in blocking mode where the connect succeeds. There is
also the non-blocking and where connect fails. In addition the connection can
established with the socket adaptor. So 6 possible cases for SocketChannel if
the test is expanded.
test/jdk/jdk/jfr/event/io/TestSocketEvents.java line 108:
> 106: try (Socket s = new Socket()) {
> 107: s.connect(ss.getLocalSocketAddress());
> 108:
> addExpectedEvent(IOEvent.createSocketConnectEvent(s));
This is Socket.connect success case, I assume we'll need a test for fail case
too.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/21528#discussion_r1830512546
PR Review Comment: https://git.openjdk.org/jdk/pull/21528#discussion_r1830516492
PR Review Comment: https://git.openjdk.org/jdk/pull/21528#discussion_r1830545551
PR Review Comment: https://git.openjdk.org/jdk/pull/21528#discussion_r1830537634
