Fix json polling (#1436)

Author: trask

agent/agent-bootstrap/src/main/java/com/microsoft/applicationinsights/agent/bootstrap/configuration/ConfigurationBuilder.java

@@ -176,7 +176,7 @@ private static String getEnvVarOrProperty(String envVarName, String propertyName
         return value != null ? value : trimAndEmptyToNull(System.getProperty(propertyName));
     }
 
-    static void overlayEnvVars(Configuration config) throws IOException {
+    public static void overlayEnvVars(Configuration config) throws IOException {
         config.connectionString = overlayWithEnvVar(APPLICATIONINSIGHTS_CONNECTION_STRING, config.connectionString);
         if (config.connectionString == null) {
             // this is for backwards compatibility only

agent/agent-tooling/src/main/java/com/microsoft/applicationinsights/agent/internal/JsonConfigPolling.java

@@ -48,7 +48,8 @@ public class JsonConfigPolling implements Runnable {
     private volatile double lastReadSamplingPercentage;
     private static final Logger logger = LoggerFactory.getLogger(JsonConfigPolling.class);
 
-    private JsonConfigPolling(Path path, long lastModifiedTime, double lastReadSamplingPercentage) {
+    // visible for testing
+    JsonConfigPolling(Path path, long lastModifiedTime, double lastReadSamplingPercentage) {
         this.path = path;
         this.lastModifiedTime = lastModifiedTime;
         this.lastReadSamplingPercentage = lastReadSamplingPercentage;
@@ -80,16 +81,14 @@ public void run() {
             if (lastModifiedTime != fileTime.toMillis()) {
                 lastModifiedTime = fileTime.toMillis();
                 Configuration configuration = ConfigurationBuilder.loadJsonConfigFile(path);
+                // important to overlay env vars here, so that we don't overwrite the value set by env var
+                ConfigurationBuilder.overlayEnvVars(configuration);
 
-                // TODO only want to update connectionString with value from configuration file if original value
-                //  is from configuration file (not if original value is from APPLICATIONINSIGHTS_CONNECTION_STRING env var)
                 if (!configuration.connectionString.equals(TelemetryConfiguration.getActive().getConnectionString())) {
                     logger.debug("Connection string from the JSON config file is overriding the previously configured connection string.");
                     TelemetryConfiguration.getActive().setConnectionString(configuration.connectionString);
                 }
 
-                // TODO only want to update sampling percentage with value from configuration file if original value
-                //  is from configuration file (not if original value is from APPLICATIONINSIGHTS_SAMPLING_PERCENTAGE env var)
                 if (configuration.sampling.percentage != lastReadSamplingPercentage) {
                     logger.debug("Updating sampling percentage from {} to {}", lastReadSamplingPercentage, configuration.sampling.percentage);
                     double roundedSamplingPercentage = SamplingPercentage.roundToNearest(configuration.sampling.percentage);

agent/agent-tooling/src/test/java/com/microsoft/applicationinsights/agent/internal/JsonConfigPollingTest.java

@@ -0,0 +1,81 @@
+/*
+ * ApplicationInsights-Java
+ * Copyright (c) Microsoft Corporation
+ * All rights reserved.
+ *
+ * MIT License
+ * Permission is hereby granted, free of charge, to any person obtaining a copy of this
+ * software and associated documentation files (the ""Software""), to deal in the Software
+ * without restriction, including without limitation the rights to use, copy, modify, merge,
+ * publish, distribute, sublicense, and/or sell copies of the Software, and to permit
+ * persons to whom the Software is furnished to do so, subject to the following conditions:
+ * The above copyright notice and this permission notice shall be included in all copies or
+ * substantial portions of the Software.
+ * THE SOFTWARE IS PROVIDED *AS IS*, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED,
+ * INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR
+ * PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE
+ * FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
+ * OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+ * DEALINGS IN THE SOFTWARE.
+ */
+
+package com.microsoft.applicationinsights.agent.internal;
+
+import java.io.File;
+import java.nio.file.Path;
+
+import com.google.common.io.Resources;
+import com.microsoft.applicationinsights.TelemetryConfiguration;
+import com.microsoft.applicationinsights.agent.internal.sampling.SamplingPercentage;
+import io.opentelemetry.sdk.OpenTelemetrySdk;
+import io.opentelemetry.sdk.trace.config.TraceConfig;
+import org.junit.*;
+import org.junit.contrib.java.lang.system.*;
+
+import static org.junit.Assert.*;
+
+public class JsonConfigPollingTest {
+
+    @Rule
+    public EnvironmentVariables envVars = new EnvironmentVariables();
+
+    @AfterClass
+    public static void tearDown() {
+        // need to reset trace config back to default (with default sampler)
+        // otherwise tests run after this can fail
+        OpenTelemetrySdk.getGlobalTracerManagement().updateActiveTraceConfig(TraceConfig.getDefault());
+    }
+
+    @Test
+    public void shouldUpdate() {
+        // given
+        TelemetryConfiguration.getActive().setConnectionString("InstrumentationKey=00000000-0000-0000-0000-000000000000");
+        Global.setSamplingPercentage(SamplingPercentage.roundToNearest(90));
+
+        // when
+        Path path = new File(Resources.getResource("applicationinsights.json").getPath()).toPath();
+        new JsonConfigPolling(path, 0, 90).run();
+
+        // then
+        assertEquals("InstrumentationKey=11111111-1111-1111-1111-111111111111", TelemetryConfiguration.getActive().getConnectionString());
+        assertEquals(Global.getSamplingPercentage(), 10, 0);
+    }
+
+    @Test
+    public void shouldNotUpdate() {
+        // given
+        TelemetryConfiguration.getActive().setConnectionString("InstrumentationKey=00000000-0000-0000-0000-000000000000");
+        Global.setSamplingPercentage(SamplingPercentage.roundToNearest(90));
+        envVars.set("APPLICATIONINSIGHTS_CONNECTION_STRING", "InstrumentationKey=00000000-0000-0000-0000-000000000000");
+        envVars.set("APPLICATIONINSIGHTS_SAMPLING_PERCENTAGE", "90");
+
+        // when
+        Path path = new File(Resources.getResource("applicationinsights.json").getPath()).toPath();
+        new JsonConfigPolling(path, 0, 90).run();
+
+        // then
+        // FIXME uncomment this after https://github.com/microsoft/ApplicationInsights-Java/pull/1431 is merged
+        // assertEquals("InstrumentationKey=00000000-0000-0000-0000-000000000000", TelemetryConfiguration.getActive().getConnectionString());
+        assertEquals(Global.getSamplingPercentage(), SamplingPercentage.roundToNearest(90), 0);
+    }
+}

agent/agent-tooling/src/test/resources/applicationinsights.json

@@ -0,0 +1,6 @@
+{
+  "connectionString": "InstrumentationKey=11111111-1111-1111-1111-111111111111",
+  "sampling": {
+    "percentage": 10
+  }
+}