RFR: 7198: add websocket server that pushes data on selection updates [v6]
Jean-Philippe Bempel
jpbempel at openjdk.java.net
Mon Sep 20 15:53:56 UTC 2021
On Thu, 16 Sep 2021 21:10:12 GMT, Alex Ciminian <github.com+348973+cimi at openjdk.org> wrote:
>> This PR adds a websocket server that pushes event data as JSON to all connected clients whenever the user updates their current selection in JMC. The server is disabled by default and can be enabled and disabled from JMC preferences. This is a rework of the proof-of-concept code in https://github.com/openjdk/jmc/pull/225 and uses the JSON serialiser introduced in #279.
>>
>> 
>>
>> ### Dependency management
>>
>>
>> Brice pointed out that there are newer versions of the Jetty websocket server available under a different artefact name - I used the same version as required in the UI tests (`10.0.5`) and this made the build succeed in both eclipse and CLI. I've had to add SPI fly since it was required by jetty, but I don't really understand what it does.
>>
>> Initially I struggled to add the required dependencies to the project. Initially I was only testing in eclipse with the setup from #225 and it worked fine.`mvn package` also worked on the command line. However, the spotless task from our CI script failed with the following error:
>>
>>
>> org.apache.felix.resolver.reason.ReasonException: Uses constraint violation. Unable to resolve resource org.openjdk.jmc.flightrecorder.ui [osgi.identity; osgi.identity="org.openjdk.jmc.flightrecorder.ui"; type="osgi.bundle"; version:Version="8.2.0.qualifier"; singleton:="true"] because it is exposed to package 'javax.servlet' from resources javax.servlet [osgi.identity; osgi.identity="javax.servlet"; type="osgi.bundle"; version:Version="3.1.0.v201410161800"] and jakarta.servlet-api [osgi.identity; osgi.identity="jakarta.servlet-api"; type="osgi.bundle"; version:Version="4.0.0"] via two dependency chains.
>> [ERROR]
>> [ERROR] Chain 1:
>> [ERROR] org.openjdk.jmc.flightrecorder.ui [osgi.identity; osgi.identity="org.openjdk.jmc.flightrecorder.ui"; type="osgi.bundle"; version:Version="8.2.0.qualifier"; singleton:="true"]
>> [ERROR] require: (osgi.wiring.bundle=javax.servlet)
>> [ERROR] |
>> [ERROR] provide: osgi.wiring.bundle: javax.servlet
>> [ERROR] javax.servlet [osgi.identity; osgi.identity="javax.servlet"; type="osgi.bundle"; version:Version="3.1.0.v201410161800"]
>> [ERROR]
>> [ERROR] Chain 2:
>> [ERROR] org.openjdk.jmc.flightrecorder.ui [osgi.identity; osgi.identity="org.openjdk.jmc.flightrecorder.ui"; type="osgi.bundle"; version:Version="8.2.0.qualifier"; singleton:="true"]
>> [ERROR] require: (osgi.wiring.bundle=org.eclipse.jetty.server)
>> [ERROR] |
>> [ERROR] provide: osgi.wiring.bundle; bundle-version:Version="10.0.5"; osgi.wiring.bundle="org.eclipse.jetty.server"
>> [ERROR] org.eclipse.jetty.server [osgi.identity; osgi.identity="org.eclipse.jetty.server"; type="osgi.bundle"; version:Version="10.0.5"]
>> [ERROR] import: (&(osgi.wiring.package=javax.servlet)(&(version>=4.0.0)(!(version>=5.0.0))))
>> [ERROR] |
>> [ERROR] export: osgi.wiring.package: javax.servlet
>> [ERROR] jakarta.servlet-api [osgi.identity; osgi.identity="jakarta.servlet-api"; type="osgi.bundle"; version:Version="4.0.0"]
>>
>>
>> This is because the task is run with the `-P uitests` and something in the UI test configuration pulls in jetty.server 10.0.5. Since [the latest available version](https://mvnrepository.com/artifact/org.eclipse.jetty.websocket/websocket-server) for the jetty websocket server is `9.4.43.v20210629` we can't use the newer `10.x` for the transitive dependencies.
>>
>> I was able to work around this problem by setting bundle-version constraints in the `flightrecorder.ui` manifest. This makes all CI checks pass, but unfortunately it also breaks the JFR editor view in eclipse ��
>>
>>
>> org.osgi.framework.BundleException: Could not resolve module: org.openjdk.jmc.joverflow.ui [457]
>> Unresolved requirement: Import-Package: org.openjdk.jmc.flightrecorder.ui
>> -> Export-Package: org.openjdk.jmc.flightrecorder.ui; bundle-symbolic-name="org.openjdk.jmc.flightrecorder.ui"; bundle-version="8.2.0.qualifier"; version="0.0.0"
>> org.openjdk.jmc.flightrecorder.ui [445]
>> Unresolved requirement: Require-Bundle: org.eclipse.jetty.server; bundle-version="[9.4.43,10.0.0)"
>
> Alex Ciminian has updated the pull request incrementally with two additional commits since the last revision:
>
> - Add tree and graph handlers to websocket server
> - Revert whitespace changes in core/features.xml
application/org.openjdk.jmc.feature.flightrecorder/feature.xml line 3:
> 1: <?xml version="1.0" encoding="UTF-8"?>
> 2: <!--
> 3: Copyright (c) 2018, 2020, 2021 Oracle and/or its affiliates. All rights reserved.
only 2 years as a range: `2018, 2021, Oracle`
application/org.openjdk.jmc.flightrecorder.ui/src/main/java/org/openjdk/jmc/flightrecorder/ui/FlightRecorderUI.java line 3:
> 1: /*
> 2: * Copyright (c) 2018, 2021, Oracle and/or its affiliates. All rights reserved.
> 3: * Copyright (c) 2021, Datadog, Inc. All rights reserved.
should not add Datadog copyright for existing files, only for new ones.
application/org.openjdk.jmc.flightrecorder.ui/src/main/java/org/openjdk/jmc/flightrecorder/ui/JfrEditor.java line 130:
> 128: };
> 129: if (FlightRecorderUI.getDefault().isWebsocketServerEnabled()) {
> 130: long websocketServerPort = FlightRecorderUI.getDefault().getWebsocketPort();
why `long` for the variable type? it forces you to cast to int when calling `WebsocketServer` constructor below while `getWebsocketPort()` returns int anyway
application/org.openjdk.jmc.flightrecorder.ui/src/main/java/org/openjdk/jmc/flightrecorder/ui/messages/internal/Messages.java line 3:
> 1: /*
> 2: * Copyright (c) 2018, 2021 Oracle and/or its affiliates. All rights reserved.
> 3: * Copyright (c) 2021, Datadog, Inc. All rights reserved.
no datadog copyright
application/org.openjdk.jmc.flightrecorder.ui/src/main/java/org/openjdk/jmc/flightrecorder/ui/preferences/GeneralPage.java line 3:
> 1: /*
> 2: * Copyright (c) 2018, 2021, Oracle and/or its affiliates. All rights reserved.
> 3: * Copyright (c) 2021, Datadog, Inc. All rights reserved.
no datadog copyright
application/org.openjdk.jmc.flightrecorder.ui/src/main/java/org/openjdk/jmc/flightrecorder/ui/preferences/PreferenceKeys.java line 3:
> 1: /*
> 2: * Copyright (c) 2018, 2021, Oracle and/or its affiliates. All rights reserved.
> 3: * Copyright (c) 2021, Datadog, Inc. All rights reserved.
no datadog copyright
application/org.openjdk.jmc.flightrecorder.ui/src/main/java/org/openjdk/jmc/flightrecorder/ui/websocket/WebsocketServer.java line 41:
> 39: private List<WebsocketConnectionHandler> handlers = new ArrayList<>();
> 40: private List<WebsocketConnectionHandler> treeHandlers = new ArrayList<>();
> 41: private List<WebsocketConnectionHandler> graphHandlers = new ArrayList<>();
Since there is more than one thread involved into accessing those handlers I would use CopyOnWriteArrayList to be safe on the access.
application/org.openjdk.jmc.flightrecorder.ui/src/main/resources/org/openjdk/jmc/flightrecorder/ui/messages/internal/messages.properties line 3:
> 1: #
> 2: # Copyright (c) 2018, 2021 Oracle and/or its affiliates. All rights reserved.
> 3: # Copyright (c) 2021, Datadog, Inc. All rights reserved.
no datadog copyright
releng/platform-definitions/platform-definition-2019-12/platform-definition-2019-12.target line 4:
> 2: <!--
> 3: Copyright (c) 2019, 2021 Oracle and/or its affiliates. All rights reserved.
> 4: Copyright (c) 2019, 2021 Datadog, Inc. All rights reserved.
should be only 2021 if it's a new file
releng/third-party/pom.xml line 3:
> 1: <?xml version="1.0" encoding="UTF-8"?>
> 2: <!--
> 3: Copyright (c) 2018, 2019, 2021 Oracle and/or its affiliates. All rights reserved.
years as a range
releng/third-party/pom.xml line 4:
> 2: <!--
> 3: Copyright (c) 2018, 2019, 2021 Oracle and/or its affiliates. All rights reserved.
> 4: Copyright (c) 2021, Datadog, Inc. All rights reserved.
no datadog copyright
-------------
PR: https://git.openjdk.java.net/jmc/pull/306
More information about the jmc-dev
mailing list