feat: improve Quarkus component detection

[stuartwdouglas]

Quarkus projects often have lots of modules that are not intended to be components, but still have io.quarkus dependencies.

This commit will only mark a module as a Quarkus component if it explicitly references the Quarkus plugin that creates a runnable application.

In addition if a Java project contains components that explicitly specify frameworks then modules without frameworks will be removed.

This will help with the usability issues identified in RHTAPBUGS-552.

Testing against Apicurio Registry this reduces the component count down from 50 to 8, which is expected.

[codecov]

Codecov Report

Attention: Patch coverage is 76.47059% with 4 lines in your changes are missing coverage. Please review.

Project coverage is 67.96%. Comparing base (060a15e) to head (ce15ea7).
Report is 26 commits behind head on main.

Files	Patch %	Lines
pkg/apis/recognizer/component_recognizer.go	76.47%	3 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main      #21      +/-   ##
==========================================
+ Coverage   67.89%   67.96%   +0.07%     
==========================================
  Files          11       11              
  Lines        1523     1539      +16     
==========================================
+ Hits         1034     1046      +12     
- Misses        423      426       +3     
- Partials       66       67       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[stuartwdouglas]

Is there anything else I need to do here?

[thepetk]

Shall we add this as part of the pkg/utils/langfiles/resources/languages-customization.yaml like:

Java:
  configuration_files:
    - "pom.xml"
    - "build.gradle"
  component: true
  framework_preferred: true

[thepetk]

I'm temporarily request changes, as it is still under consideration regarding the changes we make in the component detection behavior for java components.

More detailed, I want to confirm team-wise that we would like to introduce this new behavior inside the component_recognizer.go

[thepetk]

I'm temporarily request changes, as it is still under consideration regarding the changes we make in the component detection behavior for java components.

More detailed, I want to confirm team-wise that we would like to introduce this new behavior inside the component_recognizer.go

Resuming discussion for the PR

• I think is a very nice addition to have a more tight detection logic for quarkus:

//note that we don't specify the group id because this can sometimes be a property substitution
if hasFwk, _ := hasFramework(config, "", "quarkus-maven-plugin"); hasFwk {
  language.Frameworks = append(language.Frameworks, "Quarkus")
} else if hasFwk, _ = hasFramework(config, "", "io.quarkus.gradle.plugin"); hasFwk {
  language.Frameworks = append(language.Frameworks, "Quarkus")
}

• I see the frameworkPreffered as a cool feature in the future for Alizer. As right now we don't support all frameworks (or at least the most important) I think it would have big impact if we change so drastically the behavior of the detectors. Additionaly, the framework field is "treated" as additional information for a component atm. So after a language analysis we are trying to enrich every component found. Now, with the current workaround, we are making the framework deciding for the components.

That said, IMO I would suggest that we could keep the fix for quarkus hasFramework func. But I'd suggest to move all the updates for the component filtering to the consumer side and filter alizer's response there. As a result alizer will still be able to provide detailed information about the source code, avoiding the risk of skipping important components with not supported frameworks. WDYT?

[stuartwdouglas]

Sorry, I completely missed the review on this.

The reason why I did the filtering where it is was because I wanted to consider the whole project when making this change (i.e. the hasFrameworks field). If no frameworks are detected it keeps the existing behavior.

[openshift-ci]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: mike-hoang, stuartwdouglas
Once this PR has been reviewed and has the lgtm label, please ask for approval from thepetk. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[thepetk]

Sorry, I completely missed the review on this.

The reason why I did the filtering where it is was because I wanted to consider the whole project when making this change (i.e. the hasFrameworks field). If no frameworks are detected it keeps the existing behavior.

@stuartwdouglas no worries!

I have went again through the proposed workaround. As mentioned in a previous comment, atm we are not covering all the frameworks so we may have cases that we are excluding a legit component. I tried to run an example for a hypothetical monolithic repo which has different components inside.

Example dir

jsf-component/
open-liberty-component/
quarkus-component/
reactjs-component/

From the four above components the 3 of them will be enriched with a framework (e.g. openliberty, quarkus and react). There is one which will not be enriched with a framework, the JSF (randomly picked as example).

With quarkus improvements we will be able to detect only 3 components, while with the existing functionality we are able to detect all 4 components stored in root dir.

[stuartwdouglas]

I guess to make this decision properly you would need to track dependency information between the modules. If a module with no framework is a dependency of another module with a framework, then this module is likely not a component.

If a module is standalone then it is likely a component even without a detected framework.

That said I am not really sure how to make such a change in the current code base, I think it would be a fairly significant change.

[thepetk]

I guess to make this decision properly you would need to track dependency information between the modules. If a module with no framework is a dependency of another module with a framework, then this module is likely not a component.

If a module is standalone then it is likely a component even without a detected framework.

That said I am not really sure how to make such a change in the current code base, I think it would be a fairly significant change.

@stuartwdouglas mostly I agree with what you are saying. The only thing I'd like to note, is that IMO a module that has no framework and is a dependency of another module could be a component. It could be detected, but we could also add a field in alizer's response mentioning any related/dependent components for each component. I see this as a cool feature for alizer. We could also add a flag like "parent-components-only" (name not important) so a user can choose if they want to see only parent components (makes this workaround backwards compatible too). WDYT?

[openshift-merge-robot]

PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[github-actions]

This PR is stale because it has been open for 90 days with no activity. Remove stale label or comment or this will be closed in 60 days.

[github-actions]

This PR is stale because it has been open for 90 days with no activity. Remove stale label or comment or this will be closed in 60 days.