Implement getAttributes() method of MBean, and update Number-based MBean attributes to return Number objects instead of Strings

[cgossett]

User description

Thanks for contributing to Selenium!
A PR well described will help maintainers to quickly review and merge it

Before submitting your PR, please check our contributing guidelines.
Avoid large PRs, help reviewers by making them as simple and short as possible.

Description

This change implements the getAttributes() method on the JMX MBean object(s), returning an AttributeList with the current attributes of the MBean.

It also updates the return type of the MBean attributes which are Number based to their appropriate Number data type instead of String objects.

Motivation and Context

Motivation for this change is driven by trying to export the custom JMX metrics out of the Grid and into Prometheus, with the end goal being that those metrics once in Prometheus in an OpenShift environment can drive autoscaling of Node pods for a Grid instance. The JMX exporter in question is the Prometheus JMX Exporter, which leverages the getAttributes() method of any DynamicMBeans in order to determine if any attributes should be exported.

If implemented, this addition will allow those running Selenium Grid to more easily export the custom Grid metrics into a Prometheus environment.

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

Checklist

• I have read the contributing document.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have added tests to cover my changes.
• All new and existing tests passed.

---

PR Type

Enhancement, Tests

---

Description

• Implemented the getAttributes() method in the MBean class to return an AttributeList with current attributes.
• Updated the getAttribute method to correctly handle and parse Number types.
• Enhanced error handling in the getAttribute method to include ParseException.
• Added comprehensive unit tests for the getAttributes method.
• Updated existing tests to ensure MBean attributes return Number types instead of String.
• Introduced a helper method getAttributeList in tests to streamline attribute retrieval.

---

Changes walkthrough 📝

	Relevant files
Enhancement	MBean.java Implement `getAttributes` method and update `getAttribute` handling java/src/org/openqa/selenium/grid/jmx/MBean.java Implemented getAttributes() method to return an AttributeList with current MBean attributes. Updated getAttribute method to handle Number types and parse them correctly. Added error handling for ParseException. +22/-2
Tests	JmxTest.java Add unit tests for `getAttributes` and update attribute type checks java/test/org/openqa/selenium/grid/router/JmxTest.java Added unit tests for the getAttributes method. Updated existing tests to validate that MBean attributes return Number types instead of String. Introduced helper method getAttributeList to streamline attribute retrieval in tests. +76/-24

---

💡 PR-Agent usage:
Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

[CLAassistant]

All committers have signed the CLA.

[codiumai-pr-agent-pro]

PR Reviewer Guide 🔍

⏱️ Estimated effort to review [1-5]	3
🧪 Relevant tests	Yes
🔒 Security concerns	No
⚡ Key issues to review	Possible Bug: The use of NumberFormat.getInstance().parse(res.toString()) in getAttribute method might not be the most efficient or error-free way to convert numbers. Consider using more direct methods like Integer.valueOf() or Double.valueOf() depending on the expected type.
	Exception Handling: The broad catch of Exception in the getAttributes method might mask other unexpected runtime issues. It's generally better to catch more specific exceptions if possible.

[codiumai-pr-agent-pro]

PR Code Suggestions ✨

Category	Suggestion	Score
Best practice	Catch specific exceptions instead of a generic Exception in the getAttributes method Instead of catching a generic Exception in the getAttributes method, catch specific exceptions to avoid masking other potential issues. java/src/org/openqa/selenium/grid/jmx/MBean.java [259-261] -} catch (Exception e) { +} catch (AttributeNotFoundException | MBeanException | ReflectionException e) { LOG.severe("Error during execution: " + e.getMessage()); } Apply this suggestion Suggestion importance[1-10]: 8 Why: Catching specific exceptions instead of a generic Exception is a best practice as it prevents masking other potential issues and helps in handling exceptions more accurately.	8
Performance	Return Number directly instead of parsing its string representation back to a number Use Number directly instead of parsing the string representation back to a number in the getAttribute method. java/src/org/openqa/selenium/grid/jmx/MBean.java [228] -return NumberFormat.getInstance().parse(res.toString()); +return res; Apply this suggestion Suggestion importance[1-10]: 7 Why: Returning the Number object directly is more efficient and cleaner than parsing its string representation back to a number, thus improving performance.	7
Maintainability	Extract repeated parsing and assertion logic into a helper method Extract the repeated parsing and assertion logic into a helper method to reduce code duplication and improve readability. java/test/org/openqa/selenium/grid/router/JmxTest.java [148-152] -assertThat(Integer.parseInt(currentSessions.toString())).isZero(); -assertThat(Integer.parseInt(maxSessions.toString())).isEqualTo(1); +assertNumberAttribute(currentSessions, 0); +assertNumberAttribute(maxSessions, 1); +private void assertNumberAttribute(Object attribute, int expectedValue) { + assertThat(attribute).isNotNull(); + assertThat(Integer.parseInt(attribute.toString())).isEqualTo(expectedValue); +} + Apply this suggestion Suggestion importance[1-10]: 7 Why: Extracting repeated logic into a helper method reduces code duplication and improves readability, which enhances maintainability.	7
Possible bug	Add null checks before parsing attributes to avoid potential NullPointerException Add assertions to check for null values before parsing the attributes to avoid potential NullPointerException. java/test/org/openqa/selenium/grid/router/JmxTest.java [148] +assertThat(currentSessions).isNotNull(); assertThat(Integer.parseInt(currentSessions.toString())).isZero(); Apply this suggestion Suggestion importance[1-10]: 6 Why: Adding null checks before parsing attributes is a crucial safety measure to prevent NullPointerException, which can cause runtime errors.	6