Vert.x Web instrumentation #1697

AlexanderWert · 2021-03-12T15:54:37Z

Provides Instrumentation for Vertx.x Web / HttpServer and Context propagation into Vert.x event engine.

apmmachine · 2021-03-12T16:14:25Z

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS

Expand to view the summary

Build stats

Build Cause: Pull request #1697 updated
Start Time: 2021-05-05T07:10:51.788+0000
Duration: 58 min 51 sec
Commit: b9833df

Test stats 🧪

Test	Results
Failed	0
Passed	2126
Skipped	19
Total	2145

Trends 🧪

💚 Flaky test report

Tests succeeded.

Expand to view the summary

Test stats 🧪

Test	Results
Failed	0
Passed	2126
Skipped	19
Total	2145

felixbarny

A first pass on this. Looks pretty good so far 👏

...x-3_6-plugin/src/main/java/co/elastic/apm/agent/vertx_3_6/VertxEventLoopInstrumentation.java

.../apm-vertx-3_6-plugin/src/main/java/co/elastic/apm/agent/vertx_3_6/Http1Instrumentation.java

...m-vertx-3_6-plugin/src/main/java/co/elastic/apm/agent/vertx_3_6/VertxWebInstrumentation.java

...x-3_6-plugin/src/main/java/co/elastic/apm/agent/vertx_3_6/wrapper/GenericHandlerWrapper.java

...s/apm-vertx/apm-vertx-common/src/main/java/co/elastic/apm/agent/vertx/NettyByteTransfer.java

...apm-vertx-3_6-plugin/src/main/java/co/elastic/apm/agent/vertx_3_6/helper/VertxWebHelper.java

.../apm-vertx-3_6-plugin/src/main/java/co/elastic/apm/agent/vertx_3_6/Http1Instrumentation.java

...x-3_6-plugin/src/main/java/co/elastic/apm/agent/vertx_3_6/wrapper/GenericHandlerWrapper.java

...x/apm-vertx-4-plugin/src/main/java/co/elastic/apm/agent/vertx_4/VertxWebInstrumentation.java

...x/apm-vertx-common/src/main/java/co/elastic/apm/agent/vertx/MultiMapHeadersGetterSetter.java

...vertx-4-plugin/src/main/java/co/elastic/apm/agent/vertx_4/VertxEventLoopInstrumentation.java

...tx-common/src/main/java/co/elastic/apm/agent/vertx/AbstractRoutingContextHandlerWrapper.java

…test reliability.

…-instrumentation

…agent-java into vertx-instrumentation

…-instrumentation

SylvainJuge

LGTM

...apm-vertx-common/src/main/java/co/elastic/apm/agent/vertx/AbstractHttpTransactionHelper.java

SylvainJuge · 2021-05-03T08:12:00Z

...vertx/apm-vertx4-plugin/src/main/java/co/elastic/apm/agent/vertx/v4/web/NoopVertxTracer.java

