diff --git a/opamp/src/main/java/com/splunk/opentelemetry/opamp/DeclarativeConfigurationInterceptor.java b/opamp/src/main/java/com/splunk/opentelemetry/opamp/DeclarativeConfigurationInterceptor.java index b3a46a6e1..ffb3e452b 100644 --- a/opamp/src/main/java/com/splunk/opentelemetry/opamp/DeclarativeConfigurationInterceptor.java +++ b/opamp/src/main/java/com/splunk/opentelemetry/opamp/DeclarativeConfigurationInterceptor.java @@ -35,7 +35,7 @@ public static OpenTelemetryConfigurationModel getConfigurationModel() { } @VisibleForTesting - static void reset() { + public static void reset() { configurationModel = null; } diff --git a/opamp/src/main/java/com/splunk/opentelemetry/opamp/OpampActivator.java b/opamp/src/main/java/com/splunk/opentelemetry/opamp/OpampActivator.java index b7d4452ef..d98bd2a60 100644 --- a/opamp/src/main/java/com/splunk/opentelemetry/opamp/OpampActivator.java +++ b/opamp/src/main/java/com/splunk/opentelemetry/opamp/OpampActivator.java @@ -17,11 +17,12 @@ package com.splunk.opentelemetry.opamp; import static io.opentelemetry.opamp.client.internal.request.service.HttpRequestService.DEFAULT_DELAY_BETWEEN_RETRIES; -import static io.opentelemetry.sdk.autoconfigure.AutoConfigureUtil.getConfig; import static io.opentelemetry.sdk.autoconfigure.AutoConfigureUtil.getResource; import static java.util.logging.Level.WARNING; import com.google.auto.service.AutoService; +import com.splunk.opentelemetry.opamp.effectiveconfig.EffectiveConfigReporter; +import com.splunk.opentelemetry.opamp.effectiveconfig.UpdatableEffectiveConfigState; import com.splunk.opentelemetry.profiler.ProfilingSupervisor; import io.opentelemetry.javaagent.extension.AgentListener; import io.opentelemetry.opamp.client.OpampClient; @@ -31,7 +32,6 @@ import io.opentelemetry.opamp.client.internal.request.service.HttpRequestService; import io.opentelemetry.opamp.client.internal.response.MessageData; import io.opentelemetry.opamp.client.internal.state.State; -import io.opentelemetry.sdk.autoconfigure.AutoConfigureUtil; import io.opentelemetry.sdk.autoconfigure.AutoConfiguredOpenTelemetrySdk; import io.opentelemetry.sdk.resources.Resource; import java.io.IOException; @@ -55,18 +55,20 @@ public void afterAgent(AutoConfiguredOpenTelemetrySdk autoConfiguredOpenTelemetr } Resource resource = getResource(autoConfiguredOpenTelemetrySdk); - EffectiveConfigFactory effectiveConfigFactory = - createEffectiveConfigFactory(autoConfiguredOpenTelemetrySdk); - State.EffectiveConfig effectiveConfig = buildEffectiveConfig(effectiveConfigFactory); + UpdatableEffectiveConfigState effectiveConfigState = new UpdatableEffectiveConfigState(); + EffectiveConfigReporter effectiveConfigReporter = + EffectiveConfigReporter.create(autoConfiguredOpenTelemetrySdk, effectiveConfigState); + effectiveConfigReporter.reportEffectiveConfigIfChanged(); ServerToAgentMessageHandler serverToAgentMessageHandler = - new ServerToAgentMessageHandler(ProfilingSupervisor.SUPPLIER.get()); + new ServerToAgentMessageHandler( + ProfilingSupervisor.SUPPLIER.get(), effectiveConfigReporter); OpampClient client = startOpampClient( opampClientConfiguration, resource, - effectiveConfig, + effectiveConfigState, new OpampClient.Callbacks() { @Override public void onConnect(OpampClient opampClient) { @@ -108,13 +110,6 @@ public int order() { return Integer.MAX_VALUE; } - private EffectiveConfigFactory createEffectiveConfigFactory(AutoConfiguredOpenTelemetrySdk sdk) { - if (AutoConfigureUtil.isDeclarativeConfig(sdk)) { - return new DeclarativeEffectiveConfigFileFactory(); - } - return new EnvVarsEffectiveConfigFileFactory(getConfig(sdk)); - } - static OpampClient startOpampClient( OpampClientConfiguration opampClientConfiguration, Resource resource, @@ -146,15 +141,6 @@ static OpampClient startOpampClient( return builder.build(callbacks); } - static State.EffectiveConfig buildEffectiveConfig(EffectiveConfigFactory effectiveConfigFactory) { - return new State.EffectiveConfig() { - @Override - public opamp.proto.EffectiveConfig get() { - return new opamp.proto.EffectiveConfig(effectiveConfigFactory.createEffectiveConfigMap()); - } - }; - } - private static ComponentHealth createInitialHealthReport() { Instant now = Instant.now(); long nowNanos = now.getEpochSecond() * 1_000_000_000L + now.getNano(); diff --git a/opamp/src/main/java/com/splunk/opentelemetry/opamp/RemoteConfigProcessor.java b/opamp/src/main/java/com/splunk/opentelemetry/opamp/RemoteConfigProcessor.java index 92881a41e..96fa3bc82 100644 --- a/opamp/src/main/java/com/splunk/opentelemetry/opamp/RemoteConfigProcessor.java +++ b/opamp/src/main/java/com/splunk/opentelemetry/opamp/RemoteConfigProcessor.java @@ -19,6 +19,7 @@ import static io.opentelemetry.api.incubator.config.DeclarativeConfigProperties.empty; import com.google.common.annotations.VisibleForTesting; +import com.splunk.opentelemetry.opamp.effectiveconfig.EffectiveConfigReporter; import com.splunk.opentelemetry.profiler.ProfilerConfiguration; import com.splunk.opentelemetry.profiler.ProfilerDeclarativeConfigurationFactory; import com.splunk.opentelemetry.profiler.ProfilingSupervisor; @@ -28,7 +29,9 @@ import java.io.ByteArrayInputStream; import java.util.Map; import java.util.Objects; +import java.util.logging.Level; import java.util.logging.Logger; +import okio.ByteString; import opamp.proto.AgentConfigFile; import opamp.proto.AgentRemoteConfig; import opamp.proto.RemoteConfigStatus; @@ -40,10 +43,13 @@ public class RemoteConfigProcessor { private static final String REMOTE_CONFIG_FILE_NAME = "splunk.remote.config"; private static final String PROFILING_NODE_NAME = "profiling"; - public ProfilingSupervisor profilingSupervisor; + private final ProfilingSupervisor profilingSupervisor; + private final EffectiveConfigReporter effectiveConfigReporter; - public RemoteConfigProcessor(ProfilingSupervisor profilingSupervisor) { + public RemoteConfigProcessor( + ProfilingSupervisor profilingSupervisor, EffectiveConfigReporter effectiveConfigReporter) { this.profilingSupervisor = Objects.requireNonNull(profilingSupervisor); + this.effectiveConfigReporter = Objects.requireNonNull(effectiveConfigReporter); } public void applyConfig(AgentRemoteConfig remoteConfig, OpampClient opampClient) { @@ -60,34 +66,51 @@ public void applyConfig(AgentRemoteConfig remoteConfig, OpampClient opampClient) return; } - DeclarativeConfigProperties remoteConfigProperties = toDeclarativeConfigProperties(configFile); - DeclarativeConfigProperties splunkDistributionConfigProperties = - remoteConfigProperties - .getStructured("distribution", empty()) - .getStructured("splunk", empty()); - - // Update profiler configuration only when profiling node exists - if (splunkDistributionConfigProperties.getPropertyKeys().contains(PROFILING_NODE_NAME)) { - DeclarativeConfigProperties profilingConfigProperties = - splunkDistributionConfigProperties.getStructured(PROFILING_NODE_NAME, empty()); - ProfilerConfiguration profilingConfig = - ProfilerDeclarativeConfigurationFactory.create(profilingConfigProperties); - // TODO: should be merged with current profiling config. Probably we will need profiler - // configuration refactoring and some listeners implemented for profiler configuration - // changes. For POC use this temporary solution - if (profilingConfig.isEnabled()) { - profilingSupervisor.requestStartProfiling(); - } else { - profilingSupervisor.requestStopProfiling(); + try { + DeclarativeConfigProperties remoteConfigProperties = + toDeclarativeConfigProperties(configFile); + DeclarativeConfigProperties distributionRemoteConfigProperties = + remoteConfigProperties + .getStructured("distribution", empty()) + .getStructured("splunk", empty()); + + // Update profiler configuration only when profiling node exists + if (distributionRemoteConfigProperties.getPropertyKeys().contains(PROFILING_NODE_NAME)) { + ProfilerConfiguration receivedProfilerConfig = + ProfilerDeclarativeConfigurationFactory.create( + distributionRemoteConfigProperties.getStructured(PROFILING_NODE_NAME, empty())); + + ProfilerConfiguration currentProfilerConfiguration = ProfilerConfiguration.SUPPLIER.get(); + ProfilerConfiguration updatedProfilerConfig = + currentProfilerConfiguration.toBuilder() + .setEnabled(receivedProfilerConfig.isEnabled()) + .setCallStackInterval(receivedProfilerConfig.getCallStackInterval()) + .build(); + + if (!currentProfilerConfiguration.equals(updatedProfilerConfig)) { + ProfilerConfiguration.SUPPLIER.configure(updatedProfilerConfig); + profilingSupervisor.requestReinitializeProfiling(); + } } + + // Confirm to the OpAMP Server that remote config has been applied. + reportRemoteConfigStatus( + remoteConfig.config_hash, + RemoteConfigStatuses.RemoteConfigStatuses_APPLIED, + "", + opampClient); + + } catch (Exception e) { + logger.log(Level.WARNING, "Remote config not applied due to exception", e); + reportRemoteConfigStatus( + remoteConfig.config_hash, + RemoteConfigStatuses.RemoteConfigStatuses_FAILED, + "Exception occurred: " + e.getMessage(), + opampClient); } - // Confirm to the OpAMP Server that remote config has been applied. - opampClient.setRemoteConfigStatus( - new RemoteConfigStatus.Builder() - .last_remote_config_hash(remoteConfig.config_hash) - .status(RemoteConfigStatuses.RemoteConfigStatuses_APPLIED) - .build()); + // TODO: Maybe should be postponed after profiler is enabled/disabled? + effectiveConfigReporter.reportEffectiveConfigIfChanged(); } @VisibleForTesting @@ -95,4 +118,17 @@ static DeclarativeConfigProperties toDeclarativeConfigProperties(AgentConfigFile return DeclarativeConfiguration.toConfigProperties( new ByteArrayInputStream(configFile.body.toByteArray())); } + + private void reportRemoteConfigStatus( + ByteString configHash, + RemoteConfigStatuses status, + String errorMessage, + OpampClient opampClient) { + opampClient.setRemoteConfigStatus( + new RemoteConfigStatus.Builder() + .last_remote_config_hash(configHash) + .error_message(errorMessage) + .status(status) + .build()); + } } diff --git a/opamp/src/main/java/com/splunk/opentelemetry/opamp/ServerToAgentMessageHandler.java b/opamp/src/main/java/com/splunk/opentelemetry/opamp/ServerToAgentMessageHandler.java index f0ab02d36..30d938135 100644 --- a/opamp/src/main/java/com/splunk/opentelemetry/opamp/ServerToAgentMessageHandler.java +++ b/opamp/src/main/java/com/splunk/opentelemetry/opamp/ServerToAgentMessageHandler.java @@ -17,6 +17,7 @@ package com.splunk.opentelemetry.opamp; import com.google.common.annotations.VisibleForTesting; +import com.splunk.opentelemetry.opamp.effectiveconfig.EffectiveConfigReporter; import com.splunk.opentelemetry.profiler.ProfilingSupervisor; import io.opentelemetry.opamp.client.OpampClient; import io.opentelemetry.opamp.client.internal.response.MessageData; @@ -24,8 +25,9 @@ public class ServerToAgentMessageHandler { private final RemoteConfigProcessor remoteConfigProcessor; - public ServerToAgentMessageHandler(ProfilingSupervisor profilingSupervisor) { - this(new RemoteConfigProcessor(profilingSupervisor)); + public ServerToAgentMessageHandler( + ProfilingSupervisor profilingSupervisor, EffectiveConfigReporter effectiveConfigReporter) { + this(new RemoteConfigProcessor(profilingSupervisor, effectiveConfigReporter)); } @VisibleForTesting diff --git a/opamp/src/main/java/com/splunk/opentelemetry/opamp/DeclarativeEffectiveConfigFileFactory.java b/opamp/src/main/java/com/splunk/opentelemetry/opamp/effectiveconfig/DeclarativeEffectiveConfigFileFactory.java similarity index 98% rename from opamp/src/main/java/com/splunk/opentelemetry/opamp/DeclarativeEffectiveConfigFileFactory.java rename to opamp/src/main/java/com/splunk/opentelemetry/opamp/effectiveconfig/DeclarativeEffectiveConfigFileFactory.java index 7ab53d8af..7fe5d6ebe 100644 --- a/opamp/src/main/java/com/splunk/opentelemetry/opamp/DeclarativeEffectiveConfigFileFactory.java +++ b/opamp/src/main/java/com/splunk/opentelemetry/opamp/effectiveconfig/DeclarativeEffectiveConfigFileFactory.java @@ -14,9 +14,10 @@ * limitations under the License. */ -package com.splunk.opentelemetry.opamp; +package com.splunk.opentelemetry.opamp.effectiveconfig; import com.google.common.annotations.VisibleForTesting; +import com.splunk.opentelemetry.opamp.DeclarativeConfigurationInterceptor; import com.splunk.opentelemetry.profiler.ProfilerConfiguration; import com.splunk.opentelemetry.profiler.snapshot.SnapshotProfilingConfiguration; import com.splunk.opentelemetry.profiler.snapshot.SnapshotProfilingDeclarativeConfiguration; diff --git a/opamp/src/main/java/com/splunk/opentelemetry/opamp/EffectiveConfigBuilder.java b/opamp/src/main/java/com/splunk/opentelemetry/opamp/effectiveconfig/EffectiveConfigBuilder.java similarity index 95% rename from opamp/src/main/java/com/splunk/opentelemetry/opamp/EffectiveConfigBuilder.java rename to opamp/src/main/java/com/splunk/opentelemetry/opamp/effectiveconfig/EffectiveConfigBuilder.java index 08ab87bbb..8bfa48a54 100644 --- a/opamp/src/main/java/com/splunk/opentelemetry/opamp/EffectiveConfigBuilder.java +++ b/opamp/src/main/java/com/splunk/opentelemetry/opamp/effectiveconfig/EffectiveConfigBuilder.java @@ -14,7 +14,7 @@ * limitations under the License. */ -package com.splunk.opentelemetry.opamp; +package com.splunk.opentelemetry.opamp.effectiveconfig; import java.time.Duration; diff --git a/opamp/src/main/java/com/splunk/opentelemetry/opamp/EffectiveConfigFactory.java b/opamp/src/main/java/com/splunk/opentelemetry/opamp/effectiveconfig/EffectiveConfigFactory.java similarity index 97% rename from opamp/src/main/java/com/splunk/opentelemetry/opamp/EffectiveConfigFactory.java rename to opamp/src/main/java/com/splunk/opentelemetry/opamp/effectiveconfig/EffectiveConfigFactory.java index 13c454783..a68a7466f 100644 --- a/opamp/src/main/java/com/splunk/opentelemetry/opamp/EffectiveConfigFactory.java +++ b/opamp/src/main/java/com/splunk/opentelemetry/opamp/effectiveconfig/EffectiveConfigFactory.java @@ -14,7 +14,7 @@ * limitations under the License. */ -package com.splunk.opentelemetry.opamp; +package com.splunk.opentelemetry.opamp.effectiveconfig; import static java.nio.charset.StandardCharsets.UTF_8; diff --git a/opamp/src/main/java/com/splunk/opentelemetry/opamp/effectiveconfig/EffectiveConfigReporter.java b/opamp/src/main/java/com/splunk/opentelemetry/opamp/effectiveconfig/EffectiveConfigReporter.java new file mode 100644 index 000000000..587021bd9 --- /dev/null +++ b/opamp/src/main/java/com/splunk/opentelemetry/opamp/effectiveconfig/EffectiveConfigReporter.java @@ -0,0 +1,75 @@ +/* + * Copyright Splunk Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.splunk.opentelemetry.opamp.effectiveconfig; + +import static io.opentelemetry.sdk.autoconfigure.AutoConfigureUtil.getConfig; +import static java.nio.charset.StandardCharsets.UTF_8; + +import com.google.common.annotations.VisibleForTesting; +import io.opentelemetry.sdk.autoconfigure.AutoConfigureUtil; +import io.opentelemetry.sdk.autoconfigure.AutoConfiguredOpenTelemetrySdk; +import java.util.HashMap; +import java.util.Map; +import okio.ByteString; +import opamp.proto.AgentConfigFile; +import opamp.proto.AgentConfigMap; + +public class EffectiveConfigReporter { + private final UpdatableEffectiveConfigState effectiveConfigState; + private final EffectiveConfigFactory effectiveConfigFactory; + private String lastReportedConfigContent; + + @VisibleForTesting + EffectiveConfigReporter( + EffectiveConfigFactory effectiveConfigFactory, + UpdatableEffectiveConfigState effectiveConfigState) { + this.effectiveConfigFactory = effectiveConfigFactory; + this.effectiveConfigState = effectiveConfigState; + } + + public static EffectiveConfigReporter create( + AutoConfiguredOpenTelemetrySdk sdk, UpdatableEffectiveConfigState effectiveConfigState) { + return new EffectiveConfigReporter(createEffectiveConfigFactory(sdk), effectiveConfigState); + } + + public boolean reportEffectiveConfigIfChanged() { + // Detect if effectiveConfig changed and needs to be reported + String configContent = effectiveConfigFactory.createEffectiveConfigContent(); + if (configContent.equals(lastReportedConfigContent)) { + return false; + } + + Map configMap = new HashMap<>(); + AgentConfigFile configFile = + new AgentConfigFile( + new ByteString(configContent.getBytes(UTF_8)), effectiveConfigFactory.getContentType()); + configMap.put(effectiveConfigFactory.getFileName(), configFile); + + effectiveConfigState.set(new AgentConfigMap(configMap)); + lastReportedConfigContent = configContent; + + return true; + } + + private static EffectiveConfigFactory createEffectiveConfigFactory( + AutoConfiguredOpenTelemetrySdk sdk) { + if (AutoConfigureUtil.isDeclarativeConfig(sdk)) { + return new DeclarativeEffectiveConfigFileFactory(); + } + return new EnvVarsEffectiveConfigFileFactory(getConfig(sdk)); + } +} diff --git a/opamp/src/main/java/com/splunk/opentelemetry/opamp/EnvVarsEffectiveConfigFileFactory.java b/opamp/src/main/java/com/splunk/opentelemetry/opamp/effectiveconfig/EnvVarsEffectiveConfigFileFactory.java similarity index 95% rename from opamp/src/main/java/com/splunk/opentelemetry/opamp/EnvVarsEffectiveConfigFileFactory.java rename to opamp/src/main/java/com/splunk/opentelemetry/opamp/effectiveconfig/EnvVarsEffectiveConfigFileFactory.java index 021a5ae1a..687fda85a 100644 --- a/opamp/src/main/java/com/splunk/opentelemetry/opamp/EnvVarsEffectiveConfigFileFactory.java +++ b/opamp/src/main/java/com/splunk/opentelemetry/opamp/effectiveconfig/EnvVarsEffectiveConfigFileFactory.java @@ -14,10 +14,9 @@ * limitations under the License. */ -package com.splunk.opentelemetry.opamp; +package com.splunk.opentelemetry.opamp.effectiveconfig; import com.splunk.opentelemetry.profiler.ProfilerConfiguration; -import com.splunk.opentelemetry.profiler.ProfilerEnvVarsConfigurationFactory; import com.splunk.opentelemetry.profiler.snapshot.SnapshotProfilingConfiguration; import com.splunk.opentelemetry.profiler.snapshot.SnapshotProfilingEnvVarsConfiguration; import io.opentelemetry.sdk.autoconfigure.spi.ConfigProperties; @@ -49,8 +48,7 @@ public String createEffectiveConfigContent() { } private void addSplunkEnvVars(EffectiveConfigBuilder builder) { - ProfilerConfiguration profilerConfiguration = - ProfilerEnvVarsConfigurationFactory.create(config); + ProfilerConfiguration profilerConfiguration = ProfilerConfiguration.SUPPLIER.get(); SnapshotProfilingConfiguration snapshotConfiguration = new SnapshotProfilingEnvVarsConfiguration(config); diff --git a/opamp/src/main/java/com/splunk/opentelemetry/opamp/effectiveconfig/UpdatableEffectiveConfigState.java b/opamp/src/main/java/com/splunk/opentelemetry/opamp/effectiveconfig/UpdatableEffectiveConfigState.java new file mode 100644 index 000000000..242109ab5 --- /dev/null +++ b/opamp/src/main/java/com/splunk/opentelemetry/opamp/effectiveconfig/UpdatableEffectiveConfigState.java @@ -0,0 +1,34 @@ +/* + * Copyright Splunk Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.splunk.opentelemetry.opamp.effectiveconfig; + +import io.opentelemetry.opamp.client.internal.state.State; +import opamp.proto.AgentConfigMap; + +public class UpdatableEffectiveConfigState extends State.EffectiveConfig { + private AgentConfigMap agentConfigMap; + + public void set(AgentConfigMap configMap) { + agentConfigMap = configMap; + notifyUpdate(); + } + + @Override + public opamp.proto.EffectiveConfig get() { + return new opamp.proto.EffectiveConfig(agentConfigMap); + } +} diff --git a/opamp/src/main/java/com/splunk/opentelemetry/opamp/YamlNodeBuilder.java b/opamp/src/main/java/com/splunk/opentelemetry/opamp/effectiveconfig/YamlNodeBuilder.java similarity index 99% rename from opamp/src/main/java/com/splunk/opentelemetry/opamp/YamlNodeBuilder.java rename to opamp/src/main/java/com/splunk/opentelemetry/opamp/effectiveconfig/YamlNodeBuilder.java index 1bd57418d..e38bfe946 100644 --- a/opamp/src/main/java/com/splunk/opentelemetry/opamp/YamlNodeBuilder.java +++ b/opamp/src/main/java/com/splunk/opentelemetry/opamp/effectiveconfig/YamlNodeBuilder.java @@ -14,7 +14,7 @@ * limitations under the License. */ -package com.splunk.opentelemetry.opamp; +package com.splunk.opentelemetry.opamp.effectiveconfig; import java.util.LinkedHashMap; import java.util.Map; diff --git a/opamp/src/test/java/com/splunk/opentelemetry/opamp/OpampActivatorTest.java b/opamp/src/test/java/com/splunk/opentelemetry/opamp/OpampActivatorTest.java index ca85b97da..689590a19 100644 --- a/opamp/src/test/java/com/splunk/opentelemetry/opamp/OpampActivatorTest.java +++ b/opamp/src/test/java/com/splunk/opentelemetry/opamp/OpampActivatorTest.java @@ -16,7 +16,6 @@ package com.splunk.opentelemetry.opamp; -import static com.splunk.opentelemetry.opamp.OpampActivator.buildEffectiveConfig; import static io.opentelemetry.api.common.AttributeKey.booleanKey; import static io.opentelemetry.api.common.AttributeKey.doubleKey; import static io.opentelemetry.api.common.AttributeKey.longKey; @@ -29,16 +28,15 @@ import static io.opentelemetry.semconv.incubating.OsIncubatingAttributes.OS_TYPE; import static io.opentelemetry.semconv.incubating.OsIncubatingAttributes.OS_VERSION; import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.mock; +import com.splunk.opentelemetry.opamp.effectiveconfig.UpdatableEffectiveConfigState; import io.opentelemetry.api.common.AttributeKey; import io.opentelemetry.api.common.Attributes; import io.opentelemetry.api.common.Value; import io.opentelemetry.instrumentation.testing.internal.AutoCleanupExtension; import io.opentelemetry.opamp.client.OpampClient; import io.opentelemetry.opamp.client.internal.response.MessageData; -import io.opentelemetry.opamp.client.internal.state.State; -import io.opentelemetry.sdk.autoconfigure.spi.ConfigProperties; -import io.opentelemetry.sdk.autoconfigure.spi.internal.DefaultConfigProperties; import io.opentelemetry.sdk.resources.Resource; import io.opentelemetry.testing.internal.armeria.common.HttpResponse; import io.opentelemetry.testing.internal.armeria.common.HttpStatus; @@ -133,9 +131,7 @@ void testOpamp() throws Exception { .build(); server.enqueue(HttpResponse.of(HttpStatus.OK, MediaType.X_PROTOBUF, response.encode())); - ConfigProperties config = DefaultConfigProperties.createFromMap(Map.of()); - State.EffectiveConfig effectiveConfig = - buildEffectiveConfig(new EnvVarsEffectiveConfigFileFactory(config)); + UpdatableEffectiveConfigState effectiveConfig = mock(); CompletableFuture result = new CompletableFuture<>(); OpampClientConfiguration opampClientConfiguration = diff --git a/opamp/src/test/java/com/splunk/opentelemetry/opamp/RemoteConfigProcessorTest.java b/opamp/src/test/java/com/splunk/opentelemetry/opamp/RemoteConfigProcessorTest.java index 5f134fd19..5ad66018c 100644 --- a/opamp/src/test/java/com/splunk/opentelemetry/opamp/RemoteConfigProcessorTest.java +++ b/opamp/src/test/java/com/splunk/opentelemetry/opamp/RemoteConfigProcessorTest.java @@ -17,10 +17,14 @@ package com.splunk.opentelemetry.opamp; import static org.assertj.core.api.Assertions.assertThat; -import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoInteractions; +import static org.mockito.Mockito.verifyNoMoreInteractions; +import com.splunk.opentelemetry.opamp.effectiveconfig.EffectiveConfigReporter; +import com.splunk.opentelemetry.profiler.ProfilerConfiguration; +import com.splunk.opentelemetry.profiler.ProfilingSupervisor; import io.opentelemetry.opamp.client.OpampClient; import java.util.Map; import okio.ByteString; @@ -29,15 +33,34 @@ import opamp.proto.AgentRemoteConfig; import opamp.proto.RemoteConfigStatus; import opamp.proto.RemoteConfigStatuses; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.ArgumentCaptor; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; +@ExtendWith(MockitoExtension.class) class RemoteConfigProcessorTest { - private final RemoteConfigProcessor handler = new RemoteConfigProcessor(mock()); - private final OpampClient opampClient = mock(OpampClient.class); + @Mock ProfilingSupervisor profilingSupervisor; + @Mock EffectiveConfigReporter effectiveConfigReporter; + @Mock OpampClient opampClient; + private RemoteConfigProcessor handler; + + @BeforeEach + void setUp() { + ProfilerConfiguration.SUPPLIER.configure(ProfilerConfiguration.builder().build()); + handler = new RemoteConfigProcessor(profilingSupervisor, effectiveConfigReporter); + } + + @AfterEach + void tearDown() { + ProfilerConfiguration.SUPPLIER.reset(); + } @Test - void shouldMarkRemoteConfigAsApplied() { + void shouldMarkRemoteConfigAsAppliedWhenProfilingConfigIsNotProvided() { // given String remoteConfigYaml = "test-config:"; ByteString configHash = ByteString.encodeUtf8("test-config-hash"); @@ -54,12 +77,127 @@ void shouldMarkRemoteConfigAsApplied() { handler.applyConfig(remoteConfig, opampClient); // then - ArgumentCaptor statusCaptor = - ArgumentCaptor.forClass(RemoteConfigStatus.class); - verify(opampClient).setRemoteConfigStatus(statusCaptor.capture()); - RemoteConfigStatus status = statusCaptor.getValue(); + RemoteConfigStatus status = getReportedRemoteConfigStatus(); + assertThat(status.last_remote_config_hash).isEqualTo(configHash); + assertThat(status.status).isEqualTo(RemoteConfigStatuses.RemoteConfigStatuses_APPLIED); + assertThat(status.error_message).isEmpty(); + assertThat(ProfilerConfiguration.SUPPLIER.get().isEnabled()).isFalse(); + verify(effectiveConfigReporter).reportEffectiveConfigIfChanged(); + verifyNoInteractions(profilingSupervisor); + } + + @Test + void shouldStartProfilingWhenRemoteConfigEnablesProfiler() { + // given + String remoteConfigYaml = + """ + distribution: + splunk: + profiling: + always_on: + cpu_profiler: + """; + ByteString configHash = ByteString.encodeUtf8("test-config-hash"); + AgentRemoteConfig remoteConfig = createRemoteConfig(configHash, remoteConfigYaml); + + // when + handler.applyConfig(remoteConfig, opampClient); + + // then + RemoteConfigStatus status = getReportedRemoteConfigStatus(); + assertThat(status.last_remote_config_hash).isEqualTo(configHash); + assertThat(status.status).isEqualTo(RemoteConfigStatuses.RemoteConfigStatuses_APPLIED); + assertThat(status.error_message).isEmpty(); + assertThat(ProfilerConfiguration.SUPPLIER.get().isEnabled()).isTrue(); + verify(profilingSupervisor).requestReinitializeProfiling(); + verifyNoMoreInteractions(profilingSupervisor); + verify(effectiveConfigReporter).reportEffectiveConfigIfChanged(); + } + + @Test + void shouldReportErrorWhenRemoteConfigProcessingFailed() { + // given + String remoteConfigYaml = + """ + distribution: + splunk: + profiling: + INVALID YAML HERE + """; + ByteString configHash = ByteString.encodeUtf8("test-config-hash"); + AgentRemoteConfig remoteConfig = createRemoteConfig(configHash, remoteConfigYaml); + + // when + handler.applyConfig(remoteConfig, opampClient); + + // then + RemoteConfigStatus status = getReportedRemoteConfigStatus(); + assertThat(status.last_remote_config_hash).isEqualTo(configHash); + assertThat(status.status).isEqualTo(RemoteConfigStatuses.RemoteConfigStatuses_FAILED); + assertThat(status.error_message).startsWith("Exception occurred:"); + assertThat(ProfilerConfiguration.SUPPLIER.get().isEnabled()).isFalse(); + assertThat(ProfilerConfiguration.SUPPLIER.get().getCallStackInterval().toMillis()) + .isEqualTo(10000); + verifyNoInteractions(profilingSupervisor); + verify(effectiveConfigReporter).reportEffectiveConfigIfChanged(); + } + + @Test + void shouldUpdateProfilerConfigWhileRunning() { + // given + ProfilerConfiguration.SUPPLIER.configure( + ProfilerConfiguration.builder().setEnabled(true).build()); + + String remoteConfigYaml = + """ + distribution: + splunk: + profiling: + always_on: + cpu_profiler: + sampling_interval: 123 + """; + ByteString configHash = ByteString.encodeUtf8("test-config-hash"); + AgentRemoteConfig remoteConfig = createRemoteConfig(configHash, remoteConfigYaml); + + // when + handler.applyConfig(remoteConfig, opampClient); + + // then + assertThat(ProfilerConfiguration.SUPPLIER.get().isEnabled()).isTrue(); + assertThat(ProfilerConfiguration.SUPPLIER.get().getCallStackInterval().toMillis()) + .isEqualTo(123); + verify(profilingSupervisor).requestReinitializeProfiling(); + verifyNoMoreInteractions(profilingSupervisor); + verify(effectiveConfigReporter).reportEffectiveConfigIfChanged(); + } + + @Test + void shouldStopProfilingWhenRemoteConfigDisablesProfiler() { + // given + ProfilerConfiguration.SUPPLIER.configure( + ProfilerConfiguration.builder().setEnabled(true).build()); + String remoteConfigYaml = + """ + distribution: + splunk: + profiling: + """; + ByteString configHash = ByteString.encodeUtf8("test-config-hash"); + AgentRemoteConfig remoteConfig = createRemoteConfig(configHash, remoteConfigYaml); + + // when + handler.applyConfig(remoteConfig, opampClient); + + // then + RemoteConfigStatus status = getReportedRemoteConfigStatus(); assertThat(status.last_remote_config_hash).isEqualTo(configHash); assertThat(status.status).isEqualTo(RemoteConfigStatuses.RemoteConfigStatuses_APPLIED); + assertThat(status.error_message).isEmpty(); + assertThat(ProfilerConfiguration.SUPPLIER.get().isEnabled()).isFalse(); + verify(profilingSupervisor).requestReinitializeProfiling(); + verify(profilingSupervisor, never()).requestStartProfiling(); + verify(effectiveConfigReporter).reportEffectiveConfigIfChanged(); } @Test @@ -76,7 +214,24 @@ void shouldIgnoreRemoteConfigWithoutExpectedConfigFile() { handler.applyConfig(remoteConfig, opampClient); // then - verify(opampClient, never()).setRemoteConfigStatus(org.mockito.ArgumentMatchers.any()); + assertThat(ProfilerConfiguration.SUPPLIER.get().isEnabled()).isFalse(); + verifyNoInteractions(opampClient, profilingSupervisor); + verify(effectiveConfigReporter, never()).reportEffectiveConfigIfChanged(); + } + + private RemoteConfigStatus getReportedRemoteConfigStatus() { + ArgumentCaptor statusCaptor = + ArgumentCaptor.forClass(RemoteConfigStatus.class); + verify(opampClient).setRemoteConfigStatus(statusCaptor.capture()); + return statusCaptor.getValue(); + } + + private static AgentRemoteConfig createRemoteConfig(ByteString configHash, String config) { + return createRemoteConfig( + configHash, + Map.of( + "splunk.remote.config", + new AgentConfigFile.Builder().body(ByteString.encodeUtf8(config)).build())); } private static AgentRemoteConfig createRemoteConfig( diff --git a/opamp/src/test/java/com/splunk/opentelemetry/opamp/DeclarativeEffectiveConfigFileFactoryTest.java b/opamp/src/test/java/com/splunk/opentelemetry/opamp/effectiveconfig/DeclarativeEffectiveConfigFileFactoryTest.java similarity index 99% rename from opamp/src/test/java/com/splunk/opentelemetry/opamp/DeclarativeEffectiveConfigFileFactoryTest.java rename to opamp/src/test/java/com/splunk/opentelemetry/opamp/effectiveconfig/DeclarativeEffectiveConfigFileFactoryTest.java index 6e0c44e88..091c6bf7a 100644 --- a/opamp/src/test/java/com/splunk/opentelemetry/opamp/DeclarativeEffectiveConfigFileFactoryTest.java +++ b/opamp/src/test/java/com/splunk/opentelemetry/opamp/effectiveconfig/DeclarativeEffectiveConfigFileFactoryTest.java @@ -14,7 +14,7 @@ * limitations under the License. */ -package com.splunk.opentelemetry.opamp; +package com.splunk.opentelemetry.opamp.effectiveconfig; import static io.opentelemetry.api.incubator.config.DeclarativeConfigProperties.empty; import static io.opentelemetry.sdk.autoconfigure.AutoConfigureUtil.getDistributionConfig; @@ -22,6 +22,7 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.Mockito.when; +import com.splunk.opentelemetry.opamp.DeclarativeConfigurationInterceptor; import com.splunk.opentelemetry.profiler.ProfilerConfiguration; import com.splunk.opentelemetry.profiler.ProfilerDeclarativeConfigurationFactory; import com.splunk.opentelemetry.profiler.snapshot.SnapshotProfilingConfiguration; diff --git a/opamp/src/test/java/com/splunk/opentelemetry/opamp/effectiveconfig/EffectiveConfigReporterTest.java b/opamp/src/test/java/com/splunk/opentelemetry/opamp/effectiveconfig/EffectiveConfigReporterTest.java new file mode 100644 index 000000000..411134a57 --- /dev/null +++ b/opamp/src/test/java/com/splunk/opentelemetry/opamp/effectiveconfig/EffectiveConfigReporterTest.java @@ -0,0 +1,100 @@ +/* + * Copyright Splunk Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.splunk.opentelemetry.opamp.effectiveconfig; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import opamp.proto.AgentConfigFile; +import opamp.proto.AgentConfigMap; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.ArgumentCaptor; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; + +@ExtendWith(MockitoExtension.class) +class EffectiveConfigReporterTest { + private static final String CONFIG_FILE_NAME = "splunk-effective-config.properties"; + private static final String CONTENT_TYPE = "text/plain; format=properties; vendor=splunk"; + + @Mock private EffectiveConfigFactory effectiveConfigFactory; + @Mock private UpdatableEffectiveConfigState effectiveConfigState; + + private EffectiveConfigReporter reporter; + + @BeforeEach + void setUp() { + when(effectiveConfigFactory.getFileName()).thenReturn(CONFIG_FILE_NAME); + when(effectiveConfigFactory.getContentType()).thenReturn(CONTENT_TYPE); + reporter = new EffectiveConfigReporter(effectiveConfigFactory, effectiveConfigState); + } + + @Test + void reportEffectiveConfigIfChanged_reportsGeneratedConfig() { + when(effectiveConfigFactory.createEffectiveConfigContent()).thenReturn("first-config"); + + boolean reported = reporter.reportEffectiveConfigIfChanged(); + + assertThat(reported).isTrue(); + AgentConfigFile configFile = captureReportedConfigFile(); + assertThat(configFile.body.utf8()).isEqualTo("first-config"); + assertThat(configFile.content_type).isEqualTo(CONTENT_TYPE); + } + + @Test + void reportEffectiveConfigIfChanged_skipsUnchangedConfig() { + when(effectiveConfigFactory.createEffectiveConfigContent()).thenReturn("same-config"); + + boolean firstReport = reporter.reportEffectiveConfigIfChanged(); + boolean secondReport = reporter.reportEffectiveConfigIfChanged(); + + assertThat(firstReport).isTrue(); + assertThat(secondReport).isFalse(); + verify(effectiveConfigState, times(1)).set(any()); + } + + @Test + void reportEffectiveConfigIfChanged_reportsUpdatedConfig() { + when(effectiveConfigFactory.createEffectiveConfigContent()) + .thenReturn("first-config", "second-config"); + + boolean firstReport = reporter.reportEffectiveConfigIfChanged(); + boolean secondReport = reporter.reportEffectiveConfigIfChanged(); + + assertThat(firstReport).isTrue(); + assertThat(secondReport).isTrue(); + + ArgumentCaptor configMapCaptor = ArgumentCaptor.forClass(AgentConfigMap.class); + verify(effectiveConfigState, times(2)).set(configMapCaptor.capture()); + assertThat(configMapCaptor.getAllValues()) + .extracting(configMap -> configMap.config_map.get(CONFIG_FILE_NAME).body.utf8()) + .containsExactly("first-config", "second-config"); + } + + private AgentConfigFile captureReportedConfigFile() { + ArgumentCaptor configMapCaptor = ArgumentCaptor.forClass(AgentConfigMap.class); + verify(effectiveConfigState).set(configMapCaptor.capture()); + AgentConfigMap configMap = configMapCaptor.getValue(); + assertThat(configMap.config_map).containsOnlyKeys(CONFIG_FILE_NAME); + return configMap.config_map.get(CONFIG_FILE_NAME); + } +} diff --git a/opamp/src/test/java/com/splunk/opentelemetry/opamp/EnvVarsEffectiveConfigFileFactoryTest.java b/opamp/src/test/java/com/splunk/opentelemetry/opamp/effectiveconfig/EnvVarsEffectiveConfigFileFactoryTest.java similarity index 94% rename from opamp/src/test/java/com/splunk/opentelemetry/opamp/EnvVarsEffectiveConfigFileFactoryTest.java rename to opamp/src/test/java/com/splunk/opentelemetry/opamp/effectiveconfig/EnvVarsEffectiveConfigFileFactoryTest.java index f48165485..5e2c4d578 100644 --- a/opamp/src/test/java/com/splunk/opentelemetry/opamp/EnvVarsEffectiveConfigFileFactoryTest.java +++ b/opamp/src/test/java/com/splunk/opentelemetry/opamp/effectiveconfig/EnvVarsEffectiveConfigFileFactoryTest.java @@ -14,19 +14,27 @@ * limitations under the License. */ -package com.splunk.opentelemetry.opamp; +package com.splunk.opentelemetry.opamp.effectiveconfig; import static org.assertj.core.api.Assertions.assertThat; +import com.splunk.opentelemetry.profiler.ProfilerConfiguration; +import com.splunk.opentelemetry.profiler.ProfilerEnvVarsConfigurationFactory; import io.opentelemetry.sdk.autoconfigure.spi.internal.DefaultConfigProperties; import java.io.IOException; import java.io.StringReader; import java.util.Map; import java.util.Properties; +import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Test; class EnvVarsEffectiveConfigFileFactoryTest { + @AfterEach + void tearDown() { + ProfilerConfiguration.SUPPLIER.reset(); + } + @Test void createFile_reportsCorrectContentType() { DefaultConfigProperties config = DefaultConfigProperties.createFromMap(Map.of()); @@ -154,6 +162,7 @@ void buildFileContent_usesSignalSpecificProtocolWhenResolvingEndpoints() throws private static Properties createFileContent(Map configMap) throws IOException { DefaultConfigProperties config = DefaultConfigProperties.createFromMap(configMap); + ProfilerConfiguration.SUPPLIER.configure(ProfilerEnvVarsConfigurationFactory.create(config)); String fileContent = new EnvVarsEffectiveConfigFileFactory(config).createEffectiveConfigContent(); diff --git a/opamp/src/test/java/com/splunk/opentelemetry/opamp/YamlNodeBuilderTest.java b/opamp/src/test/java/com/splunk/opentelemetry/opamp/effectiveconfig/YamlNodeBuilderTest.java similarity index 98% rename from opamp/src/test/java/com/splunk/opentelemetry/opamp/YamlNodeBuilderTest.java rename to opamp/src/test/java/com/splunk/opentelemetry/opamp/effectiveconfig/YamlNodeBuilderTest.java index fd9d7e751..319902ea2 100644 --- a/opamp/src/test/java/com/splunk/opentelemetry/opamp/YamlNodeBuilderTest.java +++ b/opamp/src/test/java/com/splunk/opentelemetry/opamp/effectiveconfig/YamlNodeBuilderTest.java @@ -14,7 +14,7 @@ * limitations under the License. */ -package com.splunk.opentelemetry.opamp; +package com.splunk.opentelemetry.opamp.effectiveconfig; import static org.assertj.core.api.Assertions.assertThat; diff --git a/profiler/src/main/java/com/splunk/opentelemetry/profiler/PeriodicRecordingFlusher.java b/profiler/src/main/java/com/splunk/opentelemetry/profiler/PeriodicRecordingFlusher.java index ae6c2f16e..3ecfc58e1 100644 --- a/profiler/src/main/java/com/splunk/opentelemetry/profiler/PeriodicRecordingFlusher.java +++ b/profiler/src/main/java/com/splunk/opentelemetry/profiler/PeriodicRecordingFlusher.java @@ -20,7 +20,6 @@ import com.google.common.annotations.VisibleForTesting; import com.splunk.opentelemetry.profiler.util.HelpfulExecutors; -import io.opentelemetry.sdk.resources.Resource; import java.time.Duration; import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.ScheduledFuture; @@ -78,9 +77,4 @@ void handleInterval() { logger.log(SEVERE, "Profiler periodic task failed.", throwable); } } - - public static PeriodicRecordingFlusherBuilder builder( - ProfilerConfiguration config, Resource resource) { - return new PeriodicRecordingFlusherBuilder(config, resource); - } } diff --git a/profiler/src/main/java/com/splunk/opentelemetry/profiler/PeriodicRecordingFlusherBuilder.java b/profiler/src/main/java/com/splunk/opentelemetry/profiler/PeriodicRecordingFlusherFactory.java similarity index 94% rename from profiler/src/main/java/com/splunk/opentelemetry/profiler/PeriodicRecordingFlusherBuilder.java rename to profiler/src/main/java/com/splunk/opentelemetry/profiler/PeriodicRecordingFlusherFactory.java index 54297452a..332490caa 100644 --- a/profiler/src/main/java/com/splunk/opentelemetry/profiler/PeriodicRecordingFlusherBuilder.java +++ b/profiler/src/main/java/com/splunk/opentelemetry/profiler/PeriodicRecordingFlusherFactory.java @@ -37,25 +37,11 @@ import java.time.Duration; import java.util.Map; -class PeriodicRecordingFlusherBuilder { +class PeriodicRecordingFlusherFactory { private static final java.util.logging.Logger logger = - java.util.logging.Logger.getLogger(PeriodicRecordingFlusherBuilder.class.getName()); - private final ProfilerConfiguration config; - private final Resource resource; + java.util.logging.Logger.getLogger(PeriodicRecordingFlusherFactory.class.getName()); - private JFR jfr; - - public PeriodicRecordingFlusherBuilder(ProfilerConfiguration config, Resource resource) { - this.config = config; - this.resource = resource; - } - - PeriodicRecordingFlusherBuilder jfr(JFR jfr) { - this.jfr = jfr; - return this; - } - - PeriodicRecordingFlusher build() { + PeriodicRecordingFlusher create(ProfilerConfiguration config, Resource resource, JFR jfr) { if (jfr == null) { jfr = JFR.getInstance(); } diff --git a/profiler/src/main/java/com/splunk/opentelemetry/profiler/ProfilerConfiguration.java b/profiler/src/main/java/com/splunk/opentelemetry/profiler/ProfilerConfiguration.java index 12593b2a2..9bf4a7ed7 100644 --- a/profiler/src/main/java/com/splunk/opentelemetry/profiler/ProfilerConfiguration.java +++ b/profiler/src/main/java/com/splunk/opentelemetry/profiler/ProfilerConfiguration.java @@ -170,6 +170,54 @@ public Object getConfigProperties() { return configProperties; } + @Override + public boolean equals(Object other) { + if (this == other) { + return true; + } + if (!(other instanceof ProfilerConfiguration)) { + return false; + } + ProfilerConfiguration that = (ProfilerConfiguration) other; + return enabled == that.enabled + && memoryEnabled == that.memoryEnabled + && memoryEventRateLimitEnabled == that.memoryEventRateLimitEnabled + && useAllocationSampleEvent == that.useAllocationSampleEvent + && includeAgentInternalStacks == that.includeAgentInternalStacks + && includeJvmInternalStacks == that.includeJvmInternalStacks + && tracingStacksOnly == that.tracingStacksOnly + && stackDepth == that.stackDepth + && keepFiles == that.keepFiles + && Objects.equals(ingestUrl, that.ingestUrl) + && Objects.equals(otlpProtocol, that.otlpProtocol) + && Objects.equals(memoryEventRate, that.memoryEventRate) + && Objects.equals(callStackInterval, that.callStackInterval) + && Objects.equals(profilerDirectory, that.profilerDirectory) + && Objects.equals(recordingDuration, that.recordingDuration) + && Objects.equals(configProperties, that.configProperties); + } + + @Override + public int hashCode() { + return Objects.hash( + enabled, + ingestUrl, + otlpProtocol, + memoryEnabled, + memoryEventRateLimitEnabled, + memoryEventRate, + useAllocationSampleEvent, + callStackInterval, + includeAgentInternalStacks, + includeJvmInternalStacks, + tracingStacksOnly, + stackDepth, + keepFiles, + profilerDirectory, + recordingDuration, + configProperties); + } + public static int getJavaVersion() { String javaSpecVersion = System.getProperty("java.specification.version"); if ("1.8".equals(javaSpecVersion)) { diff --git a/profiler/src/main/java/com/splunk/opentelemetry/profiler/ProfilingSupervisor.java b/profiler/src/main/java/com/splunk/opentelemetry/profiler/ProfilingSupervisor.java index 2b9269ffd..dbb362338 100644 --- a/profiler/src/main/java/com/splunk/opentelemetry/profiler/ProfilingSupervisor.java +++ b/profiler/src/main/java/com/splunk/opentelemetry/profiler/ProfilingSupervisor.java @@ -45,6 +45,7 @@ public class ProfilingSupervisor { private final JFR jfr; private final AutoConfiguredOpenTelemetrySdk sdk; private final BlockingQueue commandQueue; + private final PeriodicRecordingFlusherFactory recordingFlusherFactory; private final AtomicReference recordingFlusher = new AtomicReference<>(); private static final AtomicReference jfrContextStorage = @@ -57,10 +58,21 @@ public class ProfilingSupervisor { JFR jfr, AutoConfiguredOpenTelemetrySdk sdk, BlockingQueue commandQueue) { + this(configSupplier, jfr, sdk, commandQueue, new PeriodicRecordingFlusherFactory()); + } + + @VisibleForTesting + ProfilingSupervisor( + OptionalConfigurableSupplier configSupplier, + JFR jfr, + AutoConfiguredOpenTelemetrySdk sdk, + BlockingQueue commandQueue, + PeriodicRecordingFlusherFactory recordingFlusherFactory) { this.configSupplier = configSupplier; this.jfr = jfr; this.sdk = sdk; this.commandQueue = commandQueue; + this.recordingFlusherFactory = recordingFlusherFactory; } static ProfilingSupervisor createAndStart(AutoConfiguredOpenTelemetrySdk sdk) { @@ -105,6 +117,10 @@ public void requestStopProfiling() { commandQueue.add(ProfilingCommand.STOP); } + public void requestReinitializeProfiling() { + commandQueue.add(ProfilingCommand.REINITIALIZE); + } + private void handleCommand(ProfilingCommand command) { switch (command) { case START: @@ -113,6 +129,9 @@ private void handleCommand(ProfilingCommand command) { case STOP: tryStop(); break; + case REINITIALIZE: + tryReinitialize(); + break; } } @@ -153,13 +172,26 @@ private void tryStop() { logger.info("Profiler is deactivated."); } + private void tryReinitialize() { + logger.info("Reinitializing profiler."); + // Stop if currently running + if (isJfrRecordingActive()) { + tryStop(); + } + // Start with current setting if profiling is enabled. If settings changed since last start they + // will be applied. + if (configSupplier.get().isEnabled()) { + tryStart(); + } + } + private boolean isJfrRecordingActive() { return recordingFlusher.get() != null; } private void activateJfrRecording(Resource resource) { PeriodicRecordingFlusher recordingFlusher = - makeRecordingFlusherBuilder(resource).jfr(jfr).build(); + recordingFlusherFactory.create(configSupplier.get(), resource, jfr); if (this.recordingFlusher.compareAndSet(null, recordingFlusher)) { recordingFlusher.start(); } @@ -172,11 +204,6 @@ private void deactivateJfrRecording() { } } - // Exists for testing - PeriodicRecordingFlusherBuilder makeRecordingFlusherBuilder(Resource resource) { - return PeriodicRecordingFlusher.builder(configSupplier.get(), resource); - } - static void setupJfrContextStorage() { if (!contextStorageSetup.compareAndSet(false, true)) { return; @@ -192,6 +219,7 @@ static void setupJfrContextStorage() { enum ProfilingCommand { START, - STOP + STOP, + REINITIALIZE } } diff --git a/profiler/src/test/java/com/splunk/opentelemetry/profiler/PeriodicRecordingFlusherBuilderTest.java b/profiler/src/test/java/com/splunk/opentelemetry/profiler/PeriodicRecordingFlusherFactoryTest.java similarity index 79% rename from profiler/src/test/java/com/splunk/opentelemetry/profiler/PeriodicRecordingFlusherBuilderTest.java rename to profiler/src/test/java/com/splunk/opentelemetry/profiler/PeriodicRecordingFlusherFactoryTest.java index 0ab6b23f0..e3c414a85 100644 --- a/profiler/src/test/java/com/splunk/opentelemetry/profiler/PeriodicRecordingFlusherBuilderTest.java +++ b/profiler/src/test/java/com/splunk/opentelemetry/profiler/PeriodicRecordingFlusherFactoryTest.java @@ -33,7 +33,7 @@ import org.junit.jupiter.api.io.TempDir; import org.mockito.MockedConstruction; -class PeriodicRecordingFlusherBuilderTest { +class PeriodicRecordingFlusherFactoryTest { @TempDir Path tempDir; @@ -43,62 +43,62 @@ void tearDown() { } @Test - void buildConfiguresJfrAndWiresRecorderIntoSequencer() { + void createConfiguresJfrAndWiresRecorderIntoSequencer() { JFR jfr = mock(JFR.class); ProfilerConfiguration config = config(tempDir).setStackDepth(73).setRecordingDuration(Duration.ofMillis(100)).build(); ProfilerConfiguration.SUPPLIER.configure(config); + PeriodicRecordingFlusherFactory factory = new PeriodicRecordingFlusherFactory(); try (MockedConstruction recorderConstruction = mockConstruction(JfrRecorder.class)) { - PeriodicRecordingFlusher sequencer = - PeriodicRecordingFlusher.builder(config, Resource.empty()).jfr(jfr).build(); + PeriodicRecordingFlusher flusher = factory.create(config, Resource.empty(), jfr); - assertThat(sequencer).isNotNull(); + assertThat(flusher).isNotNull(); assertThat(recorderConstruction.constructed()).hasSize(1); verify(jfr).setStackDepth(73); JfrRecorder recorder = recorderConstruction.constructed().get(0); when(recorder.isStarted()).thenReturn(true); - sequencer.handleInterval(); + flusher.handleInterval(); verify(recorder).flushSnapshot(); } } @Test - void buildCreatesMissingOutputDirectoryWhenKeepingFiles() { + void createCreatesMissingOutputDirectoryWhenKeepingFiles() { Path outputDir = tempDir.resolve("profiler-output"); JFR jfr = mock(JFR.class); ProfilerConfiguration config = config(outputDir).setKeepFiles(true).build(); ProfilerConfiguration.SUPPLIER.configure(config); + PeriodicRecordingFlusherFactory factory = new PeriodicRecordingFlusherFactory(); try (MockedConstruction recorderConstruction = mockConstruction(JfrRecorder.class)) { - PeriodicRecordingFlusher sequencer = - PeriodicRecordingFlusher.builder(config, Resource.empty()).jfr(jfr).build(); + PeriodicRecordingFlusher flusher = factory.create(config, Resource.empty(), jfr); - assertThat(sequencer).isNotNull(); + assertThat(flusher).isNotNull(); assertThat(outputDir).isDirectory(); assertThat(recorderConstruction.constructed()).hasSize(1); } } @Test - void buildContinuesWhenKeepFilesPathIsNotADirectory() throws Exception { + void createContinuesWhenKeepFilesPathIsNotADirectory() throws Exception { Path outputFile = tempDir.resolve("profiler-output"); Files.createFile(outputFile); JFR jfr = mock(JFR.class); ProfilerConfiguration config = config(outputFile).setKeepFiles(true).build(); ProfilerConfiguration.SUPPLIER.configure(config); + PeriodicRecordingFlusherFactory factory = new PeriodicRecordingFlusherFactory(); try (MockedConstruction recorderConstruction = mockConstruction(JfrRecorder.class)) { - PeriodicRecordingFlusher sequencer = - PeriodicRecordingFlusher.builder(config, Resource.empty()).jfr(jfr).build(); + PeriodicRecordingFlusher flusher = factory.create(config, Resource.empty(), jfr); - assertThat(sequencer).isNotNull(); + assertThat(flusher).isNotNull(); assertThat(recorderConstruction.constructed()).hasSize(1); } } diff --git a/profiler/src/test/java/com/splunk/opentelemetry/profiler/ProfilerConfigurationTest.java b/profiler/src/test/java/com/splunk/opentelemetry/profiler/ProfilerConfigurationTest.java index 449b0ba24..daf2c95cd 100644 --- a/profiler/src/test/java/com/splunk/opentelemetry/profiler/ProfilerConfigurationTest.java +++ b/profiler/src/test/java/com/splunk/opentelemetry/profiler/ProfilerConfigurationTest.java @@ -49,7 +49,8 @@ void toBuilder_shouldCopyExistingConfigurationWithoutMutation() { ProfilerConfiguration copy = original.toBuilder().build(); assertThat(copy).isNotSameAs(original); - assertThat(copy).usingRecursiveComparison().isEqualTo(original); + assertThat(copy).isEqualTo(original); + assertThat(copy.hashCode()).isEqualTo(original.hashCode()); assertThat(copy.getConfigProperties()).isSameAs(configProperties); } @@ -97,6 +98,7 @@ void toBuilder_shouldCopyExistingConfigurationAndAllowMutation() { .setConfigProperties(mutatedConfigProperties) .build(); + assertThat(copy).isNotEqualTo(original); assertThat(copy.isEnabled()).isTrue(); assertThat(copy.getIngestUrl()).isEqualTo("https://mutated-logs.example.com"); assertThat(copy.getOtlpProtocol()).isEqualTo("http/protobuf"); diff --git a/profiler/src/test/java/com/splunk/opentelemetry/profiler/ProfilingSupervisorTest.java b/profiler/src/test/java/com/splunk/opentelemetry/profiler/ProfilingSupervisorTest.java index 1d4a1d0b1..724744a77 100644 --- a/profiler/src/test/java/com/splunk/opentelemetry/profiler/ProfilingSupervisorTest.java +++ b/profiler/src/test/java/com/splunk/opentelemetry/profiler/ProfilingSupervisorTest.java @@ -16,10 +16,11 @@ package com.splunk.opentelemetry.profiler; -import static org.assertj.core.api.Assertions.assertThat; import static org.awaitility.Awaitility.await; +import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyInt; -import static org.mockito.Mockito.mock; +import static org.mockito.ArgumentMatchers.same; +import static org.mockito.Mockito.lenient; import static org.mockito.Mockito.never; import static org.mockito.Mockito.spy; import static org.mockito.Mockito.times; @@ -46,9 +47,11 @@ class ProfilingSupervisorTest { @Mock JFR jfr; + @Mock PeriodicRecordingFlusherFactory recordingFlusherFactory; + @Mock PeriodicRecordingFlusher recordingFlusher; ProfilerConfiguration config; - TestPeriodicRecordingFlusherBuilder builder; + OptionalConfigurableSupplier configSupplier; AutoConfiguredOpenTelemetrySdk sdk; ExecutorService executor; ProfilingSupervisor supervisor; @@ -63,7 +66,13 @@ void setUp(@TempDir Path tempDir) { .setStackDepth(4321) .setRecordingDuration(Duration.ofMinutes(1)) .build()); - builder = new TestPeriodicRecordingFlusherBuilder(config, mock(Resource.class)); + configSupplier = new OptionalConfigurableSupplier<>(); + configSupplier.configure(config); + lenient() + .when( + recordingFlusherFactory.create( + any(ProfilerConfiguration.class), any(Resource.class), any(JFR.class))) + .thenReturn(recordingFlusher); executor = Executors.newSingleThreadExecutor(); sdk = AutoConfiguredOpenTelemetrySdk.builder() @@ -80,7 +89,9 @@ void setUp(@TempDir Path tempDir) { "otel.service.name", "profiling-supervisor-test")) .build(); - supervisor = new TestProfilingSupervisor(config, jfr, sdk, builder); + supervisor = + new ProfilingSupervisor( + configSupplier, jfr, sdk, new LinkedBlockingQueue<>(), recordingFlusherFactory); supervisor.start(executor); } @@ -90,121 +101,170 @@ void tearDown() { } @Test - void requestStartDoesNotStartProfilerWhenJfrIsUnavailable() { + void requestStartProfiling_doesNotStartProfilerWhenJfrIsUnavailable() { + // given when(jfr.isAvailable()).thenReturn(false); + // when supervisor.requestStartProfiling(); + // then await().untilAsserted(() -> verify(jfr).isAvailable()); verify(config, never()).log(); verify(jfr, never()).setStackDepth(anyInt()); - assertThat(builder.buildCalled).isFalse(); + verify(recordingFlusherFactory, never()).create(any(), any(), any()); } @Test - void requestStartProfilingBuildsAndStartsRecordingSequencer() { + void requestStartProfiling_buildsAndStartsRecordingFlusher() { + // given when(jfr.isAvailable()).thenReturn(true); + // when supervisor.requestStartProfiling(); - await().untilAsserted(() -> assertThat(builder.buildCalled).isTrue()); + // then + await().untilAsserted(() -> verify(recordingFlusher).start()); verify(config).log(); - assertThat(builder.jfr).isSameAs(jfr); - assertThat(builder.config).isSameAs(config); - assertThat(builder.resource).isNotNull(); - verify(builder.flusher).start(); + verifyRecordingFlusherCreatedWith(config); } @Test - void requestStartProfilingOnlyStartsOnce() { + void requestStartProfiling_startsOnlyOnce() { + // given when(jfr.isAvailable()).thenReturn(true); + // when supervisor.requestStartProfiling(); - await().untilAsserted(() -> assertThat(builder.buildCalled).isTrue()); + await().untilAsserted(() -> verify(recordingFlusherFactory).create(any(), any(), any())); supervisor.requestStartProfiling(); - await().during(Duration.ofMillis(200)).untilAsserted(() -> verify(builder.flusher).start()); - assertThat(builder.buildCount).isEqualTo(1); + // then + await() + .during(Duration.ofMillis(200)) + .untilAsserted( + () -> { + verify(recordingFlusher).start(); + verifyRecordingFlusherCreated(1); + }); } @Test - void requestStopProfilingStopsActiveRecordingFlusher() { + void requestStopProfiling_stopsActiveRecordingFlusher() { + // given when(jfr.isAvailable()).thenReturn(true); - supervisor.requestStartProfiling(); - await().untilAsserted(() -> verify(builder.flusher).start()); + await().untilAsserted(() -> verify(recordingFlusher).start()); + // when supervisor.requestStopProfiling(); - await().untilAsserted(() -> verify(builder.flusher).stop()); + // then + await().untilAsserted(() -> verify(recordingFlusher).stop()); } @Test - void requestStartProfilingCanStartAgainAfterStop() { + void requestStartProfiling_startsAfterStop() { + // given when(jfr.isAvailable()).thenReturn(true); - supervisor.requestStartProfiling(); - await().untilAsserted(() -> verify(builder.flusher).start()); + await().untilAsserted(() -> verify(recordingFlusher).start()); supervisor.requestStopProfiling(); - await().untilAsserted(() -> verify(builder.flusher).stop()); + await().untilAsserted(() -> verify(recordingFlusher).stop()); + + // when + supervisor.requestStartProfiling(); + + // then + await().untilAsserted(() -> verify(recordingFlusher, times(2)).start()); + verifyRecordingFlusherCreated(2); + } + + @Test + void requestReinitializeProfiling_restartsActiveProfilerWhenEnabled() { + // given + when(jfr.isAvailable()).thenReturn(true); + supervisor.requestStartProfiling(); + await().untilAsserted(() -> verify(recordingFlusher).start()); + + ProfilerConfiguration updatedConfig = spy(config.toBuilder().setStackDepth(1234).build()); + configSupplier.configure(updatedConfig); + + // when + supervisor.requestReinitializeProfiling(); + // then + await().untilAsserted(() -> verify(recordingFlusher, times(2)).start()); + verify(recordingFlusher).stop(); + verify(updatedConfig).log(); + verifyRecordingFlusherCreated(2); + verifyRecordingFlusherCreatedWith(updatedConfig); + } + + @Test + void requestReinitializeProfiling_stopsActiveProfilerWhenDisabled() { + // given + when(jfr.isAvailable()).thenReturn(true); supervisor.requestStartProfiling(); + await().untilAsserted(() -> verify(recordingFlusher).start()); + + ProfilerConfiguration disabledConfig = spy(config.toBuilder().setEnabled(false).build()); + configSupplier.configure(disabledConfig); + + // when + supervisor.requestReinitializeProfiling(); + + // then + await().untilAsserted(() -> verify(recordingFlusher).stop()); + verify(disabledConfig).isEnabled(); + await() + .during(Duration.ofMillis(200)) + .untilAsserted( + () -> { + verify(recordingFlusher).start(); + verifyRecordingFlusherCreated(1); + }); + } + + @Test + void requestReinitializeProfiling_startsInactiveProfilerWhenEnabled() { + // given + when(jfr.isAvailable()).thenReturn(true); + ProfilerConfiguration updatedConfig = spy(config.toBuilder().setStackDepth(1234).build()); + configSupplier.configure(updatedConfig); + + // when + supervisor.requestReinitializeProfiling(); - await().untilAsserted(() -> verify(builder.flusher, times(2)).start()); - assertThat(builder.buildCount).isEqualTo(2); + // then + await().untilAsserted(() -> verify(recordingFlusher).start()); + verify(recordingFlusher, never()).stop(); + verify(updatedConfig).log(); + verifyRecordingFlusherCreatedWith(updatedConfig); + } + + @Test + void requestReinitializeProfiling_keepsInactiveProfilerStoppedWhenDisabled() { + // given + ProfilerConfiguration disabledConfig = spy(config.toBuilder().setEnabled(false).build()); + configSupplier.configure(disabledConfig); + + // when + supervisor.requestReinitializeProfiling(); + + // then + await().untilAsserted(() -> verify(disabledConfig).isEnabled()); + verify(jfr, never()).isAvailable(); + verify(recordingFlusher, never()).start(); + verify(recordingFlusher, never()).stop(); + verify(recordingFlusherFactory, never()).create(any(), any(), any()); } - private static class TestProfilingSupervisor extends ProfilingSupervisor { - private final PeriodicRecordingFlusherBuilder builder; - - TestProfilingSupervisor( - ProfilerConfiguration config, - JFR jfr, - AutoConfiguredOpenTelemetrySdk sdk, - PeriodicRecordingFlusherBuilder builder) { - super(configSupplier(config), jfr, sdk, new LinkedBlockingQueue<>()); - this.builder = builder; - } - - @Override - PeriodicRecordingFlusherBuilder makeRecordingFlusherBuilder(Resource resource) { - return builder; - } - - private static OptionalConfigurableSupplier configSupplier( - ProfilerConfiguration config) { - OptionalConfigurableSupplier supplier = - new OptionalConfigurableSupplier<>(); - supplier.configure(config); - return supplier; - } + private void verifyRecordingFlusherCreated(int count) { + verify(recordingFlusherFactory, times(count)).create(any(), any(), any()); } - private static class TestPeriodicRecordingFlusherBuilder extends PeriodicRecordingFlusherBuilder { - final PeriodicRecordingFlusher flusher = mock(PeriodicRecordingFlusher.class); - private final ProfilerConfiguration config; - private final Resource resource; - JFR jfr; - boolean buildCalled; - int buildCount; - - public TestPeriodicRecordingFlusherBuilder(ProfilerConfiguration config, Resource resource) { - super(config, resource); - this.config = config; - this.resource = resource; - } - - @Override - PeriodicRecordingFlusherBuilder jfr(JFR jfr) { - this.jfr = jfr; - return this; - } - - @Override - PeriodicRecordingFlusher build() { - buildCalled = true; - buildCount++; - return flusher; - } + private void verifyRecordingFlusherCreatedWith(ProfilerConfiguration config) { + verify(recordingFlusherFactory).create(same(config), any(Resource.class), same(jfr)); } }