-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
base: main
Are you sure you want to change the base?
LGTM Quarkus Dashboard #41264
Conversation
...ices/testcontainers/src/main/java/io/quarkus/observability/testcontainers/LgtmContainer.java
Outdated
Show resolved
Hide resolved
OK I resolved that issue |
90c1a11
to
1c41146
Compare
This comment has been minimized.
This comment has been minimized.
f312998
to
20135bd
Compare
This comment has been minimized.
This comment has been minimized.
@melloware, back from vacations. |
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? |
i also think the colors and times for uptimes might need to be tweaked but i thought it was a pretty good first stab 😄 |
...testcontainers/src/main/java/io/quarkus/observability/testcontainers/PrometheusYamlFile.java
Outdated
Show resolved
Hide resolved
bee696e
to
1b6fca0
Compare
This comment has been minimized.
This comment has been minimized.
1b6fca0
to
11ede0d
Compare
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this 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:
Also, we should pull up the HTTP parts. |
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. |
e4a6c25
to
280f66d
Compare
6466da8
to
0bb22b6
Compare
0bb22b6
to
fcd314f
Compare
@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? |
@melloware strange... I waited a few minutes. |
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> |
I understand what's going on now... I wonder if the scraper could be activated by a property. I'm not sure if it should be on or off by default. |
its just checking the Micrometer metric {
"datasource": {
"type": "prometheus",
"uid": "prometheus"
},
"expr": "process_start_time_seconds * 1000",
"format": "time_series",
"intervalFactor": 2,
"legendFormat": "",
"metric": "",
"refId": "A",
"step": 14400
} |
Status for workflow
|
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( |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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:
There was a problem hiding this comment.
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 ...
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
We need this! |
Fix #40933: LGTM Quarkus Dashboard
Fix #41944
cc @brunobat @alesj