Do not use static state variables in Extension class

[gbhat618]

Flakiness in the test was identified (possibly) due to the use of static stateful fields in the @Extension class. See this comment for details. Issue was likely parallel test execution mutating the state of static field.

This PR addresses the issue following the comment here.

Note: This problem only affects tests and does not impact Jenkins functionality. Whether the data is stored in a static cache or within a field inside a singleton instance, it effectively centralizes event data in one place.

Testing done

• Automated tests still executing correctly.
• Manual testing
  • verify the duplicate events monitor functionality is correct as before (tested by sending duplicate events - admin monitor comes, restart jenkins - monitor goes away)

Submitter checklist

• Make sure you are opening from a topic/feature/bugfix branch (right side) and not your main branch!
• Ensure that the pull request title represents the desired changelog entry
• Please describe what you did
• Link to relevant issues in GitHub or Jira
• Link to relevant pull requests, esp. upstream and downstream changes
• Ensure you have provided tests - that demonstrates feature works or fixes the issue

[gbhat618]

I'm using ExtensionList.lookupSingleton in two places in this class.

I considered keeping a class-level subscriber field, but that seem to require extra code to manage the reference correctly.

• Since both this monitor class and the subscriber have @Extension, I think they can't store direct references to each other—perhaps because their initialization order isn't guaranteed?
• We could store the reference during execution (if (subscriber != null) { subscriber = ExtensionList.lookupSingleton(...); }), but extensions are already looked up in more hot code path - ref code (executed for every event)

So I think this approach is fine - (ExtensionList.lookupSingleton probably doesn't introduce any overhead)

[jglick]

BTW I think this would be better written using a stream on entrySet.

[gbhat618]

entrySet because it is more performance optimal?

would be like,

return eventTracker.asMap().entrySet().stream()
                   .filter(entry -> entry.getValue() != null)
                   .map(Map.Entry::getKey)
                   .collect(Collectors.toSet());

But since this is a helper method for tests only, and we only need the keys, ig probably keeping on keySet() is easy read.. 🤔

(I now renamed the method to getPresentEventKeys which makes more sense)

[jglick]

Yeah it is just a bad pattern to iterate a map’s key set and then look up the value each time. Does not really matter of course.