-
Notifications
You must be signed in to change notification settings - Fork 828
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
Add Jodd-Http instrumentation #7868
Add Jodd-Http instrumentation #7868
Conversation
|
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.
Very cool @PhilKes thanks for the contribution. It looks good to me, with just a few small suggestions (and at least one missing unit test).
...a/io/opentelemetry/javaagent/instrumentation/joddhttp/v4_2/JoddHttpHttpAttributesGetter.java
Show resolved
Hide resolved
...t/src/main/java/io/opentelemetry/javaagent/instrumentation/joddhttp/v4_2/JoddHttpHelper.java
Outdated
Show resolved
Hide resolved
...a/io/opentelemetry/javaagent/instrumentation/joddhttp/v4_2/JoddHttpHttpAttributesGetter.java
Outdated
Show resolved
Hide resolved
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.
Looks pretty good!
Thanks @PhilKes
.../io/opentelemetry/javaagent/instrumentation/joddhttp/v4_2/JoddHttpInstrumentationModule.java
Outdated
Show resolved
Hide resolved
...n/java/io/opentelemetry/javaagent/instrumentation/joddhttp/v4_2/JoddHttpInstrumentation.java
Outdated
Show resolved
Hide resolved
...t/src/main/java/io/opentelemetry/javaagent/instrumentation/joddhttp/v4_2/JoddHttpHelper.java
Outdated
Show resolved
Hide resolved
...a/io/opentelemetry/javaagent/instrumentation/joddhttp/v4_2/JoddHttpHttpAttributesGetter.java
Outdated
Show resolved
Hide resolved
…pentelemetry-java-instrumentation into feat/jodd-http-instrumentation
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.
Thanks @PhilKes !
...n/java/io/opentelemetry/javaagent/instrumentation/joddhttp/v4_2/JoddHttpInstrumentation.java
Outdated
Show resolved
Hide resolved
...n/java/io/opentelemetry/javaagent/instrumentation/joddhttp/v4_2/JoddHttpInstrumentation.java
Outdated
Show resolved
Hide resolved
Co-authored-by: Mateusz Rzeszutek <mrzeszutek@splunk.com>
This PR resolves #7629
This adds javaagent instrumentation for the jodd-http
HttpRequest
.It creates
Http Client Spans
andHttp Client Metrics
, the lowest supported version isorg.jodd:jodd-http:4.2.0
(most recent:6.3.0
), since this is the first version of the library supporting java 8, having follow-redirect capability andHttpRequest#overwriteHeader()
method. The instrumented method's signature and return typeHttpRequest#send()
has not been modified since, and therefore the instrumentation works for alljodd-http
versions above4.2.0
.Since this is my first contribution/instrumentation, I orientated myself on the
apache-httpclient-5.0
instrumentation, but obviously I would be glad to get some feedback on this