From 7e4390f32133138a5b748a9e10c39bba13da46f5 Mon Sep 17 00:00:00 2001 From: Cesar Munoz <56847527+LikeTheSalad@users.noreply.github.com> Date: Wed, 30 Jul 2025 09:02:41 +0200 Subject: [PATCH 01/13] Making OpanClient closeable --- .../opamp/client/internal/OpampClient.java | 44 ++++---------- .../client/internal/OpampClientBuilder.java | 8 +-- .../client/internal/impl/OpampClientImpl.java | 59 ++++++++----------- .../internal/impl/OpampClientImplTest.java | 21 ++----- 4 files changed, 39 insertions(+), 93 deletions(-) diff --git a/opamp-client/src/main/java/io/opentelemetry/opamp/client/internal/OpampClient.java b/opamp-client/src/main/java/io/opentelemetry/opamp/client/internal/OpampClient.java index c835c6f55..5889642d8 100644 --- a/opamp-client/src/main/java/io/opentelemetry/opamp/client/internal/OpampClient.java +++ b/opamp-client/src/main/java/io/opentelemetry/opamp/client/internal/OpampClient.java @@ -6,45 +6,23 @@ package io.opentelemetry.opamp.client.internal; import io.opentelemetry.opamp.client.internal.response.MessageData; +import java.io.Closeable; import javax.annotation.Nullable; import opamp.proto.AgentDescription; import opamp.proto.RemoteConfigStatus; import opamp.proto.ServerErrorResponse; -public interface OpampClient { +public interface OpampClient extends Closeable { static OpampClientBuilder builder() { return new OpampClientBuilder(); } /** - * Starts the client and begin attempts to connect to the Server. Once connection is established - * the client will attempt to maintain it by reconnecting if the connection is lost. All failed - * connection attempts will be reported via {@link Callbacks#onConnectFailed(Throwable)} callback. - * - *

This method does not wait until the connection to the Server is established and will likely - * return before the connection attempts are even made. - * - *

This method may be called only once. - * - * @param callbacks The Callback to which the Client will notify about any Server requests and - * responses. - */ - void start(Callbacks callbacks); - - /** - * Stops the client. May be called only after {@link #start(Callbacks)}. May be called only once. - * After this call returns successfully it is guaranteed that no callbacks will be called. Once - * stopped, the client cannot be started again. - */ - void stop(); - - /** - * Sets attributes of the Agent. The attributes will be included in the next status report sent to - * the Server. When called after {@link #start(Callbacks)}, the attributes will be included in the - * next outgoing status report. This is typically used by Agents which allow their - * AgentDescription to change dynamically while the OpAMPClient is started. May be also called - * from {@link Callbacks#onMessage(MessageData)}. + * Sets attributes of the Agent. The attributes will be included in the next outgoing status + * report. This is typically used by Agents which allow their AgentDescription to change + * dynamically while the OpAMPClient is started. May be also called from {@link + * Callbacks#onMessage(MessageData)}. * * @param agentDescription The new agent description. */ @@ -59,16 +37,14 @@ static OpampClientBuilder builder() { interface Callbacks { /** - * Called when the connection is successfully established to the Server. May be called after - * {@link #start(Callbacks)} is called and every time a connection is established to the Server. - * For WebSocket clients this is called after the handshake is completed without any error. For - * HTTP clients this is called for any request if the response status is OK. + * Called when the connection is successfully established to the Server. For WebSocket clients + * this is called after the handshake is completed without any error. For HTTP clients this is + * called for any request if the response status is OK. */ void onConnect(); /** - * Called when the connection to the Server cannot be established. May be called after {@link - * #start(Callbacks)} is called and tries to connect to the Server. May also be called if the + * Called when the connection to the Server cannot be established. May also be called if the * connection is lost and reconnection attempt fails. * * @param throwable The exception. diff --git a/opamp-client/src/main/java/io/opentelemetry/opamp/client/internal/OpampClientBuilder.java b/opamp-client/src/main/java/io/opentelemetry/opamp/client/internal/OpampClientBuilder.java index 7e021c716..3e4d0f4aa 100644 --- a/opamp-client/src/main/java/io/opentelemetry/opamp/client/internal/OpampClientBuilder.java +++ b/opamp-client/src/main/java/io/opentelemetry/opamp/client/internal/OpampClientBuilder.java @@ -381,11 +381,7 @@ public OpampClientBuilder setEffectiveConfigState(State.EffectiveConfig effectiv return this; } - public OpampClient build() { - if (service == null) { - throw new IllegalStateException( - "The request service is not set. You must provide it by calling setRequestService()"); - } + public OpampClient build(OpampClient.Callbacks callbacks) { List protoIdentifyingAttributes = new ArrayList<>(); List protoNonIdentifyingAttributes = new ArrayList<>(); identifyingAttributes.forEach( @@ -411,7 +407,7 @@ public OpampClient build() { new State.InstanceUid(instanceUid), new State.Flags(0L), effectiveConfigState); - return OpampClientImpl.create(service, state); + return OpampClientImpl.create(service, state, callbacks); } private static State.EffectiveConfig createEffectiveConfigNoop() { diff --git a/opamp-client/src/main/java/io/opentelemetry/opamp/client/internal/impl/OpampClientImpl.java b/opamp-client/src/main/java/io/opentelemetry/opamp/client/internal/impl/OpampClientImpl.java index 8dcf6bb7e..7ada06c42 100644 --- a/opamp-client/src/main/java/io/opentelemetry/opamp/client/internal/impl/OpampClientImpl.java +++ b/opamp-client/src/main/java/io/opentelemetry/opamp/client/internal/impl/OpampClientImpl.java @@ -28,11 +28,9 @@ import java.util.ArrayList; import java.util.Collections; import java.util.List; -import java.util.Objects; import java.util.concurrent.atomic.AtomicBoolean; import java.util.function.Supplier; import javax.annotation.Nonnull; -import javax.annotation.Nullable; import okio.ByteString; import opamp.proto.AgentDescription; import opamp.proto.AgentToServer; @@ -51,9 +49,8 @@ public final class OpampClientImpl private final AgentToServerAppenders appenders; private final OpampClientState state; private final RecipeManager recipeManager; - private final AtomicBoolean isRunning = new AtomicBoolean(false); private final AtomicBoolean hasStopped = new AtomicBoolean(false); - @Nullable private Callbacks callbacks; + private final Callbacks callbacks; /** Fields that must always be sent. */ private static final List REQUIRED_FIELDS; @@ -84,7 +81,8 @@ public final class OpampClientImpl COMPRESSABLE_FIELDS = Collections.unmodifiableList(compressableFields); } - public static OpampClientImpl create(RequestService requestService, OpampClientState state) { + public static OpampClientImpl create( + RequestService requestService, OpampClientState state, Callbacks callbacks) { AgentToServerAppenders appenders = new AgentToServerAppenders( AgentDescriptionAppender.create(state.agentDescription), @@ -95,41 +93,35 @@ public static OpampClientImpl create(RequestService requestService, OpampClientS InstanceUidAppender.create(state.instanceUid), FlagsAppender.create(state.flags), AgentDisconnectAppender.create()); - return new OpampClientImpl( - requestService, appenders, state, RecipeManager.create(REQUIRED_FIELDS)); + OpampClientImpl client = + new OpampClientImpl( + requestService, appenders, state, RecipeManager.create(REQUIRED_FIELDS), callbacks); + + // Start + requestService.start(client, client); + client.disableCompression(); + client.startObservingStateChange(); + requestService.sendRequest(); + + return client; } private OpampClientImpl( RequestService requestService, AgentToServerAppenders appenders, OpampClientState state, - RecipeManager recipeManager) { + RecipeManager recipeManager, + Callbacks callbacks) { this.requestService = requestService; this.appenders = appenders; this.state = state; this.recipeManager = recipeManager; + this.callbacks = callbacks; } @Override - public void start(@Nonnull Callbacks callbacks) { - if (hasStopped.get()) { - throw new IllegalStateException("The client cannot start after it has been stopped."); - } - if (isRunning.compareAndSet(false, true)) { - this.callbacks = callbacks; - requestService.start(this, this); - disableCompression(); - startObservingStateChange(); - requestService.sendRequest(); - } else { - throw new IllegalStateException("The client has already been started"); - } - } - - @Override - public void stop() { - if (isRunning.compareAndSet(true, false)) { - hasStopped.set(true); + public void close() { + if (hasStopped.compareAndSet(false, true)) { stopObservingStateChange(); prepareDisconnectRequest(); requestService.stop(); @@ -154,12 +146,12 @@ public void setRemoteConfigStatus(@Nonnull RemoteConfigStatus remoteConfigStatus @Override public void onConnectionSuccess() { - getCallbacks().onConnect(); + callbacks.onConnect(); } @Override public void onConnectionFailed(Throwable throwable) { - getCallbacks().onConnectFailed(throwable); + callbacks.onConnectFailed(throwable); } @Override @@ -176,7 +168,7 @@ public void onRequestFailed(Throwable throwable) { preserveFailedRequestRecipe(); if (throwable instanceof OpampServerResponseException) { ServerErrorResponse errorResponse = ((OpampServerResponseException) throwable).errorResponse; - getCallbacks().onErrorResponse(errorResponse); + callbacks.onErrorResponse(errorResponse); } } @@ -203,7 +195,7 @@ private void handleResponsePayload(ServerToAgent response) { } if (notifyOnMessage) { - getCallbacks().onMessage(messageBuilder.build()); + callbacks.onMessage(messageBuilder.build()); } } @@ -224,11 +216,6 @@ private void prepareDisconnectRequest() { recipeManager.next().addField(Field.AGENT_DISCONNECT); } - @Nonnull - private Callbacks getCallbacks() { - return Objects.requireNonNull(callbacks); - } - @Override public Request get() { AgentToServer.Builder builder = new AgentToServer.Builder(); diff --git a/opamp-client/src/test/java/io/opentelemetry/opamp/client/internal/impl/OpampClientImplTest.java b/opamp-client/src/test/java/io/opentelemetry/opamp/client/internal/impl/OpampClientImplTest.java index ec630dc7e..a976a50ad 100644 --- a/opamp-client/src/test/java/io/opentelemetry/opamp/client/internal/impl/OpampClientImplTest.java +++ b/opamp-client/src/test/java/io/opentelemetry/opamp/client/internal/impl/OpampClientImplTest.java @@ -6,7 +6,6 @@ package io.opentelemetry.opamp.client.internal.impl; import static org.assertj.core.api.Assertions.assertThat; -import static org.assertj.core.api.Assertions.fail; import static org.awaitility.Awaitility.await; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.never; @@ -90,7 +89,7 @@ void setUp() { @AfterEach void tearDown() { - client.stop(); + client.close(); } @Test @@ -178,23 +177,12 @@ void verifyStop() { initializeClient(); enqueueServerToAgentResponse(new ServerToAgent.Builder().build()); - client.stop(); + client.close(); AgentToServer agentToServerMessage = getAgentToServerMessage(takeRequest()); assertThat(agentToServerMessage.agent_disconnect).isNotNull(); } - @Test - void verifyStartOnlyOnce() { - initializeClient(); - try { - client.start(callbacks); - fail("Should have thrown an exception"); - } catch (IllegalStateException e) { - assertThat(e).hasMessage("The client has already been started"); - } - } - @Test void onSuccess_withChangesToReport_notifyCallbackOnMessage() { initializeClient(); @@ -403,13 +391,12 @@ private RecordedRequest initializeClient() { } private RecordedRequest initializeClient(ServerToAgent initialResponse) { - client = OpampClientImpl.create(requestService, state); - // Prepare first request on start enqueueServerToAgentResponse(initialResponse); callbacks = spy(new TestCallbacks()); - client.start(callbacks); + client = OpampClientImpl.create(requestService, state, callbacks); + return takeRequest(); } From a3d4a5426a6206571c6c6d9a67ef4a5aa6cd22fd Mon Sep 17 00:00:00 2001 From: Cesar Munoz <56847527+LikeTheSalad@users.noreply.github.com> Date: Wed, 30 Jul 2025 10:03:57 +0200 Subject: [PATCH 02/13] Creating animalsniffer convention --- buildSrc/build.gradle.kts | 1 + .../otel.animalsniffer-conventions.gradle.kts | 24 +++++++++++++++++++ disk-buffering/build.gradle.kts | 18 +------------- 3 files changed, 26 insertions(+), 17 deletions(-) create mode 100644 buildSrc/src/main/kotlin/otel.animalsniffer-conventions.gradle.kts diff --git a/buildSrc/build.gradle.kts b/buildSrc/build.gradle.kts index ba5b81ceb..bf473fff1 100644 --- a/buildSrc/build.gradle.kts +++ b/buildSrc/build.gradle.kts @@ -16,6 +16,7 @@ dependencies { implementation("net.ltgt.gradle:gradle-errorprone-plugin:4.3.0") implementation("net.ltgt.gradle:gradle-nullaway-plugin:2.2.0") implementation("org.owasp:dependency-check-gradle:12.1.3") + implementation("ru.vyarus.animalsniffer:ru.vyarus.animalsniffer.gradle.plugin:2.0.1") } spotless { diff --git a/buildSrc/src/main/kotlin/otel.animalsniffer-conventions.gradle.kts b/buildSrc/src/main/kotlin/otel.animalsniffer-conventions.gradle.kts new file mode 100644 index 000000000..0e10e8b04 --- /dev/null +++ b/buildSrc/src/main/kotlin/otel.animalsniffer-conventions.gradle.kts @@ -0,0 +1,24 @@ +import ru.vyarus.gradle.plugin.animalsniffer.AnimalSniffer + +plugins { + id("otel.java-conventions") + id("ru.vyarus.animalsniffer") +} + +dependencies { + signature("com.toasttab.android:gummy-bears-api-21:0.12.0:coreLib@signature") +} + +animalsniffer { + sourceSets = listOf(java.sourceSets.main.get()) +} + +// Always having declared output makes this task properly participate in tasks up-to-date checks +tasks.withType { + reports.text.required.set(true) +} + +// Attaching animalsniffer check to the compilation process. +tasks.named("classes").configure { + finalizedBy("animalsnifferMain") +} diff --git a/disk-buffering/build.gradle.kts b/disk-buffering/build.gradle.kts index 8250c1bdf..794beb26d 100644 --- a/disk-buffering/build.gradle.kts +++ b/disk-buffering/build.gradle.kts @@ -1,12 +1,11 @@ import com.github.jengelman.gradle.plugins.shadow.tasks.ShadowJar -import ru.vyarus.gradle.plugin.animalsniffer.AnimalSniffer plugins { id("otel.java-conventions") id("otel.publish-conventions") + id("otel.animalsniffer-conventions") id("com.github.johnrengelman.shadow") id("me.champeau.jmh") version "0.7.3" - id("ru.vyarus.animalsniffer") version "2.0.1" id("com.squareup.wire") version "5.3.5" } @@ -20,27 +19,12 @@ dependencies { implementation("io.opentelemetry:opentelemetry-api-incubator") compileOnly("com.google.auto.value:auto-value-annotations") annotationProcessor("com.google.auto.value:auto-value") - signature("com.toasttab.android:gummy-bears-api-21:0.12.0:coreLib@signature") testImplementation("org.mockito:mockito-inline") testImplementation("io.opentelemetry:opentelemetry-sdk-testing") protos("io.opentelemetry.proto:opentelemetry-proto:1.7.0-alpha@jar") } -animalsniffer { - sourceSets = listOf(java.sourceSets.main.get()) -} - -// Always having declared output makes this task properly participate in tasks up-to-date checks -tasks.withType { - reports.text.required.set(true) -} - -// Attaching animalsniffer check to the compilation process. -tasks.named("classes").configure { - finalizedBy("animalsnifferMain") -} - jmh { warmupIterations.set(0) fork.set(2) From a710f38ca675496459e16c27f1ddce4453176ad6 Mon Sep 17 00:00:00 2001 From: Cesar Munoz <56847527+LikeTheSalad@users.noreply.github.com> Date: Wed, 30 Jul 2025 10:05:20 +0200 Subject: [PATCH 03/13] Applying animalsniffer conventions to opamp-client --- opamp-client/build.gradle.kts | 1 + 1 file changed, 1 insertion(+) diff --git a/opamp-client/build.gradle.kts b/opamp-client/build.gradle.kts index e41d1fff9..17fa3cab1 100644 --- a/opamp-client/build.gradle.kts +++ b/opamp-client/build.gradle.kts @@ -4,6 +4,7 @@ import java.net.URL plugins { id("otel.java-conventions") + id("otel.animalsniffer-conventions") id("de.undercouch.download") version "5.6.0" id("com.squareup.wire") version "5.3.5" } From cc57339358dcb1509708c0042ec1e870c62eca0c Mon Sep 17 00:00:00 2001 From: Cesar Munoz <56847527+LikeTheSalad@users.noreply.github.com> Date: Wed, 30 Jul 2025 10:50:26 +0200 Subject: [PATCH 04/13] Creating ExponentialBackoffPeriodicDelay --- .../ExponentialBackoffPeriodicDelay.java | 33 +++++++++++++++++++ .../ExponentialBackoffPeriodicDelayTest.java | 24 ++++++++++++++ 2 files changed, 57 insertions(+) create mode 100644 opamp-client/src/main/java/io/opentelemetry/opamp/client/internal/request/delay/ExponentialBackoffPeriodicDelay.java create mode 100644 opamp-client/src/test/java/io/opentelemetry/opamp/client/internal/request/delay/ExponentialBackoffPeriodicDelayTest.java diff --git a/opamp-client/src/main/java/io/opentelemetry/opamp/client/internal/request/delay/ExponentialBackoffPeriodicDelay.java b/opamp-client/src/main/java/io/opentelemetry/opamp/client/internal/request/delay/ExponentialBackoffPeriodicDelay.java new file mode 100644 index 000000000..0245f8e1b --- /dev/null +++ b/opamp-client/src/main/java/io/opentelemetry/opamp/client/internal/request/delay/ExponentialBackoffPeriodicDelay.java @@ -0,0 +1,33 @@ +package io.opentelemetry.opamp.client.internal.request.delay; + +import java.time.Duration; +import javax.annotation.concurrent.GuardedBy; + +public final class ExponentialBackoffPeriodicDelay implements PeriodicDelay { + private final Duration initialDelay; + private final Object delayNanosLock = new Object(); + + @GuardedBy("delayNanosLock") + private long delayNanos; + + public ExponentialBackoffPeriodicDelay(Duration initialDelay) { + this.initialDelay = initialDelay; + delayNanos = initialDelay.toNanos(); + } + + @Override + public Duration getNextDelay() { + synchronized (delayNanosLock) { + long previousValue = delayNanos; + delayNanos = delayNanos * 2; + return Duration.ofNanos(previousValue); + } + } + + @Override + public void reset() { + synchronized (delayNanosLock) { + delayNanos = initialDelay.toNanos(); + } + } +} diff --git a/opamp-client/src/test/java/io/opentelemetry/opamp/client/internal/request/delay/ExponentialBackoffPeriodicDelayTest.java b/opamp-client/src/test/java/io/opentelemetry/opamp/client/internal/request/delay/ExponentialBackoffPeriodicDelayTest.java new file mode 100644 index 000000000..03500814e --- /dev/null +++ b/opamp-client/src/test/java/io/opentelemetry/opamp/client/internal/request/delay/ExponentialBackoffPeriodicDelayTest.java @@ -0,0 +1,24 @@ +package io.opentelemetry.opamp.client.internal.request.delay; + +import static org.assertj.core.api.Assertions.assertThat; + +import java.time.Duration; +import org.junit.jupiter.api.Test; + +class ExponentialBackoffPeriodicDelayTest { + @Test + void verifyDelayUpdates() { + ExponentialBackoffPeriodicDelay delay = + new ExponentialBackoffPeriodicDelay(Duration.ofSeconds(1)); + + assertThat(delay.getNextDelay()).isEqualTo(Duration.ofSeconds(1)); + assertThat(delay.getNextDelay()).isEqualTo(Duration.ofSeconds(2)); + assertThat(delay.getNextDelay()).isEqualTo(Duration.ofSeconds(4)); + assertThat(delay.getNextDelay()).isEqualTo(Duration.ofSeconds(8)); + assertThat(delay.getNextDelay()).isEqualTo(Duration.ofSeconds(16)); + + // Reset + delay.reset(); + assertThat(delay.getNextDelay()).isEqualTo(Duration.ofSeconds(1)); + } +} From cf75d7e4f961041156cdc8abdd20c685fcdece8b Mon Sep 17 00:00:00 2001 From: Cesar Munoz <56847527+LikeTheSalad@users.noreply.github.com> Date: Wed, 30 Jul 2025 11:00:55 +0200 Subject: [PATCH 05/13] Creating RetryPeriodicDelay --- .../request/delay/RetryPeriodicDelay.java | 36 +++++++++++++++++++ .../request/delay/RetryPeriodicDelayTest.java | 26 ++++++++++++++ 2 files changed, 62 insertions(+) create mode 100644 opamp-client/src/main/java/io/opentelemetry/opamp/client/internal/request/delay/RetryPeriodicDelay.java create mode 100644 opamp-client/src/test/java/io/opentelemetry/opamp/client/internal/request/delay/RetryPeriodicDelayTest.java diff --git a/opamp-client/src/main/java/io/opentelemetry/opamp/client/internal/request/delay/RetryPeriodicDelay.java b/opamp-client/src/main/java/io/opentelemetry/opamp/client/internal/request/delay/RetryPeriodicDelay.java new file mode 100644 index 000000000..abfa5c8b5 --- /dev/null +++ b/opamp-client/src/main/java/io/opentelemetry/opamp/client/internal/request/delay/RetryPeriodicDelay.java @@ -0,0 +1,36 @@ +package io.opentelemetry.opamp.client.internal.request.delay; + +import java.time.Duration; +import java.util.Objects; +import java.util.concurrent.atomic.AtomicReference; + +/** Defaults to an exponential backoff strategy, unless a delay is suggested. */ +public final class RetryPeriodicDelay implements PeriodicDelay, AcceptsDelaySuggestion { + private final ExponentialBackoffPeriodicDelay exponentialBackoff; + private final AtomicReference currentDelay; + + public static RetryPeriodicDelay create(Duration initialDelay) { + return new RetryPeriodicDelay(new ExponentialBackoffPeriodicDelay(initialDelay)); + } + + private RetryPeriodicDelay(ExponentialBackoffPeriodicDelay exponentialBackoff) { + this.exponentialBackoff = exponentialBackoff; + currentDelay = new AtomicReference<>(exponentialBackoff); + } + + @Override + public void suggestDelay(Duration delay) { + currentDelay.set(PeriodicDelay.ofFixedDuration(delay)); + } + + @Override + public Duration getNextDelay() { + return Objects.requireNonNull(currentDelay.get()).getNextDelay(); + } + + @Override + public void reset() { + exponentialBackoff.reset(); + currentDelay.set(exponentialBackoff); + } +} diff --git a/opamp-client/src/test/java/io/opentelemetry/opamp/client/internal/request/delay/RetryPeriodicDelayTest.java b/opamp-client/src/test/java/io/opentelemetry/opamp/client/internal/request/delay/RetryPeriodicDelayTest.java new file mode 100644 index 000000000..4a007f48f --- /dev/null +++ b/opamp-client/src/test/java/io/opentelemetry/opamp/client/internal/request/delay/RetryPeriodicDelayTest.java @@ -0,0 +1,26 @@ +package io.opentelemetry.opamp.client.internal.request.delay; + +import static org.assertj.core.api.Assertions.assertThat; + +import java.time.Duration; +import org.junit.jupiter.api.Test; + +class RetryPeriodicDelayTest { + @Test + public void verifyDelayBehavior() { + RetryPeriodicDelay retryPeriodicDelay = RetryPeriodicDelay.create(Duration.ofSeconds(1)); + + // Without suggested delay + assertThat(retryPeriodicDelay.getNextDelay()).isEqualTo(Duration.ofSeconds(1)); + assertThat(retryPeriodicDelay.getNextDelay()).isEqualTo(Duration.ofSeconds(2)); + assertThat(retryPeriodicDelay.getNextDelay()).isEqualTo(Duration.ofSeconds(4)); + retryPeriodicDelay.reset(); + assertThat(retryPeriodicDelay.getNextDelay()).isEqualTo(Duration.ofSeconds(1)); + + // With suggested delay + retryPeriodicDelay.suggestDelay(Duration.ofSeconds(5)); + assertThat(retryPeriodicDelay.getNextDelay()).isEqualTo(Duration.ofSeconds(5)); + retryPeriodicDelay.reset(); + assertThat(retryPeriodicDelay.getNextDelay()).isEqualTo(Duration.ofSeconds(1)); + } +} From f11a73038bb319651f1960e0086aa11cf41d3a61 Mon Sep 17 00:00:00 2001 From: Cesar Munoz <56847527+LikeTheSalad@users.noreply.github.com> Date: Wed, 30 Jul 2025 11:03:04 +0200 Subject: [PATCH 06/13] Using retry delays --- .../client/internal/request/service/HttpRequestService.java | 5 ++++- .../internal/request/service/WebSocketRequestService.java | 3 ++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/opamp-client/src/main/java/io/opentelemetry/opamp/client/internal/request/service/HttpRequestService.java b/opamp-client/src/main/java/io/opentelemetry/opamp/client/internal/request/service/HttpRequestService.java index da1759c22..569447342 100644 --- a/opamp-client/src/main/java/io/opentelemetry/opamp/client/internal/request/service/HttpRequestService.java +++ b/opamp-client/src/main/java/io/opentelemetry/opamp/client/internal/request/service/HttpRequestService.java @@ -11,6 +11,7 @@ import io.opentelemetry.opamp.client.internal.request.Request; import io.opentelemetry.opamp.client.internal.request.delay.AcceptsDelaySuggestion; import io.opentelemetry.opamp.client.internal.request.delay.PeriodicDelay; +import io.opentelemetry.opamp.client.internal.request.delay.RetryPeriodicDelay; import io.opentelemetry.opamp.client.internal.response.OpampServerResponseException; import io.opentelemetry.opamp.client.internal.response.Response; import java.io.IOException; @@ -47,6 +48,8 @@ public final class HttpRequestService implements RequestService { @Nullable private Supplier requestSupplier; public static final PeriodicDelay DEFAULT_DELAY_BETWEEN_REQUESTS = PeriodicDelay.ofFixedDuration(Duration.ofSeconds(30)); + public static final PeriodicDelay DEFAULT_DELAY_BETWEEN_RETRIES = + RetryPeriodicDelay.create(Duration.ofSeconds(30)); /** * Creates an {@link HttpRequestService}. @@ -54,7 +57,7 @@ public final class HttpRequestService implements RequestService { * @param requestSender The HTTP sender implementation. */ public static HttpRequestService create(HttpSender requestSender) { - return create(requestSender, DEFAULT_DELAY_BETWEEN_REQUESTS, DEFAULT_DELAY_BETWEEN_REQUESTS); + return create(requestSender, DEFAULT_DELAY_BETWEEN_REQUESTS, DEFAULT_DELAY_BETWEEN_RETRIES); } /** diff --git a/opamp-client/src/main/java/io/opentelemetry/opamp/client/internal/request/service/WebSocketRequestService.java b/opamp-client/src/main/java/io/opentelemetry/opamp/client/internal/request/service/WebSocketRequestService.java index 2bd283631..13ef9b117 100644 --- a/opamp-client/src/main/java/io/opentelemetry/opamp/client/internal/request/service/WebSocketRequestService.java +++ b/opamp-client/src/main/java/io/opentelemetry/opamp/client/internal/request/service/WebSocketRequestService.java @@ -10,6 +10,7 @@ import io.opentelemetry.opamp.client.internal.request.Request; import io.opentelemetry.opamp.client.internal.request.delay.AcceptsDelaySuggestion; import io.opentelemetry.opamp.client.internal.request.delay.PeriodicDelay; +import io.opentelemetry.opamp.client.internal.request.delay.RetryPeriodicDelay; import io.opentelemetry.opamp.client.internal.response.OpampServerResponseException; import io.opentelemetry.opamp.client.internal.response.Response; import java.io.ByteArrayOutputStream; @@ -30,7 +31,7 @@ public final class WebSocketRequestService implements RequestService, WebSocket.Listener { private static final PeriodicDelay DEFAULT_DELAY_BETWEEN_RETRIES = - PeriodicDelay.ofFixedDuration(Duration.ofSeconds(30)); + RetryPeriodicDelay.create(Duration.ofSeconds(30)); private final WebSocket webSocket; private final AtomicBoolean isRunning = new AtomicBoolean(false); From 00d1ae1b2b28ea27765dd3e236ec1b0275e06aee Mon Sep 17 00:00:00 2001 From: Cesar Munoz <56847527+LikeTheSalad@users.noreply.github.com> Date: Wed, 30 Jul 2025 11:04:39 +0200 Subject: [PATCH 07/13] Adding publishing conventions to opamp-client --- opamp-client/build.gradle.kts | 1 + 1 file changed, 1 insertion(+) diff --git a/opamp-client/build.gradle.kts b/opamp-client/build.gradle.kts index 17fa3cab1..3ab1009b2 100644 --- a/opamp-client/build.gradle.kts +++ b/opamp-client/build.gradle.kts @@ -4,6 +4,7 @@ import java.net.URL plugins { id("otel.java-conventions") + id("otel.publish-conventions") id("otel.animalsniffer-conventions") id("de.undercouch.download") version "5.6.0" id("com.squareup.wire") version "5.3.5" From e7d76b09e97d7648abc7de477edbd9d90dc85259 Mon Sep 17 00:00:00 2001 From: Cesar Munoz <56847527+LikeTheSalad@users.noreply.github.com> Date: Wed, 30 Jul 2025 11:19:47 +0200 Subject: [PATCH 08/13] Updating README --- opamp-client/README.md | 50 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/opamp-client/README.md b/opamp-client/README.md index 9cb2dd97d..d8c0369a1 100644 --- a/opamp-client/README.md +++ b/opamp-client/README.md @@ -3,6 +3,56 @@ Java implementation of the OpAMP client [spec](https://github.com/open-telemetry/opamp-spec/blob/main/specification.md). +> [!WARNING] +> This is an incubating feature. Breaking changes can happen on a new release without previous +> notice and without backward compatibility guarantees. + +## Usage + +```java +// Initializing it + +RequestService requestService = HttpRequestService.create(OkHttpSender.create("[OPAMP_SERVICE_URL]")); +// RequestService requestService = WebSocketRequestService.create(OkHttpWebSocket.create("[OPAMP_SERVICE_URL]")); // Use this instead to connect to the server via WebSocket. +OpampClient client = + OpampClient.builder() + .putIdentifyingAttribute("service.name", "My service name") + .enableRemoteConfig() + .setRequestService(requestService) + .build( + new OpampClient.Callbacks() { + @Override + public void onConnect() {} + + @Override + public void onConnectFailed(@Nullable Throwable throwable) {} + + @Override + public void onErrorResponse(ServerErrorResponse errorResponse) {} + + @Override + public void onMessage(MessageData messageData) { + AgentRemoteConfig remoteConfig = messageData.getRemoteConfig(); + if (remoteConfig != null) { + // A remote config was received + + // After applying it... + client.setRemoteConfigStatus( + new RemoteConfigStatus.Builder() + .status(RemoteConfigStatuses.RemoteConfigStatuses_APPLIED) + .build()); + } + } + }); + +// State update +client.setAgentDescription(new AgentDescription.Builder().build()); + +// App shutdown +client.close(); + +``` + ## Component owners - [Cesar Munoz](https://github.com/LikeTheSalad), Elastic From 5e5fbeb5032f27d0a343e1e420316fa3fdba658d Mon Sep 17 00:00:00 2001 From: Cesar Munoz <56847527+LikeTheSalad@users.noreply.github.com> Date: Wed, 30 Jul 2025 11:28:39 +0200 Subject: [PATCH 09/13] Spotless --- .../request/delay/ExponentialBackoffPeriodicDelay.java | 5 +++++ .../client/internal/request/delay/RetryPeriodicDelay.java | 5 +++++ .../request/delay/ExponentialBackoffPeriodicDelayTest.java | 5 +++++ .../internal/request/delay/RetryPeriodicDelayTest.java | 5 +++++ 4 files changed, 20 insertions(+) diff --git a/opamp-client/src/main/java/io/opentelemetry/opamp/client/internal/request/delay/ExponentialBackoffPeriodicDelay.java b/opamp-client/src/main/java/io/opentelemetry/opamp/client/internal/request/delay/ExponentialBackoffPeriodicDelay.java index 0245f8e1b..88b608c7e 100644 --- a/opamp-client/src/main/java/io/opentelemetry/opamp/client/internal/request/delay/ExponentialBackoffPeriodicDelay.java +++ b/opamp-client/src/main/java/io/opentelemetry/opamp/client/internal/request/delay/ExponentialBackoffPeriodicDelay.java @@ -1,3 +1,8 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + package io.opentelemetry.opamp.client.internal.request.delay; import java.time.Duration; diff --git a/opamp-client/src/main/java/io/opentelemetry/opamp/client/internal/request/delay/RetryPeriodicDelay.java b/opamp-client/src/main/java/io/opentelemetry/opamp/client/internal/request/delay/RetryPeriodicDelay.java index abfa5c8b5..46f1a21da 100644 --- a/opamp-client/src/main/java/io/opentelemetry/opamp/client/internal/request/delay/RetryPeriodicDelay.java +++ b/opamp-client/src/main/java/io/opentelemetry/opamp/client/internal/request/delay/RetryPeriodicDelay.java @@ -1,3 +1,8 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + package io.opentelemetry.opamp.client.internal.request.delay; import java.time.Duration; diff --git a/opamp-client/src/test/java/io/opentelemetry/opamp/client/internal/request/delay/ExponentialBackoffPeriodicDelayTest.java b/opamp-client/src/test/java/io/opentelemetry/opamp/client/internal/request/delay/ExponentialBackoffPeriodicDelayTest.java index 03500814e..ff7a1e186 100644 --- a/opamp-client/src/test/java/io/opentelemetry/opamp/client/internal/request/delay/ExponentialBackoffPeriodicDelayTest.java +++ b/opamp-client/src/test/java/io/opentelemetry/opamp/client/internal/request/delay/ExponentialBackoffPeriodicDelayTest.java @@ -1,3 +1,8 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + package io.opentelemetry.opamp.client.internal.request.delay; import static org.assertj.core.api.Assertions.assertThat; diff --git a/opamp-client/src/test/java/io/opentelemetry/opamp/client/internal/request/delay/RetryPeriodicDelayTest.java b/opamp-client/src/test/java/io/opentelemetry/opamp/client/internal/request/delay/RetryPeriodicDelayTest.java index 4a007f48f..88bda4ec7 100644 --- a/opamp-client/src/test/java/io/opentelemetry/opamp/client/internal/request/delay/RetryPeriodicDelayTest.java +++ b/opamp-client/src/test/java/io/opentelemetry/opamp/client/internal/request/delay/RetryPeriodicDelayTest.java @@ -1,3 +1,8 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + package io.opentelemetry.opamp.client.internal.request.delay; import static org.assertj.core.api.Assertions.assertThat; From 70f85cd9306a4e76831b7f65001a6590ff54f890 Mon Sep 17 00:00:00 2001 From: Cesar Munoz <56847527+LikeTheSalad@users.noreply.github.com> Date: Mon, 4 Aug 2025 17:47:17 +0200 Subject: [PATCH 10/13] Suppressing auto instrumentation --- opamp-client/build.gradle.kts | 1 + .../internal/connectivity/http/OkHttpSender.java | 12 +++++++++--- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/opamp-client/build.gradle.kts b/opamp-client/build.gradle.kts index 3ab1009b2..d5d10937f 100644 --- a/opamp-client/build.gradle.kts +++ b/opamp-client/build.gradle.kts @@ -16,6 +16,7 @@ otelJava.moduleName.set("io.opentelemetry.contrib.opamp.client") dependencies { implementation("com.squareup.okhttp3:okhttp") implementation("com.github.f4b6a3:uuid-creator") + implementation("io.opentelemetry.instrumentation:opentelemetry-instrumentation-api") annotationProcessor("com.google.auto.value:auto-value") compileOnly("com.google.auto.value:auto-value-annotations") testImplementation("org.mockito:mockito-inline") diff --git a/opamp-client/src/main/java/io/opentelemetry/opamp/client/internal/connectivity/http/OkHttpSender.java b/opamp-client/src/main/java/io/opentelemetry/opamp/client/internal/connectivity/http/OkHttpSender.java index fcfc97ee8..eefa2741f 100644 --- a/opamp-client/src/main/java/io/opentelemetry/opamp/client/internal/connectivity/http/OkHttpSender.java +++ b/opamp-client/src/main/java/io/opentelemetry/opamp/client/internal/connectivity/http/OkHttpSender.java @@ -5,6 +5,7 @@ package io.opentelemetry.opamp.client.internal.connectivity.http; +import io.opentelemetry.api.internal.InstrumentationUtil; import java.io.IOException; import java.io.InputStream; import java.util.concurrent.CompletableFuture; @@ -13,6 +14,7 @@ import okhttp3.Callback; import okhttp3.MediaType; import okhttp3.OkHttpClient; +import okhttp3.Request; import okhttp3.RequestBody; import okio.BufferedSink; @@ -45,8 +47,14 @@ public CompletableFuture send(BodyWriter writer, int contentLength) { RequestBody body = new RawRequestBody(writer, contentLength, MEDIA_TYPE); builder.post(body); + InstrumentationUtil.suppressInstrumentation(() -> doSendRequest(builder.build(), future)); + + return future; + } + + private void doSendRequest(Request request, CompletableFuture future) { client - .newCall(builder.build()) + .newCall(request) .enqueue( new Callback() { @Override @@ -59,8 +67,6 @@ public void onFailure(Call call, IOException e) { future.completeExceptionally(e); } }); - - return future; } private static class OkHttpResponse implements Response { From 509e26f28d02e8fd8e3884aabe1a5a740191912d Mon Sep 17 00:00:00 2001 From: Cesar Munoz <56847527+LikeTheSalad@users.noreply.github.com> Date: Mon, 4 Aug 2025 17:51:09 +0200 Subject: [PATCH 11/13] Using otel api dependency --- opamp-client/build.gradle.kts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/opamp-client/build.gradle.kts b/opamp-client/build.gradle.kts index d5d10937f..58e2e5082 100644 --- a/opamp-client/build.gradle.kts +++ b/opamp-client/build.gradle.kts @@ -16,7 +16,7 @@ otelJava.moduleName.set("io.opentelemetry.contrib.opamp.client") dependencies { implementation("com.squareup.okhttp3:okhttp") implementation("com.github.f4b6a3:uuid-creator") - implementation("io.opentelemetry.instrumentation:opentelemetry-instrumentation-api") + implementation("io.opentelemetry:opentelemetry-api") annotationProcessor("com.google.auto.value:auto-value") compileOnly("com.google.auto.value:auto-value-annotations") testImplementation("org.mockito:mockito-inline") From 69b99776e034b64554c9f5ae45a16331053d00ac Mon Sep 17 00:00:00 2001 From: Cesar Munoz <56847527+LikeTheSalad@users.noreply.github.com> Date: Tue, 19 Aug 2025 10:00:23 +0200 Subject: [PATCH 12/13] Removing attaching animalsniffer check to compilation --- .../main/kotlin/otel.animalsniffer-conventions.gradle.kts | 5 ----- 1 file changed, 5 deletions(-) diff --git a/buildSrc/src/main/kotlin/otel.animalsniffer-conventions.gradle.kts b/buildSrc/src/main/kotlin/otel.animalsniffer-conventions.gradle.kts index 0e10e8b04..3fda84f4c 100644 --- a/buildSrc/src/main/kotlin/otel.animalsniffer-conventions.gradle.kts +++ b/buildSrc/src/main/kotlin/otel.animalsniffer-conventions.gradle.kts @@ -17,8 +17,3 @@ animalsniffer { tasks.withType { reports.text.required.set(true) } - -// Attaching animalsniffer check to the compilation process. -tasks.named("classes").configure { - finalizedBy("animalsnifferMain") -} From d9a6cba8063dbaf4763374c9b725dbef778b2895 Mon Sep 17 00:00:00 2001 From: Cesar Munoz <56847527+LikeTheSalad@users.noreply.github.com> Date: Tue, 19 Aug 2025 10:02:41 +0200 Subject: [PATCH 13/13] Adding comment for suppressing opamp-server request instrumentation --- .../opamp/client/internal/connectivity/http/OkHttpSender.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/opamp-client/src/main/java/io/opentelemetry/opamp/client/internal/connectivity/http/OkHttpSender.java b/opamp-client/src/main/java/io/opentelemetry/opamp/client/internal/connectivity/http/OkHttpSender.java index eefa2741f..1add016fd 100644 --- a/opamp-client/src/main/java/io/opentelemetry/opamp/client/internal/connectivity/http/OkHttpSender.java +++ b/opamp-client/src/main/java/io/opentelemetry/opamp/client/internal/connectivity/http/OkHttpSender.java @@ -47,6 +47,8 @@ public CompletableFuture send(BodyWriter writer, int contentLength) { RequestBody body = new RawRequestBody(writer, contentLength, MEDIA_TYPE); builder.post(body); + // By suppressing instrumentations, we prevent automatic instrumentations for the okhttp request + // that polls the opamp server. InstrumentationUtil.suppressInstrumentation(() -> doSendRequest(builder.build(), future)); return future;