diff --git a/opamp/src/main/java/com/splunk/opentelemetry/opamp/RemoteConfigProcessorImpl.java b/opamp/src/main/java/com/splunk/opentelemetry/opamp/RemoteConfigProcessorImpl.java index d4996a815..8392fcf9f 100644 --- a/opamp/src/main/java/com/splunk/opentelemetry/opamp/RemoteConfigProcessorImpl.java +++ b/opamp/src/main/java/com/splunk/opentelemetry/opamp/RemoteConfigProcessorImpl.java @@ -74,6 +74,11 @@ public void applyConfig(AgentRemoteConfig remoteConfig, OpampClient opampClient) } try { + reportRemoteConfigStatus( + remoteConfig.config_hash, + RemoteConfigStatuses.RemoteConfigStatuses_APPLYING, + opampClient); + DeclarativeConfigProperties remoteConfigProperties = toDeclarativeConfigProperties(configFile); DeclarativeConfigProperties distributionRemoteConfigProperties = @@ -89,18 +94,12 @@ public void applyConfig(AgentRemoteConfig remoteConfig, OpampClient opampClient) // Confirm to the OpAMP Server that remote config has been applied. reportRemoteConfigStatus( - remoteConfig.config_hash, - RemoteConfigStatuses.RemoteConfigStatuses_APPLIED, - "", - opampClient); + remoteConfig.config_hash, RemoteConfigStatuses.RemoteConfigStatuses_APPLIED, opampClient); } catch (Exception e) { logger.log(Level.WARNING, "Remote configuration not applied due to exception.", e); - reportRemoteConfigStatus( - remoteConfig.config_hash, - RemoteConfigStatuses.RemoteConfigStatuses_FAILED, - "Exception occurred: " + e.getMessage(), - opampClient); + reportRemoteConfigFailure( + remoteConfig.config_hash, "Exception occurred: " + e.getMessage(), opampClient); } // TODO: Maybe should be postponed after profiler is enabled/disabled? @@ -151,15 +150,21 @@ static DeclarativeConfigProperties toDeclarativeConfigProperties(AgentConfigFile } private void reportRemoteConfigStatus( - ByteString configHash, - RemoteConfigStatuses status, - String errorMessage, - OpampClient opampClient) { + ByteString configHash, RemoteConfigStatuses status, OpampClient opampClient) { opampClient.setRemoteConfigStatus( new RemoteConfigStatus.Builder() .last_remote_config_hash(configHash) - .error_message(errorMessage) .status(status) .build()); } + + private void reportRemoteConfigFailure( + ByteString configHash, String errorMessage, OpampClient opampClient) { + opampClient.setRemoteConfigStatus( + new RemoteConfigStatus.Builder() + .last_remote_config_hash(configHash) + .error_message(errorMessage) + .status(RemoteConfigStatuses.RemoteConfigStatuses_FAILED) + .build()); + } } diff --git a/opamp/src/test/java/com/splunk/opentelemetry/opamp/RemoteConfigProcessorImplTest.java b/opamp/src/test/java/com/splunk/opentelemetry/opamp/RemoteConfigProcessorImplTest.java index 69551afc6..f2c65f7bc 100644 --- a/opamp/src/test/java/com/splunk/opentelemetry/opamp/RemoteConfigProcessorImplTest.java +++ b/opamp/src/test/java/com/splunk/opentelemetry/opamp/RemoteConfigProcessorImplTest.java @@ -18,6 +18,7 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.Mockito.never; +import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyNoInteractions; import static org.mockito.Mockito.verifyNoMoreInteractions; @@ -28,6 +29,7 @@ import com.splunk.opentelemetry.profiler.snapshot.SnapshotProfilingConfiguration; import com.splunk.opentelemetry.profiler.snapshot.SnapshotProfilingSupervisor; import io.opentelemetry.opamp.client.OpampClient; +import java.util.List; import java.util.Map; import okio.ByteString; import opamp.proto.AgentConfigFile; @@ -68,6 +70,22 @@ void tearDown() { SnapshotProfilingConfiguration.SUPPLIER.reset(); } + @Test + void shouldMarkRemoteConfigAsApplyingWhenProcessingStarts() { + // given + ByteString configHash = ByteString.encodeUtf8("test-config-hash"); + AgentRemoteConfig remoteConfig = createRemoteConfig(configHash, "test-config:"); + + // when + handler.applyConfig(remoteConfig, opampClient); + + // then + RemoteConfigStatus status = getInitialReportedRemoteConfigStatus(); + assertThat(status.last_remote_config_hash).isEqualTo(configHash); + assertThat(status.status).isEqualTo(RemoteConfigStatuses.RemoteConfigStatuses_APPLYING); + assertThat(status.error_message).isEmpty(); + } + @Test void shouldMarkRemoteConfigAsAppliedWhenProfilingConfigIsNotProvided() { // given @@ -86,7 +104,7 @@ void shouldMarkRemoteConfigAsAppliedWhenProfilingConfigIsNotProvided() { handler.applyConfig(remoteConfig, opampClient); // then - RemoteConfigStatus status = getReportedRemoteConfigStatus(); + RemoteConfigStatus status = getFinalReportedRemoteConfigStatus(); assertThat(status.last_remote_config_hash).isEqualTo(configHash); assertThat(status.status).isEqualTo(RemoteConfigStatuses.RemoteConfigStatuses_APPLIED); assertThat(status.error_message).isEmpty(); @@ -112,7 +130,7 @@ void shouldReportErrorWhenRemoteConfigProcessingFailed() { handler.applyConfig(remoteConfig, opampClient); // then - RemoteConfigStatus status = getReportedRemoteConfigStatus(); + RemoteConfigStatus status = getFinalReportedRemoteConfigStatus(); assertThat(status.last_remote_config_hash).isEqualTo(configHash); assertThat(status.status).isEqualTo(RemoteConfigStatuses.RemoteConfigStatuses_FAILED); assertThat(status.error_message).startsWith("Exception occurred:"); @@ -162,7 +180,7 @@ void shouldStartProfilingWhenRemoteConfigEnablesProfiler() { handler.applyConfig(remoteConfig, opampClient); // then - RemoteConfigStatus status = getReportedRemoteConfigStatus(); + RemoteConfigStatus status = getFinalReportedRemoteConfigStatus(); assertThat(status.last_remote_config_hash).isEqualTo(configHash); assertThat(status.status).isEqualTo(RemoteConfigStatuses.RemoteConfigStatuses_APPLIED); assertThat(status.error_message).isEmpty(); @@ -224,7 +242,7 @@ void shouldStopProfilingWhenRemoteConfigDisablesProfiler() { handler.applyConfig(remoteConfig, opampClient); // then - RemoteConfigStatus status = getReportedRemoteConfigStatus(); + RemoteConfigStatus status = getFinalReportedRemoteConfigStatus(); assertThat(status.last_remote_config_hash).isEqualTo(configHash); assertThat(status.status).isEqualTo(RemoteConfigStatuses.RemoteConfigStatuses_APPLIED); assertThat(status.error_message).isEmpty(); @@ -255,7 +273,7 @@ void shouldStartProfilingWhenRemoteConfigEnablesProfiler() { handler.applyConfig(remoteConfig, opampClient); // then - RemoteConfigStatus status = getReportedRemoteConfigStatus(); + RemoteConfigStatus status = getFinalReportedRemoteConfigStatus(); assertThat(status.last_remote_config_hash).isEqualTo(configHash); assertThat(status.status).isEqualTo(RemoteConfigStatuses.RemoteConfigStatuses_APPLIED); assertThat(status.error_message).isEmpty(); @@ -308,7 +326,7 @@ void shouldStopProfilingWhenRemoteConfigDisablesProfiler() { handler.applyConfig(remoteConfig, opampClient); // then - RemoteConfigStatus status = getReportedRemoteConfigStatus(); + RemoteConfigStatus status = getFinalReportedRemoteConfigStatus(); assertThat(status.last_remote_config_hash).isEqualTo(configHash); assertThat(status.status).isEqualTo(RemoteConfigStatuses.RemoteConfigStatuses_APPLIED); assertThat(status.error_message).isEmpty(); @@ -320,11 +338,19 @@ void shouldStopProfilingWhenRemoteConfigDisablesProfiler() { } } - private RemoteConfigStatus getReportedRemoteConfigStatus() { + private RemoteConfigStatus getInitialReportedRemoteConfigStatus() { + return getReportedRemoteConfigStatuses().get(0); + } + + private RemoteConfigStatus getFinalReportedRemoteConfigStatus() { + return getReportedRemoteConfigStatuses().get(1); + } + + private List getReportedRemoteConfigStatuses() { ArgumentCaptor statusCaptor = ArgumentCaptor.forClass(RemoteConfigStatus.class); - verify(opampClient).setRemoteConfigStatus(statusCaptor.capture()); - return statusCaptor.getValue(); + verify(opampClient, times(2)).setRemoteConfigStatus(statusCaptor.capture()); + return statusCaptor.getAllValues(); } private static AgentRemoteConfig createRemoteConfig(ByteString configHash, String config) {