Opamp first iteration completion #2067
Conversation
LikeTheSalad
changed the title
|
@LikeTheSalad thanks for making progress on this. Once it is sufficiently complete we should also look into setting up interoperability tests that verify that Java and Go implementations can work together. |
| // Attaching animalsniffer check to the compilation process. | ||
| tasks.named("classes").configure { | ||
| finalizedBy("animalsnifferMain") | ||
| } |
There was a problem hiding this comment.
opentelemetry-java and opentelemetry-java-instrumentation don't have this
There was a problem hiding this comment.
Got it. It's just a way of getting notified of an api compatibility issue during development without having to run all the build checks, though it's not strictly necessary, so I've just removed it.
There was a problem hiding this comment.
I don't have anything particular against it just that it would be preferable to have this aligned with our other repositories.
| @@ -45,8 +47,14 @@ public CompletableFuture<Response> send(BodyWriter writer, int contentLength) { | |||
| RequestBody body = new RawRequestBody(writer, contentLength, MEDIA_TYPE); | |||
| builder.post(body); | |||
|
|
|||
| InstrumentationUtil.suppressInstrumentation(() -> doSendRequest(builder.build(), future)); | |||
There was a problem hiding this comment.
Perhaps there should be a comment explaining that this prevents automatic instrumentation from tracing the underlying okhttp request?
There was a problem hiding this comment.
Sure. I've just added a comment.
# Conflicts: # disk-buffering/build.gradle.kts
Finishes the first iteration of the
opamp-clientmodule by addressing these issues: #1902, #1956, and #1954.The following changes have been added as well: