Incorporate plugin health score in JSON metadata #840

alecharp · 2025-02-03T10:06:43Z

This adds the plugin health, from the https://github.com/jenkins-infra/plugin-health-scoring project, into the update-center output.

This is the first step to be able to display the scores in the Plugin Manager.

Testing

I tested on a limited number of plugins and it worked. I'm testing now with all plugins.

alecharp · 2025-02-03T10:07:45Z

src/main/java/io/jenkins/update_center/HealthScores.java

+import okhttp3.Request;
+
+public class HealthScores {
+    private static final String HEALTH_SCORES_URL = "https://reports.jenkins.io/plugin-health-scoring/scores.json";


Rather than using the application endpoint, which generate the output on demand, this stores a static file, generated every hour IIRC.

True per https://github.com/jenkins-infra/infra-reports/blob/main/plugin-health-scoring/Jenkinsfile#L1

dduportal

First set of feedback/point of attention:

What is the impact on the size of the JSON metadatas? I have vague recollections of impacts due to adding even an integer field causing challenges. Of course it might be hard to evaluate: if it is the case, I would propose to start with generating both JSONs files (with and without the PH score), we keep deploying the "without PH score" and measure the impact in size and build/deploy time before going forward.
This PR adds a dependency on the service reports.jenkins.io. If the service is down or not reachable, what would be the expected fall backs? This webservice is "HA" (e.g. it has a QoS bound to the underlying Azure Kubernetes cluster today) but could be down.

alecharp · 2025-02-03T13:43:01Z

What is the impact on the size of the JSON metadatas? I have vague recollections of impacts due to adding even an integer field causing challenges. Of course it might be hard to evaluate: if it is the case, I would propose to start with generating both JSONs files (with and without the PH score), we keep deploying the "without PH score" and measure the impact in size and build/deploy time before going forward.

I'll generate both and let you down the time required for the generation and the size the the files in each case.

This PR adds a dependency on the service reports.jenkins.io. If the service is down or not reachable, what would be the expected fall backs? This webservice is "HA" (e.g. it has a QoS bound to the underlying Azure Kubernetes cluster today) but could be down.

Technically, we already have this dependency:
https://github.com/jenkins-infra/update-center2/blob/master/src/main/java/io/jenkins/update_center/MaintainersSource.java#L47-L48. However, I'll try to see what happens and improve the code if needed for this situation.

dduportal · 2025-02-03T13:55:11Z

I'll generate both and let you down the time required for the generation and the size the the files in each case.

I wasn't sure it was possible or easy for you (as the production dataset could be heavy, not even speaking about the cache...). Thanks!

Technically, we already have this dependency: https://github.com/jenkins-infra/update-center2/blob/master/src/main/java/io/jenkins/update_center/MaintainersSource.java#L47-L48. However, I'll try to see what happens and improve the code if needed for this situation.

OH: I was not aware. Then your change here does NOT add any dependency but reuse existing. It makes sense then (as per my infra hat: I can answer for the scope of the code itself).

alecharp · 2025-02-03T14:31:47Z

So for the size:

❯ ls -al with*
with-health-score:
total 18528
drwxr-xr-x   5 alecharpe  *      160  3 fév 15:25 .
drwxr-xr-x  20 alecharpe  *      640  3 fév 15:28 ..
-rw-r--r--   1 alecharpe  *  3158562  3 fév 15:25 update-center.actual.json
-rw-r--r--   1 alecharpe  *  3158584  3 fév 15:25 update-center.json
-rw-r--r--   1 alecharpe  *  3158787  3 fév 15:25 update-center.json.html

without-health-score:
total 18312
drwxr-xr-x   5 alecharpe  *      160  3 fév 15:28 .
drwxr-xr-x  20 alecharpe  *      640  3 fév 15:28 ..
-rw-r--r--   1 alecharpe  *  3124324  3 fév 15:28 update-center.actual.json
-rw-r--r--   1 alecharpe  *  3124346  3 fév 15:28 update-center.json
-rw-r--r--   1 alecharpe  *  3124549  3 fév 15:28 update-center.json.html

For the timing, I ran the commands with an existing cache as downloading the files should not be part of the problem. As the health report is accessed / loaded once, this changeset should not be the biggest source of issues. Compared to accessing all the plugin GitHub repositories for example.

without the health score: java -Dfile.encoding=UTF-8 -jar --id default --www-dir without-health-score 14,61s user 1,77s system 14% cpu 1:55,94 total
with the health score: java -Dfile.encoding=UTF-8 -jar --id default --www-dir with-health-score 13,82s user 1,71s system 12% cpu 2:02,35 total

I'm still looking at what happens with no access to reports service.

alecharp · 2025-02-03T14:43:57Z

So apparently, with reports.jenkins.io is down (blocked it locally), the master branch is not handling that well: it breaks the generation entirely.
I don't think there is a point of testing this scenario for this changeset. But that's something we can track to improve in the future.

daniel-beck · 2025-02-03T16:03:48Z

So apparently, with reports.jenkins.io is down (blocked it locally), the master branch is not handling that well: it breaks the generation entirely. I don't think there is a point of testing this scenario for this changeset. But that's something we can track to improve in the future.

MaintainersSource and IssueTrackerSource (the two existing references to reports.jenkins.io) are written such that download failures generate warnings in the log. That it doesn't is a simple bug fixed in #841. Please use these classes as a blueprint how to download the data (and I should probably fix up Popularities in a similar way).

alecharp · 2025-02-03T16:47:04Z

Thanks @daniel-beck. Just improved the code to not fail if the report is not available.
I would like to take some time to see if we could remove the health score integer completely from the generated file if we didn't get a score. Cause, if we provide a 0 because reports service is down, that seems bad.

alecharp · 2025-02-03T17:18:03Z

src/main/java/io/jenkins/update_center/HealthScores.java

+        public int value;
+    }
+
+    public Integer getHealthScore(String pluginId) {


I would prefer not to have the boxing / unboxing but the current fastjson configuration do not serialise null field. Using an ObjectSerializer with another default value (like -1) is not working as only the value can be ignored from the serialisation, but the field name is generated anyway.

daniel-beck

Looks reasonable 😃

src/main/java/io/jenkins/update_center/HealthScores.java

daniel-beck · 2025-02-03T18:19:23Z

src/main/java/io/jenkins/update_center/HealthScores.java

+import okhttp3.OkHttpClient;
+import okhttp3.Request;
+
+public class HealthScores {


Javadoc for this would be useful.

Just added what made sense to me. Please let me know if it's alright or if I need to improve it.

daniel-beck · 2025-02-04T08:37:05Z

On-holding this, pending the corresponding core change.

@daniel-beck

Thanks @daniel-beck Co-authored-by: Daniel Beck <[email protected]>

feat: Incorporates plugin health score in update-center data

d834e4c

alecharp commented Feb 3, 2025

View reviewed changes

dduportal reviewed Feb 3, 2025

View reviewed changes

alecharp marked this pull request as ready for review February 3, 2025 15:45

daniel-beck mentioned this pull request Feb 3, 2025

Make IssueTrackersSource more resilient #841

Open

daniel-beck changed the title ~~Incorporates plugin health score in update-center data~~ Incorporate plugin health score in JSON metadata Feb 3, 2025

daniel-beck added enhancement This is an enhancement for the tool or wrapper scripts, typically adding features. java This PR affects Java code and would need a new release to be effective labels Feb 3, 2025

daniel-beck self-requested a review February 3, 2025 16:04

feat: Survives not getting the health score report

a5054d4

alecharp marked this pull request as draft February 3, 2025 16:47

feat: Prevents from showing invalid scores

24bba20

alecharp commented Feb 3, 2025

View reviewed changes

alecharp marked this pull request as ready for review February 3, 2025 17:27

daniel-beck reviewed Feb 3, 2025

View reviewed changes

daniel-beck added the on-hold This PR is on hold, typically because of a dependency to another change or event. label Feb 4, 2025

alecharp and others added 2 commits February 4, 2025 10:59

Apply suggestions from code review

3ce23dd

Thanks @daniel-beck Co-authored-by: Daniel Beck <[email protected]>

feat: Documents the purpose of the new HealthScores class

b73c00a

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Incorporate plugin health score in JSON metadata #840

Incorporate plugin health score in JSON metadata #840

alecharp commented Feb 3, 2025

alecharp Feb 3, 2025

daniel-beck Feb 3, 2025

dduportal left a comment

alecharp commented Feb 3, 2025

dduportal commented Feb 3, 2025

alecharp commented Feb 3, 2025

alecharp commented Feb 3, 2025

daniel-beck commented Feb 3, 2025 •

edited

Loading

alecharp commented Feb 3, 2025

alecharp Feb 3, 2025

daniel-beck left a comment

daniel-beck Feb 3, 2025

alecharp Feb 4, 2025

daniel-beck commented Feb 4, 2025

Incorporate plugin health score in JSON metadata #840

Are you sure you want to change the base?

Incorporate plugin health score in JSON metadata #840

Conversation

alecharp commented Feb 3, 2025

Testing

alecharp Feb 3, 2025

Choose a reason for hiding this comment

daniel-beck Feb 3, 2025

Choose a reason for hiding this comment

dduportal left a comment

Choose a reason for hiding this comment

alecharp commented Feb 3, 2025

dduportal commented Feb 3, 2025

alecharp commented Feb 3, 2025

alecharp commented Feb 3, 2025

daniel-beck commented Feb 3, 2025 • edited Loading

alecharp commented Feb 3, 2025

alecharp Feb 3, 2025

Choose a reason for hiding this comment

daniel-beck left a comment

Choose a reason for hiding this comment

daniel-beck Feb 3, 2025

Choose a reason for hiding this comment

alecharp Feb 4, 2025

Choose a reason for hiding this comment

daniel-beck commented Feb 4, 2025

daniel-beck commented Feb 3, 2025 •

edited

Loading