Skip to content

Add MicroMeter Prometheus tests #345

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Draft
wants to merge 9 commits into
base: master
Choose a base branch
from
Draft

Add MicroMeter Prometheus tests #345

wants to merge 9 commits into from

Conversation

marekkopecky
Copy link
Contributor

@marekkopecky marekkopecky commented Sep 26, 2025 •
edited
Loading

Server

This doesn't work on upstream non-community server.

REST assured library
REST assured authentication doesn't work

given()
  .auth().preemptive().basic(MgmtUsersSetup.USER_NAME, MgmtUsersSetup.PASSWORD)
.when()
  .get(url)
.then()...

and I was not able to figure out how to hardcode Apache Client to it, that's the reason why I'm using RESTEasy. Upstream is using Apache directly: https://github.com/wildfly/wildfly/pull/17857/files#diff-074a5e7f4019eac45c8a7be241dee05f64368102fb7a6e3c93cdd92eec9624ffR124

Default requirements:

  • Pull Request contains a description of the changes
  • Pull Request does not include fixes for multiple issues/topics
  • Code is formatted, imports ordered, code compiles and tests are passing
  • Link to the passing job is provided
  • Code is self-descriptive and/or documented
  • Description of the tests scenarios is included (see Update PR template to include TPG stuff #46)

@marekkopecky marekkopecky marked this pull request as draft September 26, 2025 12:16
@marekkopecky marekkopecky requested a review from fabiobrz October 3, 2025 08:50
@marekkopecky marekkopecky force-pushed the micrometer-prometheus branch from bdd6fdb to be6d9eb Compare October 3, 2025 12:05
fabiobrz
fabiobrz requested changes Oct 3, 2025
View reviewed changes
Copy link
Member

@fabiobrz fabiobrz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @marekkopecky - the changes mostly LGTM, but I dropped some minor comments for your evaluation.

BTW - the PR is still a draft. Do you plan to push more changes?

...meter/src/test/java/org/jboss/eap/qe/micrometer/prometheus/MicrometerPrometheusTestCase.java Outdated
public void securityTest() throws Exception {
try {
// use authnetication, although prometheus is not secured yet
MgmtUsersSetup.setup();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIUC, logically the above statement should be outside the try block, since you're reverting it in the finally block, correct?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

securityTest test method updated

...meter/src/test/java/org/jboss/eap/qe/micrometer/prometheus/MicrometerPrometheusTestCase.java Outdated
client.execute("/subsystem=metrics:remove");
client.execute("/extension=org.wildfly.extension.metrics:remove");
MicrometerPrometheusSetup.set(client, "/metrics", false);
String response = fetchPrometheusMetricsRequireStatusCode(false, 200);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIUC, logically all the above statements should be placed before the try statement, since you're reverting them in the finally block, correct?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I updated wfMetricsDisabledTest test method

...meter/src/test/java/org/jboss/eap/qe/micrometer/prometheus/MicrometerPrometheusTestCase.java Outdated
public void wfMetricsEnabledTest() throws Exception {
try {
MicrometerPrometheusSetup.set(client, "/metrics", false);
Assert.assertTrue(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIUC, logically the above statement should be placed before the try statement, since you're reverting it in the finally block, correct?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This appears as outdated in the GitHub UI, but it is actually not changes, right @marekkopecky ? See https://github.com/jboss-eap-qe/eap-microprofile-test-suite/pull/345/files#diff-5396f39821558725509335b7744ff52141796bbbbc8c6f4f3764f5e258d71a7bR108

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1, I updated it now

@marekkopecky marekkopecky force-pushed the micrometer-prometheus branch from 9d7c1dd to d128eef Compare October 8, 2025 08:57
@marekkopecky marekkopecky force-pushed the micrometer-prometheus branch from 20e59f7 to 411e9ca Compare October 8, 2025 11:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@fabiobrz fabiobrz fabiobrz requested changes

Requested changes must be addressed to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@marekkopecky @fabiobrz