+import io.vertx.core.spi.tracing.VertxTracer;
+import io.vertx.core.tracing.TracingPolicy;
+
+class NoopVertxTracer implements VertxTracer<Object, Object> {


[minor] We could simply reuse the io.vertx.core.spi.tracing.VertxTracer#NOOP constant instead

Good point, missed there is a NOOP tracer.

Tried to do the same with the io.vertx.core.spi.tracing.VertxTracer#NOOP tracer. Unfortunately, NOOP is returning null as the trace object so that the corresponding sendResponse method of the traces isn't called at all.
--> We need to use our own NoopVertxTracer!

SylvainJuge · 2021-05-03T08:14:32Z

...tx/apm-vertx4-plugin/src/main/java/co/elastic/apm/agent/vertx/v4/web/WebInstrumentation.java

+
+        @Override
+        public final ElementMatcher<? super TypeDescription> getTypeMatcher() {
+            return hasSuperType(named("io.vertx.core.spi.tracing.VertxTracer"))


[question] Does it means we instrument any existing tracer if there is any and if there is none we inject our own no-op which comes not-so-no-op anymore once it's instrumented ?

In that case if you have time to add a minor comment to explain that a bit and make it more obvious that would really be nice. I'm not sure that I'll remember that trick in a few months :-).

github-actions bot added the agent-java label Mar 12, 2021

elastic-apm-tech added this to In Progress in APM-Agents (OLD) Mar 12, 2021

AlexanderWert requested a review from felixbarny March 12, 2021 15:56

felixbarny reviewed Mar 15, 2021

View reviewed changes

SylvainJuge self-assigned this Mar 15, 2021

felixbarny reviewed Mar 16, 2021

View reviewed changes

.../apm-vertx-3_6-plugin/src/main/java/co/elastic/apm/agent/vertx_3_6/Http1Instrumentation.java Outdated Show resolved Hide resolved

AlexanderWert marked this pull request as ready for review March 17, 2021 15:46

felixbarny reviewed Mar 18, 2021

View reviewed changes

...x-3_6-plugin/src/main/java/co/elastic/apm/agent/vertx_3_6/wrapper/GenericHandlerWrapper.java Outdated Show resolved Hide resolved

...x-3_6-plugin/src/main/java/co/elastic/apm/agent/vertx_3_6/wrapper/GenericHandlerWrapper.java Outdated Show resolved Hide resolved

felixbarny reviewed Mar 25, 2021

View reviewed changes

...x/apm-vertx-4-plugin/src/main/java/co/elastic/apm/agent/vertx_4/VertxWebInstrumentation.java Outdated Show resolved Hide resolved

felixbarny reviewed Mar 25, 2021

View reviewed changes

...x/apm-vertx-common/src/main/java/co/elastic/apm/agent/vertx/MultiMapHeadersGetterSetter.java Outdated Show resolved Hide resolved

...vertx-4-plugin/src/main/java/co/elastic/apm/agent/vertx_4/VertxEventLoopInstrumentation.java Outdated Show resolved Hide resolved

felixbarny reviewed Mar 26, 2021

View reviewed changes

...tx-common/src/main/java/co/elastic/apm/agent/vertx/AbstractRoutingContextHandlerWrapper.java Outdated Show resolved Hide resolved

Vert.x Web instrumentation

88be895

AlexanderWert force-pushed the vertx-instrumentation branch from 5bdf34f to 88be895 Compare March 29, 2021 07:40

Updated version to 1.22

a423b8f

AlexanderWert added this to the 7.13 milestone Mar 29, 2021

Increased timeout for parallel / asnyc transactions test to increase …

f6fffbb

…test reliability.

cyrille-leclerc modified the milestones: 7.13, 7.14 Apr 13, 2021

AlexanderWert added the focus label Apr 19, 2021

SylvainJuge and others added 10 commits April 20, 2021 16:00

code reuse + cleanup

18fa624

avoid 'file-uploads' folder creation

a0b4d44

Merge branch 'master' into vertx-instrumentation

45604cd

inline advices classes when possible

900341e

avoid '.vertx' cache folder + fix instrumentation

a505f23

move vertx 3.x classes

3f237c5

code reformat

3a22bfe

Merge branch 'master' into vertx-instrumentation

dbd3f23

Merge branch 'master' of github.com:elastic/apm-agent-java into vertx…

9f3c3e9

…-instrumentation

move things around again

03834f2

SylvainJuge added 10 commits April 28, 2021 17:38

move things around + update to latest

3c3c3d7

cleanup

52ab64d

start optimize matching

e86c876

fast pre-filter with app packages

431a504

minor code change

010b520

Merge branch 'vertx-instrumentation' of github.com:AlexanderWert/apm-…

76e157a

…agent-java into vertx-instrumentation

Merge branch 'master' into vertx-instrumentation

f9ebe57

remove relying on application packages

edc6ba5

Merge branch 'vertx-instrumentation' of github.com:AlexanderWert/apm-…

314ce33

…agent-java into vertx-instrumentation

Merge branch 'master' of github.com:elastic/apm-agent-java into vertx…

43f893d

…-instrumentation

SylvainJuge approved these changes May 3, 2021

View reviewed changes

SylvainJuge and others added 3 commits May 3, 2021 10:20

minor trivial code esthetics

b9e93ed

Added Vert.x to changelog

115b9dc

Merge branch 'master' into vertx-instrumentation

b9833df

AlexanderWert merged commit 04ae148 into elastic:master May 5, 2021

APM-Agents (OLD) automation moved this from In Progress to Done May 5, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Vert.x Web instrumentation #1697

Vert.x Web instrumentation #1697

AlexanderWert commented Mar 12, 2021

apmmachine commented Mar 12, 2021 •

edited

Build stats

Test stats 🧪

Trends 🧪

Test stats 🧪

felixbarny left a comment

SylvainJuge left a comment

SylvainJuge May 3, 2021

AlexanderWert May 3, 2021

AlexanderWert May 5, 2021

SylvainJuge May 3, 2021

AlexanderWert May 3, 2021

SylvainJuge May 3, 2021

Vert.x Web instrumentation #1697

Vert.x Web instrumentation #1697

Conversation

AlexanderWert commented Mar 12, 2021

apmmachine commented Mar 12, 2021 • edited

💚 Build Succeeded

Build stats

Test stats 🧪

Trends 🧪

💚 Flaky test report

Test stats 🧪

felixbarny left a comment

Choose a reason for hiding this comment

SylvainJuge left a comment

Choose a reason for hiding this comment

SylvainJuge May 3, 2021

Choose a reason for hiding this comment

AlexanderWert May 3, 2021

Choose a reason for hiding this comment

AlexanderWert May 5, 2021

Choose a reason for hiding this comment

SylvainJuge May 3, 2021

Choose a reason for hiding this comment

AlexanderWert May 3, 2021

Choose a reason for hiding this comment

SylvainJuge May 3, 2021

Choose a reason for hiding this comment

apmmachine commented Mar 12, 2021 •

edited