Skip to content
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.

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

Fix daily runs: Adapt to the new format of Prometheus metrics #990

Merged
merged 1 commit into from
Jan 13, 2023

Conversation

fedinskiy
Copy link
Contributor

See quarkusio/quarkus#30343 for details

Summary

Please select the relevant options.

  • Bug fix (non-breaking change which fixes an issue)
  • Dependency update
  • Refactoring
  • Backport
  • New scenario (non-breaking change which adds functionality)
  • This change requires a documentation update
  • This change requires execution against OCP (use run tests phrase in comment)

Checklist:

  • Methods and classes used in PR scenarios are meaningful
  • Commits are well encapsulated and follow the best practices

@fedinskiy fedinskiy force-pushed the fix/prometheus-format branch from c84d433 to 2bf6b28 Compare January 13, 2023 08:26
@pjgg pjgg self-requested a review January 13, 2023 08:30
Copy link
Contributor

@pjgg pjgg left a comment

Choose a reason for hiding this comment

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

LGTM

@rsvoboda rsvoboda requested a review from pjgg January 13, 2023 09:02
@rsvoboda
Copy link
Member

Commit message is so vague, this can't go in - Commits are well encapsulated and follow the best practices

c'mon, some context needs to be in the message

otherwise we have just row of "Fix error in daily runs" commits

Copy link
Contributor

@pjgg pjgg left a comment

Choose a reason for hiding this comment

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

Please update PR and commit message, with a better description.

Starting from Quarkus 2.16, the trailing comma was dropped from Prometheus metrics
This change either allows tests to use both options, or disables them on older Quarkus
versions.
See quarkusio/quarkus#30343 for details
@fedinskiy fedinskiy force-pushed the fix/prometheus-format branch from 2bf6b28 to 9cf1fa2 Compare January 13, 2023 10:26
@fedinskiy fedinskiy requested a review from pjgg January 13, 2023 10:26
@pjgg pjgg changed the title Fix error in daily runs Fix daily runs: Adapt to the new format of Prometheus metrics Jan 13, 2023
Copy link
Contributor

@pjgg pjgg left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -22,7 +22,13 @@ public class HttpServerMetricsIT {
private static final int ASSERT_METRICS_TIMEOUT_MINUTES = 1;
private static final List<String> HTTP_SERVER_REQUESTS_METRICS_SUFFIX = Arrays.asList("count", "sum", "max");

private static final String HTTP_SERVER_REQUESTS_METRICS_FORMAT = "http_server_requests_seconds_%s{method=\"GET\",outcome=\"SUCCESS\",status=\"200\",uri=\"%s\",}";
/*
* In versions before 2.16 metrics have format '{a,b,}' (with trailing comma)
Copy link
Contributor

Choose a reason for hiding this comment

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

❤️

@fedinskiy fedinskiy merged commit c27474a into quarkus-qe:main Jan 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants