ci.maven icon indicating copy to clipboard operation
ci.maven copied to clipboard

Provide new plugin config to customize applicationMonitor configuration

Open scottkurz opened this issue 2 years ago • 5 comments

BACKGROUND

(Opening this issue based on what started as an internal, IBM-only discussion here: https://github.ibm.com/liberty-dev-ex/design-issues/issues/16)

As described in this Liberty Tools Eclipse issue, a large app might take so long to restart that a user may want to disable the default "polled" application monitor behavior, and rely on debugger HCR to quickly test the results of the change.

PROPOSAL

A proposed design would be to have a new top-level config (e.g Maven parm/property), say 'appMonitorTrigger', and do do the following:

  • If set to one of the values, 'polled', 'mbean', 'disabled', LMP/LGP generates configDropin/overrides/liberty-plugin-app-monitor-config.xml in with <applicationMonitor updateTrigger=""/> (i.e. where the set value is 'polled', 'mbean', 'disabled').
  • If not set, LMP/LGP deletes configDropin/overrides/liberty-plugin-app-monitor-config.xml

The generation/deletion would happen at basically the same time (same goals/tasks) that the liberty-plugin-variable-config.xml file is generated today.

The Liberty Tools IDE features can then set this parm accordingly as they launch dev mode, depending on whether a Run or Debug is performed in Liberty Tools.

EXAMPLE

mvn liberty:dev -DappMonitorTrigger=disabled

will generate config:

 <applicationMonitor updateTrigger="disabled"/>

TODO

I believe this will merge with existing config but should double-check

Minor design questions:

  • Do we even need 'polled' as an option, since it's already the default? Is there really a reason to override any other config?
  • Do we want an explicit "not set" option in addition to defaulting? Seems like this could be helpful

scottkurz avatar Aug 03 '23 18:08 scottkurz

Note that we describe this as requiring a new plugin config, but the intended user of this support, Liberty Tools, would probably only ever do so through a property. So it's possibly worth raising the question: would we provide a behavior that was ONLY configurable via property, and NOT via plugin parameter?

scottkurz avatar Aug 03 '23 18:08 scottkurz

I published a snapshot to Sonatype to test the described behavior using 3.8.3-appMon-SNAPSHOT. Currently can specify with: mvn liberty:dev -DapplicationMonitor=mbean. (I noticed your suggestion for appMonitorTrigger after publishing, but can change to this if preferred) Has validation to ensure the value is one of: [polled, mbean, disabled]. If not specified, the existing file in configDropins/overrides is deleted.

Relevant PRs:

  • ci.maven #1711
  • ci.common OpenLiberty/ci.common#408

Snapshot Installation:

<plugin>
    <groupId>io.openliberty.tools</groupId>
    <artifactId>liberty-maven-plugin</artifactId>
    <version>3.8.3-appMon-SNAPSHOT</version>
</plugin>

And make sure to add this under the pom.xml <project>

<pluginRepositories>
    <!-- Configure Sonatype OSS Maven snapshots repository -->
    <pluginRepository>
        <id>sonatype-nexus-snapshots</id>
        <name>Sonatype Nexus Snapshots</name>
        <url>https://oss.sonatype.org/content/repositories/snapshots/</url>
        <snapshots>
            <enabled>true</enabled>
        </snapshots>
        <releases>
            <enabled>false</enabled>
        </releases>
    </pluginRepository>
</pluginRepositories>

evie-lau avatar Aug 07 '23 20:08 evie-lau

In reviewing this with the broader runtime team, the question was raised: how does WDT/LDT handle an update to a web.xml in debug mode?

The reason for focusing on this case is that, as a non-Java change, it would fall outside the scope of the debugger's HCR capabilities to update or replace.

As it turns out, doing a test and looking at the source code: it appears that WDT/LDT will actually, in general, still call the mbean to update an app after a change, even with running in debug mode.

The handling of Java class file updates is actually a special case in the overall logic, as one can see here: https://github.com/OpenLiberty/open-liberty-tools/blob/e86be3dbc36b3dfa531804f480babdfdafbd9638/dev/com.ibm.ws.st.jee.core/src/com/ibm/ws/st/jee/core/internal/JEEServerExtension.java#L525-L528 where the publish is NOT performed if the project change is a .class file AND debug mode is enabled.

scottkurz avatar Sep 25 '23 21:09 scottkurz

Given the last comment, the new, proposed config becomes less immediately useful to Liberty Tools (IDE) function.

If we assume that the WDT/LDT-like mbean invocation would be added to Liberty Tools IDEs in the near-term, then this proposal would indeed seem useful. That mbean invocation function could include logic to update or not based on the type of resource (as in LDT/WDT).

However, without this mbean invocation function imminent, is it still worth going ahead with this proposal?

If we were to go ahead, we would probably make the debug behavior "opt-in", i.e. not the default, making it less useful. Would it be reasonable to doc for a user creating their own updateTrigger config variable and setting it (or not) in their Liberty Tools Run/Debug configs (i.e. using a custom config)?

I don't think there is a way to do this that doesn't involve some amt. of a learning curve for someone used to the WDT/LDT approach.

BTW, another thing WDT/LDT does is to implement org.eclipse.jdt.debug.core.IJavaHotCodeReplaceListener. I don't follow the logic completely, see here, but it seems to include being smart enough to offer a prompt to restart the server when the situation warrants.

scottkurz avatar Sep 25 '23 21:09 scottkurz

**2023-09-27 - DXDI update: **

Decided to wait on merging this function until we have a more complete view of how we'd leverage this in Liberty Tools IDE, which would be a significant enhancement in each IDE, and which still requires some design work.

  • As discussed before, it'd be nice to have a doc showing how to workaround this problem in each IDE.
  • Suggested that when it comes time to prototyping app update detection and mbean invocation, it should be considered at the LMP/LGP layers (rather than assuming each LT IDE adds its own function).
    • Is there a way to move update detection (i.e. is it a Java change vs. web.xml vs. annotation change) into LMP/LGP?
  • Recognized that even the LDT/WDT app update detection might have some gaps, e.g. if a urlPatterns attr changes on a @WebServlet this would require an app update. Perhaps we'd even need to go further than the LDT/WDT function does today?

scottkurz avatar Sep 27 '23 13:09 scottkurz