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

Improvements to the Grafana LGTM dashboards I #45620

Merged
merged 1 commit into from
Jan 16, 2025

Conversation

brunobat
Copy link
Contributor

@brunobat brunobat commented Jan 15, 2025

Partial fix for: #43599
More PRs on the way.

@@ -51,7 +50,7 @@ protected byte[] getResourceAsBytes(String resource) {

@SuppressWarnings("OctalInteger")
protected void addFileToContainer(byte[] content, String pathInContainer) {
log.infof("Content [%s]: \n%s", pathInContainer, new String(content, StandardCharsets.UTF_8));
// log.infof("Content [%s]: \n%s", pathInContainer, new String(content, StandardCharsets.UTF_8));
Copy link
Member

Choose a reason for hiding this comment

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

Ah cool, I found that quite annoying.

Copy link
Contributor

Choose a reason for hiding this comment

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

What about some more condensed view ... e.g. first few lines.
So that the user knows something got added ...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If the user cannot do anything with that info, I would remove it altogether or set it to debug.

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've set it to debug

@@ -82,7 +119,7 @@ private String getPrometheusConfig() {
Config runtimeConfig = ConfigProvider.getConfig();
String rootPath = runtimeConfig.getOptionalValue("quarkus.management.root-path", String.class).orElse("/q");
String metricsPath = runtimeConfig.getOptionalValue("quarkus.management.metrics.path", String.class).orElse("/metrics");
int httpPort = runtimeConfig.getOptionalValue("quarkus.http.port", Integer.class).orElse(0);
int httpPort = runtimeConfig.getOptionalValue("quarkus.http.port", Integer.class).orElse(8080); // when not set use default
Copy link
Contributor

Choose a reason for hiding this comment

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

Doesn't the 0 do that -- helps set the default?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The problem is that 0 is the default and should mean some random port, however, in devmode, the default port is 8080.
Unless the property is set by the user, the port ended up being set to 0 in the scraper, leading to no data collection. An insidious bug.

@brunobat brunobat changed the title Improvements to the Grafana LGTM dashboards Improvements to the Grafana LGTM dashboards I Jan 16, 2025
@brunobat brunobat marked this pull request as ready for review January 16, 2025 09:28

This comment has been minimized.

@brunobat brunobat force-pushed the grafana-dashboard-improvements branch from 37791ed to 9acf79d Compare January 16, 2025 10:09
Copy link

quarkus-bot bot commented Jan 16, 2025

Status for workflow Quarkus CI

This is the status report for running Quarkus CI on commit 9acf79d.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

You can consult the Develocity build scans.

@brunobat
Copy link
Contributor Author

brunobat commented Jan 16, 2025

@gsmet we decided to split the work into different PRs.
This one removes a hack, fixes a bug and upgrades the grafana version.
It can be back-ported if you agree.

@brunobat brunobat merged commit 033874f into quarkusio:main Jan 16, 2025
20 checks passed
@quarkus-bot quarkus-bot bot added this to the 3.19 - main milestone Jan 16, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants