Cover Vert.X-specific metrics. #1019

fedinskiy · 2023-01-27T13:09:44Z

Summary

Add new module, which uses Vert.X-based HTTP server Verify, that it works, and that it creates all required metrics Required for https://issues.redhat.com/browse/QUARKUS-2829

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

http/vertx/src/main/java/io/quarkus/ts/http/minimum/HelloResource.java

fedinskiy · 2023-01-27T14:27:29Z

run tests

pjgg

Over all looks good, but I am not sure if the app would be more "Vertx" with Reactive routes rather than Resteasy.

https://quarkus.io/guides/reactive-routes

Reactive Routes are still supported, especially if you want a more route-based approach, and something closer to the underlying reactive engine.

http/vertx/src/test/java/io/quarkus/ts/http/minimum/AbstractIT.java

pjgg · 2023-01-27T14:30:43Z

http/vertx/src/test/java/io/quarkus/ts/http/minimum/AbstractIT.java

+        Map<String, Metric> metrics = parseMetrics(body);
+        assertTrue(metrics.containsKey("worker_pool_active"));
+        assertTrue(metrics.containsKey("worker_pool_completed_total"));
+        assertTrue(metrics.containsKey("worker_pool_queue_size"));


Maybe we could also check the following metrics:
worker_pool_queue_delay_seconds_max
worker_pool_queue_delay_seconds_count
worker_pool_queue_delay_seconds_sum
worker_pool_usage_seconds_count
worker_pool_usage_seconds_sum
worker_pool_ratio
worker_pool_idle

They all go together anyway, and we cannot even reliably check the content of these. I am not sure additional checks will play a role there.

http/vertx/src/test/java/io/quarkus/ts/http/minimum/HttpMinimumIT.java

pjgg · 2023-01-27T14:39:03Z

http/vertx/src/test/java/io/quarkus/ts/http/minimum/OpenShiftHttpMinimumIT.java

+@OpenShiftScenario
+public class OpenShiftHttpMinimumIT extends AbstractIT {
+    @QuarkusApplication
+    static RestService app = new RestService();


I think that, you can move this RestService to "AbstractIT" and

@Override public RequestSpecification requests() { return app.given(); }

could be removed.

On AbstractIT just invoke restassurance.

for example:

public abstract class AbstractIT { @QuarkusApplication static RestService app = new RestService();

given().get("/hello?name=you").then().statusCode(HttpStatus.SC_OK).body("content", is("Hello, you!"));

So, all platform classes should be empty in this case, because you don't have any particular configuration into any component.

I would prefer to have all state in the endpoint classes and use the abstract one only as a source of common code.

ok but I think this is common to all the scenarios

@QuarkusApplication static RestService app = new RestService();

And this code Is not necessary:

@Override public RequestSpecification requests() { return app.given(); }

so all commonCode would be in vertxCommonsIT (the abstract one)

fedinskiy · 2023-01-27T15:08:43Z

Regarding reactive-routes — they are deprecated: https://code.quarkus.redhat.com/?e=reactive-routes&extension-search=origin:platform%20quarkus-reactive-routes

fedinskiy · 2023-01-27T16:00:16Z

run tests

pjgg · 2023-01-27T16:41:05Z

http/vertx/src/main/java/io/quarkus/ts/http/minimum/HelloResource.java

@@ -0,0 +1,33 @@
+package io.quarkus.ts.http.minimum;


is Http minimum the right package?

fedinskiy · 2023-01-30T09:08:53Z

run tests

pjgg

LGTM, let's wait to CI

pjgg

There are some errors on OCP native that should be reviewed

Add new module, which uses Vert.X-based HTTP server Verify, that it works, and that it creates all required metrics Required for https://issues.redhat.com/browse/QUARKUS-2829

fedinskiy · 2023-01-30T15:10:16Z

run tests

pjgg · 2023-01-31T08:08:00Z

Vertx OCP CI modules succeed: Quarkus QE TS: HTTP: Vert.X ........................ SUCCESS [11:26 min]

Add new module, which uses Vert.X-based HTTP server Verify, that it works, and that it creates all required metrics Required for https://issues.redhat.com/browse/QUARKUS-2829 (cherry picked from commit 818837f)

… modules (#1021) * Test Extended Architecture (XA) connection Provides coverage for https://issues.redhat.com/browse/QUARKUS-2742 (cherry picked from commit ce15b26) * Improve reactive rest client "process paths before sub resources" scenario (#995) There is an issue in upstream (quarkusio/quarkus#29821) that only happens under some RestClient definition hierarchy. This commit reproduces the problem in Quarkus 2.12 and earlier versions (cherry picked from commit 2350b46) * New Scenario: RequestScope custom context was removed after `javax.enterprise` event propagation When firing an async CDI Event, the requestScope context from the emitter briefly exists for the observer and is then terminated. This commit is a reproducer of this scenario that happens on Quarkus 2.13.0.Final (cherry picked from commit ef3eed6) * gRPC and SSE coverage for OpenTelemetry (cherry picked from commit f327c60) * Add coverage to eventbus '@ConsumeEvent' annotation (cherry picked from commit 7da0d4b) * Drop duplicated definition of quarkus-opentelemetry (cherry picked from commit b0cba7f) * OutboundSseEvent is not correctly serialized (cherry picked from commit 99b5eb1) * Check, that dev-mode is omitted on projects with pom packaging Required for QUARKUS-2757 (cherry picked from commit 4fd38c7) * Refactoring of QUARKUS-2748: adding http test and improving error messages (cherry picked from commit 6563431) * Add test for security annotations in rest-data-panache (#994) Quarkus extensions based on `rest-data-panache` support propagation of security annotations into generated JAX-RS resources. These tests provide coverage of this feature for extensions: - `quarkus-hibernate-orm-rest-data-panache` - `quarkus-spring-data-rest` See also related test plan: - https://github.com/quarkus-qe/quarkus-test-plans/blob/main/QUARKUS-2788.md (cherry picked from commit 3f7a4c8) * Add transaction-API classic scenario (cherry picked from commit 509d491) * Cover Vert.X-specific metrics. (#1019) Add new module, which uses Vert.X-based HTTP server Verify, that it works, and that it creates all required metrics Required for https://issues.redhat.com/browse/QUARKUS-2829 (cherry picked from commit 818837f) * Add missing opentelemetry exporter to Narayana scenario --------- Co-authored-by: Fedor Dudinskiy <[email protected]> Co-authored-by: Rostislav Svoboda <[email protected]> Co-authored-by: Josef Smrcka <[email protected]>

fedinskiy requested a review from pjgg January 27, 2023 13:09

pjgg reviewed Jan 27, 2023

View reviewed changes

http/vertx/src/main/java/io/quarkus/ts/http/minimum/HelloResource.java Outdated Show resolved Hide resolved

pjgg suggested changes Jan 27, 2023

View reviewed changes

fedinskiy force-pushed the feature/micrometer-vertx branch from 48899ce to 72202f2 Compare January 27, 2023 15:05

fedinskiy requested a review from pjgg January 27, 2023 15:08

fedinskiy added the triage/backport-2.13? It needs to backport changes to branch 2.13 label Jan 27, 2023

pjgg suggested changes Jan 27, 2023

View reviewed changes

fedinskiy force-pushed the feature/micrometer-vertx branch from 72202f2 to 17598d8 Compare January 30, 2023 08:16

pjgg self-requested a review January 30, 2023 08:34

pjgg approved these changes Jan 30, 2023

View reviewed changes

pjgg suggested changes Jan 30, 2023

View reviewed changes

Cover Vert.X-specific metrics.

f7e682f

Add new module, which uses Vert.X-based HTTP server Verify, that it works, and that it creates all required metrics Required for https://issues.redhat.com/browse/QUARKUS-2829

fedinskiy force-pushed the feature/micrometer-vertx branch from 17598d8 to f7e682f Compare January 30, 2023 15:06

pjgg self-requested a review January 31, 2023 08:08

pjgg approved these changes Jan 31, 2023

View reviewed changes

pjgg merged commit 818837f into quarkus-qe:main Jan 31, 2023

pjgg mentioned this pull request Jan 31, 2023

[2.13] Backport several scenarios related to HTTP, SQL and Monitoring modules #1021

Merged

1 task

pjgg added this to the 2.7 milestone Feb 2, 2023

pjgg removed the triage/backport-2.13? It needs to backport changes to branch 2.13 label Feb 2, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Cover Vert.X-specific metrics. #1019

Cover Vert.X-specific metrics. #1019

fedinskiy commented Jan 27, 2023

fedinskiy commented Jan 27, 2023

pjgg left a comment

pjgg Jan 27, 2023

fedinskiy Jan 27, 2023

pjgg Jan 27, 2023

fedinskiy Jan 27, 2023 •

edited

Loading

pjgg Jan 27, 2023

pjgg Jan 27, 2023

fedinskiy commented Jan 27, 2023

fedinskiy commented Jan 27, 2023

pjgg Jan 27, 2023

fedinskiy commented Jan 30, 2023

pjgg left a comment

pjgg left a comment

fedinskiy commented Jan 30, 2023

pjgg commented Jan 31, 2023

Cover Vert.X-specific metrics. #1019

Cover Vert.X-specific metrics. #1019

Conversation

fedinskiy commented Jan 27, 2023

Summary

Checklist:

fedinskiy commented Jan 27, 2023

pjgg left a comment

Choose a reason for hiding this comment

pjgg Jan 27, 2023

Choose a reason for hiding this comment

fedinskiy Jan 27, 2023

Choose a reason for hiding this comment

pjgg Jan 27, 2023

Choose a reason for hiding this comment

fedinskiy Jan 27, 2023 • edited Loading

Choose a reason for hiding this comment

pjgg Jan 27, 2023

Choose a reason for hiding this comment

pjgg Jan 27, 2023

Choose a reason for hiding this comment

fedinskiy commented Jan 27, 2023

fedinskiy commented Jan 27, 2023

pjgg Jan 27, 2023

Choose a reason for hiding this comment

fedinskiy commented Jan 30, 2023

pjgg left a comment

Choose a reason for hiding this comment

pjgg left a comment

Choose a reason for hiding this comment

fedinskiy commented Jan 30, 2023

pjgg commented Jan 31, 2023

fedinskiy Jan 27, 2023 •

edited

Loading