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

LGTM Quarkus Dashboard #41264

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

melloware
Copy link
Contributor

@melloware melloware commented Jun 17, 2024

Fix #40933: LGTM Quarkus Dashboard
Fix #41944

cc @brunobat @alesj

  • Adds Quarkus Micrometer Dashboard including JVM, HTTP, JDBC and other stats provided by Micrometer.
  • I just need some guidance on the TODO how to get those values properly

image

@melloware melloware marked this pull request as ready for review June 18, 2024 13:06
@melloware
Copy link
Contributor Author

OK I resolved that issue

@melloware melloware force-pushed the lgtm-40933 branch 2 times, most recently from 90c1a11 to 1c41146 Compare July 3, 2024 21:25
@gastaldi gastaldi requested a review from brunobat July 5, 2024 11:27
@quarkus-bot

This comment has been minimized.

@quarkus-bot

This comment has been minimized.

@brunobat
Copy link
Contributor

@melloware, back from vacations.
I wonder if we could deploy this dashboard on the grafana repository, as well.
Will need some time to review this.

@melloware
Copy link
Contributor Author

yeah i put this here because i wasn't sure if we wanted to get feedback from other Quarkus Users if there was anything missing they want on there?

@melloware
Copy link
Contributor Author

i also think the colors and times for uptimes might need to be tweaked but i thought it was a pretty good first stab 😄

@melloware melloware force-pushed the lgtm-40933 branch 3 times, most recently from bee696e to 1b6fca0 Compare August 20, 2024 12:20
@quarkus-bot

This comment has been minimized.

@quarkus-bot

This comment has been minimized.

Copy link
Contributor

@brunobat brunobat left a comment

Choose a reason for hiding this comment

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

We need to make sure the app will return data for all the dashboard components and understand why they lack data or is we need to instrument something else...
I've used this app, which is basically the Quick start"https://github.com/brunobat/quarkus-lab/blob/main/micrometer-lgtm/pom.xml
But with it, out of the box, the dashboard looks like this:
Screenshot 2024-09-17 at 11 36 01
Screenshot 2024-09-17 at 11 36 16

@brunobat
Copy link
Contributor

Also, we should pull up the HTTP parts.

@melloware
Copy link
Contributor Author

Yes this was more of an idea, and we need to check the dashboard for startup time those eventually fill in if you leave the app running.

@melloware melloware force-pushed the lgtm-40933 branch 2 times, most recently from e4a6c25 to 280f66d Compare September 17, 2024 11:54
@quarkus-bot quarkus-bot bot added area/arc Issue related to ARC (dependency injection) area/core labels Sep 17, 2024
@quarkus-bot quarkus-bot bot added area/devtools Issues/PR related to maven, gradle, platform and cli tooling/plugins area/docstyle issues related for manual docstyle review area/documentation area/graphql area/infra-automation anything related to CI, bots, etc. that are used to automated our infrastructure area/kafka area/keycloak area/metrics area/oidc area/reactive-messaging area/rest area/rest-client area/smallrye area/vertx labels Sep 17, 2024
@melloware melloware force-pushed the lgtm-40933 branch 4 times, most recently from 6466da8 to 0bb22b6 Compare September 17, 2024 11:57
@melloware
Copy link
Contributor Author

@brunobat ok i updated to move HTTP Endpoints up. Here is what mine looks like not sure why yours shows N/A at first nuless you are not wating 30 seconds for the scrape?

image

@brunobat
Copy link
Contributor

@melloware strange... I waited a few minutes.
What app are you using to feed data to the dashboard?
I'd like to take a look at the dependencies.

@melloware
Copy link
Contributor Author

Hmm i was just using a REST app with...

<dependency>
			<groupId>io.quarkus</groupId>
			<artifactId>quarkus-rest-jackson</artifactId>
		</dependency>
<dependency>
    <groupId>io.quarkus</groupId>
    <artifactId>quarkus-observability-devservices-lgtm</artifactId>
    <scope>provided</scope>
</dependency>
<dependency>
			<groupId>io.quarkus</groupId>
			<artifactId>quarkus-opentelemetry</artifactId>
		</dependency>
		<dependency>
			<groupId>io.opentelemetry.instrumentation</groupId>
			<artifactId>opentelemetry-jdbc</artifactId>
		</dependency>
		<dependency>
			<groupId>io.quarkus</groupId>
			<artifactId>quarkus-micrometer-registry-prometheus</artifactId>
		</dependency>

@brunobat
Copy link
Contributor

brunobat commented Sep 17, 2024

I understand what's going on now...
You have configured the prometheus scraper in the PR and I'm using the Micrometer OTLP registry that pushes things.
I think it makes sense to have both setups, however the visualizations will be different.

I wonder if the scraper could be activated by a property. I'm not sure if it should be on or off by default.
Ideally, all the output, in the future, should be OTLP (OpenTelemetry).

@melloware
Copy link
Contributor Author

its just checking the Micrometer metric "expr": "process_start_time_seconds * 1000" which is exposed in the Prometheus metrics.

{
          "datasource": {
            "type": "prometheus",
            "uid": "prometheus"
          },
          "expr": "process_start_time_seconds * 1000",
          "format": "time_series",
          "intervalFactor": 2,
          "legendFormat": "",
          "metric": "",
          "refId": "A",
          "step": 14400
        }

@quarkus-bot
Copy link

quarkus-bot bot commented Sep 17, 2024

Status for workflow Quarkus CI

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

✅ 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

One solution can be the least common denominator and only show metrics available with both registries on the dashboard. What do you think @melloware ?

@@ -25,12 +43,29 @@ public LgtmContainer(LgtmConfig config) {
super(config);
addExposedPorts(config.otlpPort());
withLogConsumer(new LgtmContainerLogConsumer(log).withPrefix("LGTM"));
withCopyFileToContainer(
Copy link
Contributor

Choose a reason for hiding this comment

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

This things should be made configurable -- at least: enabled / disabled.
e.g. you don't need scraping always, some registry impls know how to periodically push metrics

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes and it can be done automatically.
You can refactor things a bit so you can use his data from the ExtensionsCatalog object:

Copy link
Contributor

Choose a reason for hiding this comment

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

OK, this can be used with actual DevServices, but not with the other 2 "flavours" - DevResource and QuarkusTestResource.
Where DevResource is pretty much non-configurable, so that might be a problem.
Where QuarkusTestResource can probably get this config via QuarkusTestResourceLifecycleManager::init ...

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 will be the first to admit i am no expert on this stuff so any help is appreciated. I basically wanted to show how we can load our own dashboard and get metrics from the running DEV mode as currenlty the LGTM defaults have dashboards that do nothing?

Copy link
Contributor

Choose a reason for hiding this comment

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

No worries.
I think we should probably show the minimum common denominator metrics. The ones common to both the Prometheus and the otlp registries.
This way, all metrics will show something and it's good enough to start with... It is also simpler to develop and it's just a meter of removing stuff from the dashboard.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh OK if you tell me which sections you want to "stay" i can rip all others out.

Copy link
Contributor

Choose a reason for hiding this comment

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

Will come up with something tomorrow

@edeandrea
Copy link
Contributor

We need this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/arc Issue related to ARC (dependency injection) area/core area/dependencies Pull requests that update a dependency file area/devtools Issues/PR related to maven, gradle, platform and cli tooling/plugins area/docstyle issues related for manual docstyle review area/documentation area/graphql area/infra-automation anything related to CI, bots, etc. that are used to automated our infrastructure area/kafka area/keycloak area/metrics area/oidc area/reactive-messaging area/rest area/rest-client area/smallrye area/vertx
Projects
Status: Todo
5 participants