Closes #1108 - Introduce an option to derive the agent-command url based on the HTTP config URL

inspectit-ocelot-config/src/main/java/rocks/inspectit/ocelot/config/model/command/AgentCommandSettings.java

@@ -20,6 +20,11 @@ public class AgentCommandSettings {
      */
     private URL url;
 
+    /**
+     * Whether the agent commands URL should be derived from the HTTP configuration URL.
+     */
+    private boolean deriveFromHttpConfigUrl = false;
+
     /**
      * The timeout duration used for requests  when the agent is in discovery mode. Defining how long the agent will wait for
      * new commands.

inspectit-ocelot-config/src/main/resources/rocks/inspectit/ocelot/config/default/basics.yml

@@ -106,7 +106,9 @@ inspectit:
     # whether agent commands are enabled or not
     enabled: false
     # the URL for fetching agent commands - e.g.: http://localhost:8090/api/v1/agent/command
-    url: http://localhost:8090/api/v1/agent/command
+    url:
+    # whether the agent commands URL should be derived from the HTTP configuration URL
+    derive-from-http-config-url: false
     # the timeout duration used when the agent is in discovery mode. Defining how long the agent
     # will wait for new commands.
     live-socket-timeout: 30s

inspectit-ocelot-core/src/main/java/rocks/inspectit/ocelot/core/command/AgentCommandService.java

@@ -1,12 +1,16 @@
 package rocks.inspectit.ocelot.core.command;
 
+import com.google.common.annotations.VisibleForTesting;
 import lombok.extern.slf4j.Slf4j;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.stereotype.Service;
 import rocks.inspectit.ocelot.config.model.InspectitConfig;
 import rocks.inspectit.ocelot.config.model.command.AgentCommandSettings;
 import rocks.inspectit.ocelot.core.service.DynamicallyActivatableService;
 
+import java.net.URI;
+import java.net.URISyntaxException;
+import java.net.URL;
 import java.util.concurrent.ScheduledExecutorService;
 import java.util.concurrent.ScheduledFuture;
 import java.util.concurrent.TimeUnit;
@@ -21,12 +25,12 @@ public class AgentCommandService extends DynamicallyActivatableService implement
     @Autowired
     private ScheduledExecutorService executor;
 
-    /**
-     * The state of the used HTTP property source configuration.
-     */
     @Autowired
     private CommandHandler commandHandler;
 
+    @Autowired
+    private HttpCommandFetcher commandFetcher;
+
     /**
      * The scheduled task.
      */
@@ -38,13 +42,32 @@ public AgentCommandService() {
 
     @Override
     protected boolean checkEnabledForConfig(InspectitConfig configuration) {
-        return configuration.getAgentCommands().isEnabled();
+        AgentCommandSettings settings = configuration.getAgentCommands();
+        // the feature has to be enabled
+        if (!settings.isEnabled()) {
+            return false;
+        }
+
+        // enable the feature if the url is based on the HTTP config URL OR the url is specified directly
+        if (settings.isDeriveFromHttpConfigUrl()) {
+            return true;
+        } else {
+            return settings.getUrl() != null;
+        }
     }
 
     @Override
     protected boolean doEnable(InspectitConfig configuration) {
         log.info("Starting agent command polling service.");
 
+        try {
+            URI commandUri = getCommandUri(configuration);
+            commandFetcher.setCommandUri(commandUri);
+        } catch (Exception e) {
+            log.error("Could not enable the agent command polling service.", e);
+            return false;
+        }
+
         AgentCommandSettings settings = configuration.getAgentCommands();
         long pollingIntervalMs = settings.getPollingInterval().toMillis();
 
@@ -72,4 +95,27 @@ public void run() {
             log.error("Error while fetching agent command.", exception);
         }
     }
+
+    @VisibleForTesting
+    URI getCommandUri(InspectitConfig configuration) throws URISyntaxException {
+        AgentCommandSettings settings = configuration.getAgentCommands();
+
+        if (settings.isDeriveFromHttpConfigUrl()) {
+            URL url = configuration.getConfig().getHttp().getUrl();
+            if (url == null) {
+                throw new IllegalStateException("The URL cannot derived from the HTTP configuration URL because it is null.");
+            }
+
+            String urlBase = String.format("%s://%s", url.getProtocol(), url.getHost());
+
+            int port = url.getPort();
+            if (port != -1) {
+                urlBase += ":" + port;
+            }
+
+            return URI.create(urlBase + "/api/v1/agent/command");
+        } else {
+            return settings.getUrl().toURI();
+        }
+    }
 }

inspectit-ocelot-core/src/main/java/rocks/inspectit/ocelot/core/command/HttpCommandFetcher.java

@@ -2,6 +2,7 @@
 
 import com.fasterxml.jackson.core.JsonProcessingException;
 import com.fasterxml.jackson.databind.ObjectMapper;
+import lombok.Setter;
 import lombok.extern.slf4j.Slf4j;
 import org.apache.http.HttpResponse;
 import org.apache.http.client.HttpClient;
@@ -11,10 +12,13 @@
 import org.apache.http.entity.StringEntity;
 import org.apache.http.impl.client.HttpClientBuilder;
 import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.context.event.EventListener;
 import org.springframework.stereotype.Component;
 import rocks.inspectit.ocelot.commons.models.command.Command;
 import rocks.inspectit.ocelot.commons.models.command.response.CommandResponse;
+import rocks.inspectit.ocelot.config.model.InspectitConfig;
 import rocks.inspectit.ocelot.config.model.command.AgentCommandSettings;
+import rocks.inspectit.ocelot.core.config.InspectitConfigChangedEvent;
 import rocks.inspectit.ocelot.core.config.InspectitEnvironment;
 
 import java.io.UnsupportedEncodingException;
@@ -50,6 +54,12 @@ public class HttpCommandFetcher {
      */
     private HttpClient liveHttpClient;
 
+    /**
+     * The URI for fetching commands.
+     */
+    @Setter
+    private URI commandUri;
+
     /**
      * Returns the {@link HttpClient} which is used for fetching commands.
      *
@@ -87,12 +97,10 @@ private void updateHttpClients() {
      *
      * @return returns null if any error occurred before/while sending the request.
      */
-    public HttpResponse fetchCommand(CommandResponse commandResponse, boolean waitForCommand)  {
+    public HttpResponse fetchCommand(CommandResponse commandResponse, boolean waitForCommand) {
         HttpPost httpPost;
         try {
-            AgentCommandSettings settings = environment.getCurrentConfig().getAgentCommands();
-
-            URIBuilder uriBuilder = new URIBuilder(settings.getUrl().toURI());
+            URIBuilder uriBuilder = new URIBuilder(commandUri);
             if (waitForCommand) {
                 uriBuilder.addParameter("wait-for-command", "true");
             }
@@ -123,7 +131,7 @@ public HttpResponse fetchCommand(CommandResponse commandResponse, boolean waitFo
         try {
             return getHttpClient(waitForCommand).execute(httpPost);
         } catch (Exception e) {
-            log.error("An error occurred while fetching a new command: " + e.getMessage());
+            log.error("An error occurred while fetching an agent command.", e);
         } finally {
             httpPost.releaseConnection();
         }

inspectit-ocelot-core/src/test/java/rocks/inspectit/ocelot/core/command/AgentCommandServiceTest.java

@@ -3,21 +3,24 @@
 import org.junit.jupiter.api.Nested;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.ExtendWith;
-import org.mockito.Answers;
-import org.mockito.InjectMocks;
-import org.mockito.Mock;
-import org.mockito.Mockito;
+import org.mockito.*;
 import org.mockito.junit.jupiter.MockitoExtension;
 import rocks.inspectit.ocelot.config.model.InspectitConfig;
 import rocks.inspectit.ocelot.config.model.config.ConfigSettings;
 import rocks.inspectit.ocelot.config.model.config.HttpConfigSettings;
 
+import java.net.MalformedURLException;
+import java.net.URI;
+import java.net.URISyntaxException;
+import java.net.URL;
 import java.time.Duration;
 import java.util.concurrent.Future;
 import java.util.concurrent.ScheduledExecutorService;
 import java.util.concurrent.ScheduledFuture;
 import java.util.concurrent.TimeUnit;
 
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.anyLong;
@@ -35,20 +38,30 @@ public class AgentCommandServiceTest {
     @Mock(answer = Answers.RETURNS_DEEP_STUBS)
     private InspectitConfig configuration;
 
+    @Mock
+    private HttpCommandFetcher commandFetcher;
+
     @Nested
     public class DoEnable {
 
+        @Captor
+        private ArgumentCaptor<URI>  uriCaptor;
+
         @Test
-        public void successfullyEnabled() {
+        public void successfullyEnabled() throws MalformedURLException {
             when(configuration.getAgentCommands().getPollingInterval()).thenReturn(Duration.ofSeconds(1));
+            lenient().when(configuration.getAgentCommands().getUrl()).thenReturn(new URL("http://inspectit.rocks"));
+            when(configuration.getConfig().getHttp().getUrl()).thenReturn(new URL("http://example.org/api/endpoint"));
+            when(configuration.getAgentCommands().isDeriveFromHttpConfigUrl()).thenReturn(true);
 
             boolean result = service.doEnable(configuration);
 
-            assertTrue(result);
             verify(executor).scheduleWithFixedDelay(service, 1000, 1000, TimeUnit.MILLISECONDS);
-            verifyNoMoreInteractions(executor);
+            verify(commandFetcher).setCommandUri(uriCaptor.capture());
+            verifyNoMoreInteractions(executor, commandFetcher);
+            assertThat(result).isTrue();
+            assertThat(uriCaptor.getValue().toString()).isEqualTo("http://example.org/api/v1/agent/command");
         }
-
     }
 
     @Nested
@@ -58,22 +71,76 @@ public class DoDisable {
         public void notEnabled() {
             boolean result = service.doDisable();
 
-            assertTrue(result);
+            assertThat(result).isTrue();
+            verifyZeroInteractions(commandFetcher);
         }
 
         @Test
-        public void isEnabled() {
+        public void isEnabled() throws MalformedURLException {
             when(configuration.getAgentCommands().getPollingInterval()).thenReturn(Duration.ofSeconds(1));
+            when(configuration.getAgentCommands().getUrl()).thenReturn(new URL("http://example.org"));
             ScheduledFuture futureMock = mock(ScheduledFuture.class);
             when(executor.scheduleWithFixedDelay(any(Runnable.class), anyLong(), anyLong(), any(TimeUnit.class))).thenReturn(futureMock);
 
             service.doEnable(configuration);
 
             boolean result = service.doDisable();
 
-            assertTrue(result);
+            assertThat(result).isTrue();
             verify(futureMock).cancel(true);
+            verify(commandFetcher).setCommandUri(any());
+            verifyNoMoreInteractions(commandFetcher);
         }
     }
 
+    @Nested
+    public class GetCommandUri {
+
+        @Test
+        public void validCommandUrl() throws Exception {
+            when(configuration.getAgentCommands().getUrl()).thenReturn(new URL("http://example.org:8090/api"));
+
+            URI result = service.getCommandUri(configuration);
+
+            assertThat(result.toString()).isEqualTo("http://example.org:8090/api");
+        }
+
+        @Test
+        public void deriveUrlWithoutConfigUrl() {
+            when(configuration.getAgentCommands().isDeriveFromHttpConfigUrl()).thenReturn(true);
+
+            assertThatExceptionOfType(IllegalStateException.class).isThrownBy(() -> service.getCommandUri(configuration))
+                    .withMessage("The URL cannot derived from the HTTP configuration URL because it is null.");
+        }
+
+        @Test
+        public void deriveUrl() throws Exception {
+            when(configuration.getConfig()
+                    .getHttp()
+                    .getUrl()).thenReturn(new URL("http://example.org:8090/api/endpoint"));
+            when(configuration.getAgentCommands().isDeriveFromHttpConfigUrl()).thenReturn(true);
+            URI result = service.getCommandUri(configuration);
+
+            assertThat(result.toString()).isEqualTo("http://example.org:8090/api/v1/agent/command");
+        }
+
+        @Test
+        public void deriveUrlWithoutPort() throws Exception {
+            when(configuration.getConfig().getHttp().getUrl()).thenReturn(new URL("http://example.org/api/endpoint"));
+            when(configuration.getAgentCommands().isDeriveFromHttpConfigUrl()).thenReturn(true);
+            URI result = service.getCommandUri(configuration);
+
+            assertThat(result.toString()).isEqualTo("http://example.org/api/v1/agent/command");
+        }
+
+        @Test
+        public void verifyPrioritization() throws Exception {
+            lenient().when(configuration.getAgentCommands().getUrl()).thenReturn(new URL("http://example.org"));
+            when(configuration.getConfig().getHttp().getUrl()).thenReturn(new URL("http://example.org/api/endpoint"));
+            when(configuration.getAgentCommands().isDeriveFromHttpConfigUrl()).thenReturn(true);
+            URI result = service.getCommandUri(configuration);
+
+            assertThat(result.toString()).isEqualTo("http://example.org/api/v1/agent/command");
+        }
+    }
 }