RFR: 7231: JMC createReport for JMC8 Automated Analysis fails to evaluate several rules [v4]

[Henrik Dafgård]

On Mon, 4 Oct 2021 15:14:31 GMT, Alex Macdonald <aptmac at openjdk.org> wrote:

>> This PR addresses JMC-7231 [[0]](https://bugs.openjdk.java.net/browse/JMC-7231), in which using `JfrHtmlRulesReport` throws a number of exceptions and fails to evaluate a handful of rules.
>> 
>> I used some of the review comments [[1]](https://github.com/openjdk/jmc/pull/302#pullrequestreview-726780649) that I made in the other open PR to address the same issue.
>> 
>> As far as I can tell, there are two main issues here.
>> 
>> The first is some rules now (after the Rules 2.0 update) have dependencies, these are (4) `GcLockerRule`, `GcStallRule`, `HeapInspectionRule`, and `SystemGcRule` which depend on `GarbageCollectionInfoRule`. The problem here is that if `GarbageCollectionInfoRule` isn't evaluated then the other four rules end up throwing an NPE while trying to evaluate.
>> 
>> The second issue is that as of Rules 2.0, on the jmc.ui Automated Analysis page we use `RulesToolkit.matchesEventAvailabilityMap()` to verify is an event is is available and if the rule should be evaluated. This check wasn't replicated on the `core` implementation, so I've added it in. What's interesting here is that in the test jfr I was using, `GarbageCollectionInfoRule` was filtered out by `matchesEventAvailabilityMap()`, which was causing the above 4 Gc rules to explode.
>> 
>> The solution I've put together uses two lists to track 1). rules that are unavailable due to being filtered out by `matchesEventAvailabilityMap()`, and 2). rules that have dependencies. It later loops through the list of rules with dependencies, and checks to see if they exist in the list with unavailable rules. If a rule is not able to be evaluated by either 1). not being available or 2). by having a dependency that is not available, it returns a completed future `IResult` containing "not available", similar to how on this is handled on the jmc.ui side. Then the rest of the rules report proceeds as usual.
>> 
>> I think the easiest way to verify/try this fix is by creating a run configuration for JfrHtmlRulesReport in Eclipse, and feeding it a jfr file.
>> 
>> [0] https://bugs.openjdk.java.net/browse/JMC-7231
>> [1] https://github.com/openjdk/jmc/pull/302#pullrequestreview-726780649
>
> Alex Macdonald has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains five additional commits since the last revision:
> 
>  - update copyright year and javadoc for RulesToolkit.evaluateErrorResult()
>  - run mvn spotless:apply
>  - clean up if statement
>  - ensure rule dependency is evaluated before evaluating rules that depend on it
>  - 7231: JMC createReport for JMC8 Automated Analysis fails to evaluate several rules

Changes requested by hdafgard (Reviewer).

core/org.openjdk.jmc.flightrecorder.rules/src/main/java/org/openjdk/jmc/flightrecorder/rules/util/RulesToolkit.java line 1244:

> 1242: 					break;
> 1243: 				}
> 1244: 			}

This is not enough to determine whether or not a rule should be evaluated. The `@dependsOn` annotation specifies both a class and an optional minimum severity that the prior rule must return for the current rule to evaluate. If we disregard this we risk running into NPEs for later rules as they attempt to access results that might not be present in the RulesProvider.

core/org.openjdk.jmc.flightrecorder.rules/src/main/java/org/openjdk/jmc/flightrecorder/rules/util/RulesToolkit.java line 1287:

> 1285: 	 * @return a completed future containing an evaluation error result
> 1286: 	 */
> 1287: 	private static Future<IResult> evaluationErrorResult(IRule rule, IPreferenceValueProvider preferences) {

I think it might be slightly confusing that this is labeled with "error" if the rule evaluation did not fail, just did not take place. It could probably just be a N/A result instead, which we should already have methods for in RulesToolkit.

-------------

PR: https://git.openjdk.java.net/jmc/pull/311