Fix connection string env var precedence over ${file:...} config lookup

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

@@ -40,6 +40,7 @@
 import java.util.Locale;
 import java.util.Map;
 import java.util.function.Function;
+import java.util.function.Supplier;
 import javax.annotation.Nullable;
 import org.apache.commons.text.StringSubstitutor;
 import org.apache.commons.text.lookup.StringLookup;
@@ -715,16 +716,9 @@ static void overlayFromEnv(
       Function<String, String> envVarsFunction,
       Function<String, String> systemPropertiesFunction)
       throws IOException {
-    // load connection string from a file if connection string is in the format of
-    // "${file:mounted_connection_string_file.txt}"
-    Map<String, StringLookup> stringLookupMap =
-        Collections.singletonMap(StringLookupFactory.KEY_FILE, new FileStringLookup(baseDir));
-    StringLookup stringLookup =
-        StringLookupFactory.INSTANCE.interpolatorStringLookup(stringLookupMap, null, false);
-    StringSubstitutor stringSubstitutor = new StringSubstitutor(stringLookup);
     config.connectionString =
         overlayConnectionStringFromEnv(
-            stringSubstitutor.replace(config.connectionString),
+            () -> resolveFileLookup(config.connectionString, baseDir),
             envVarsFunction,
             systemPropertiesFunction);
 
@@ -828,27 +822,35 @@ public static void overlayFromEnv(
       Function<String, String> systemPropertiesFunction) {
     config.connectionString =
         overlayConnectionStringFromEnv(
-            config.connectionString, envVarsFunction, systemPropertiesFunction);
+            () -> config.connectionString, envVarsFunction, systemPropertiesFunction);
     config.sampling.percentage =
         overlayWithEnvVar(
             APPLICATIONINSIGHTS_SAMPLING_PERCENTAGE, config.sampling.percentage, envVarsFunction);
   }
 
+  private static String resolveFileLookup(String value, Path baseDir) {
+    Map<String, StringLookup> stringLookupMap =
+        Collections.singletonMap(StringLookupFactory.KEY_FILE, new FileStringLookup(baseDir));
+    StringLookup stringLookup =
+        StringLookupFactory.INSTANCE.interpolatorStringLookup(stringLookupMap, null, false);
+    StringSubstitutor stringSubstitutor = new StringSubstitutor(stringLookup);
+    return stringSubstitutor.replace(value);
+  }
+
   @Nullable
   private static String overlayConnectionStringFromEnv(
-      String connectionString,
+      Supplier<String> connectionStringSupplier,
       Function<String, String> envVarsFunction,
       Function<String, String> systemPropertiesFunction) {
-    String value =
+    String connectionString =
         overlayWithSysPropEnvVar(
             APPLICATIONINSIGHTS_CONNECTION_STRING_SYS,
             APPLICATIONINSIGHTS_CONNECTION_STRING_ENV,
-            connectionString,
+            connectionStringSupplier,
             envVarsFunction,
             systemPropertiesFunction);
-
-    if (value != null) {
-      return value;
+    if (connectionString != null) {
+      return connectionString;
     }
 
     // this is for backwards compatibility only
@@ -971,12 +973,31 @@ public static String overlayWithSysPropEnvVar(
       String defaultValue,
       Function<String, String> envVarsFunction,
       Function<String, String> systemPropertiesFunction) {
+    return overlayWithSysPropEnvVar(
+        systemPropertyName,
+        envVarName,
+        () -> defaultValue,
+        envVarsFunction,
+        systemPropertiesFunction);
+  }
+
+  @Nullable
+  static String overlayWithSysPropEnvVar(
+      String systemPropertyName,
+      String envVarName,
+      Supplier<String> defaultValueSupplier,
+      Function<String, String> envVarsFunction,
+      Function<String, String> systemPropertiesFunction) {
     String value = getSystemProperty(systemPropertyName, systemPropertiesFunction);
     if (value != null) {
       configurationLogger.debug("using system property: {}", systemPropertyName);
       return value;
     }
-    return overlayWithEnvVar(envVarName, defaultValue, envVarsFunction);
+    String envVarValue = getEnvVar(envVarName, envVarsFunction);
+    if (envVarValue != null) {
+      return envVarValue;
+    }
+    return defaultValueSupplier.get();
   }
 
   public static String overlayWithEnvVar(

settings.gradle.kts

@@ -70,6 +70,8 @@ hideFromDependabot(":smoke-tests:apps:ClassicSdkLogbackInterop2x")
 hideFromDependabot(":smoke-tests:apps:ClassicSdkWebInterop2x")
 hideFromDependabot(":smoke-tests:apps:ClassicSdkWebInterop3x")
 hideFromDependabot(":smoke-tests:apps:ClassicSdkWebInterop3xUsingOld3xAgent")
+hideFromDependabot(":smoke-tests:apps:ConnectionStringFileNotFound")
+hideFromDependabot(":smoke-tests:apps:ConnectionStringFromFile")
 hideFromDependabot(":smoke-tests:apps:ConnectionStringOverrides")
 hideFromDependabot(":smoke-tests:apps:CoreAndFilter2x")
 hideFromDependabot(":smoke-tests:apps:CoreAndFilter3x")

smoke-tests/apps/ConnectionStringFileNotFound/build.gradle.kts

@@ -0,0 +1,3 @@
+plugins {
+  id("ai.smoke-test-war")
+}

smoke-tests/apps/ConnectionStringFileNotFound/src/main/java/com/microsoft/applicationinsights/smoketestapp/ConnectionStringFileNotFoundServlet.java

@@ -0,0 +1,19 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT License.
+
+package com.microsoft.applicationinsights.smoketestapp;
+
+import java.io.IOException;
+import javax.servlet.annotation.WebServlet;
+import javax.servlet.http.HttpServlet;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+
+@WebServlet("/*")
+public class ConnectionStringFileNotFoundServlet extends HttpServlet {
+
+  @Override
+  protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws IOException {
+    resp.getWriter().println("ok");
+  }
+}

smoke-tests/apps/ConnectionStringFileNotFound/src/smokeTest/java/com/microsoft/applicationinsights/smoketest/ConnectionStringFileNotFoundTest.java

@@ -0,0 +1,80 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT License.
+
+package com.microsoft.applicationinsights.smoketest;
+
+import static com.microsoft.applicationinsights.smoketest.EnvironmentValue.TOMCAT_8_JAVA_11;
+import static com.microsoft.applicationinsights.smoketest.EnvironmentValue.TOMCAT_8_JAVA_11_OPENJ9;
+import static com.microsoft.applicationinsights.smoketest.EnvironmentValue.TOMCAT_8_JAVA_17;
+import static com.microsoft.applicationinsights.smoketest.EnvironmentValue.TOMCAT_8_JAVA_17_OPENJ9;
+import static com.microsoft.applicationinsights.smoketest.EnvironmentValue.TOMCAT_8_JAVA_21;
+import static com.microsoft.applicationinsights.smoketest.EnvironmentValue.TOMCAT_8_JAVA_21_OPENJ9;
+import static com.microsoft.applicationinsights.smoketest.EnvironmentValue.TOMCAT_8_JAVA_25;
+import static com.microsoft.applicationinsights.smoketest.EnvironmentValue.TOMCAT_8_JAVA_25_OPENJ9;
+import static com.microsoft.applicationinsights.smoketest.EnvironmentValue.TOMCAT_8_JAVA_8;
+import static com.microsoft.applicationinsights.smoketest.EnvironmentValue.TOMCAT_8_JAVA_8_OPENJ9;
+import static com.microsoft.applicationinsights.smoketest.EnvironmentValue.WILDFLY_13_JAVA_8;
+import static com.microsoft.applicationinsights.smoketest.EnvironmentValue.WILDFLY_13_JAVA_8_OPENJ9;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.data.MapEntry.entry;
+
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.RegisterExtension;
+
+// Reproduces https://github.com/microsoft/ApplicationInsights-Java/issues/4214
+// When applicationinsights.json contains a ${file:...} reference to a nonexistent file,
+// the APPLICATIONINSIGHTS_CONNECTION_STRING env var should still take effect.
+@UseAgent
+abstract class ConnectionStringFileNotFoundTest {
+
+  @RegisterExtension static final SmokeTestExtension testing = SmokeTestExtension.create();
+
+  @Test
+  @TargetUri("/test")
+  void shouldUseEnvVarWhenFileNotFound() throws Exception {
+    Telemetry telemetry = testing.getTelemetry(0);
+
+    assertThat(telemetry.rd.getName()).isEqualTo("GET /ConnectionStringFileNotFound/*");
+    assertThat(telemetry.rd.getResponseCode()).isEqualTo("200");
+    assertThat(telemetry.rd.getSuccess()).isTrue();
+    assertThat(telemetry.rd.getProperties())
+        .containsExactly(entry("_MS.ProcessedByMetricExtractors", "True"));
+    assertThat(telemetry.rd.getMeasurements()).isEmpty();
+  }
+
+  @Environment(TOMCAT_8_JAVA_8)
+  static class Tomcat8Java8Test extends ConnectionStringFileNotFoundTest {}
+
+  @Environment(TOMCAT_8_JAVA_8_OPENJ9)
+  static class Tomcat8Java8OpenJ9Test extends ConnectionStringFileNotFoundTest {}
+
+  @Environment(TOMCAT_8_JAVA_11)
+  static class Tomcat8Java11Test extends ConnectionStringFileNotFoundTest {}
+
+  @Environment(TOMCAT_8_JAVA_11_OPENJ9)
+  static class Tomcat8Java11OpenJ9Test extends ConnectionStringFileNotFoundTest {}
+
+  @Environment(TOMCAT_8_JAVA_17)
+  static class Tomcat8Java17Test extends ConnectionStringFileNotFoundTest {}
+
+  @Environment(TOMCAT_8_JAVA_17_OPENJ9)
+  static class Tomcat8Java17OpenJ9Test extends ConnectionStringFileNotFoundTest {}
+
+  @Environment(TOMCAT_8_JAVA_21)
+  static class Tomcat8Java21Test extends ConnectionStringFileNotFoundTest {}
+
+  @Environment(TOMCAT_8_JAVA_21_OPENJ9)
+  static class Tomcat8Java21OpenJ9Test extends ConnectionStringFileNotFoundTest {}
+
+  @Environment(TOMCAT_8_JAVA_25)
+  static class Tomcat8Java23Test extends ConnectionStringFileNotFoundTest {}
+
+  @Environment(TOMCAT_8_JAVA_25_OPENJ9)
+  static class Tomcat8Java23OpenJ9Test extends ConnectionStringFileNotFoundTest {}
+
+  @Environment(WILDFLY_13_JAVA_8)
+  static class Wildfly13Java8Test extends ConnectionStringFileNotFoundTest {}
+
+  @Environment(WILDFLY_13_JAVA_8_OPENJ9)
+  static class Wildfly13Java8OpenJ9Test extends ConnectionStringFileNotFoundTest {}
+}

smoke-tests/apps/ConnectionStringFileNotFound/src/smokeTest/resources/applicationinsights.json

@@ -0,0 +1,7 @@
+{
+  "connectionString": "${file:/app/secrets/applicationinsights_connection_string}",
+  "role": {
+    "name": "testrolename",
+    "instance": "testroleinstance"
+  }
+}

smoke-tests/apps/ConnectionStringFromFile/build.gradle.kts

@@ -0,0 +1,3 @@
+plugins {
+  id("ai.smoke-test-war")
+}

smoke-tests/apps/ConnectionStringFromFile/src/main/java/com/microsoft/applicationinsights/smoketestapp/ConnectionStringFromFileServlet.java

@@ -0,0 +1,19 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT License.
+
+package com.microsoft.applicationinsights.smoketestapp;
+
+import java.io.IOException;
+import javax.servlet.annotation.WebServlet;
+import javax.servlet.http.HttpServlet;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+
+@WebServlet("/*")
+public class ConnectionStringFromFileServlet extends HttpServlet {
+
+  @Override
+  protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws IOException {
+    resp.getWriter().println("ok");
+  }
+}

smoke-tests/apps/ConnectionStringFromFile/src/smokeTest/java/com/microsoft/applicationinsights/smoketest/ConnectionStringFromFileTest.java

@@ -0,0 +1,84 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT License.
+
+package com.microsoft.applicationinsights.smoketest;
+
+import static com.microsoft.applicationinsights.smoketest.EnvironmentValue.TOMCAT_8_JAVA_11;
+import static com.microsoft.applicationinsights.smoketest.EnvironmentValue.TOMCAT_8_JAVA_11_OPENJ9;
+import static com.microsoft.applicationinsights.smoketest.EnvironmentValue.TOMCAT_8_JAVA_17;
+import static com.microsoft.applicationinsights.smoketest.EnvironmentValue.TOMCAT_8_JAVA_17_OPENJ9;
+import static com.microsoft.applicationinsights.smoketest.EnvironmentValue.TOMCAT_8_JAVA_21;
+import static com.microsoft.applicationinsights.smoketest.EnvironmentValue.TOMCAT_8_JAVA_21_OPENJ9;
+import static com.microsoft.applicationinsights.smoketest.EnvironmentValue.TOMCAT_8_JAVA_25;
+import static com.microsoft.applicationinsights.smoketest.EnvironmentValue.TOMCAT_8_JAVA_25_OPENJ9;
+import static com.microsoft.applicationinsights.smoketest.EnvironmentValue.TOMCAT_8_JAVA_8;
+import static com.microsoft.applicationinsights.smoketest.EnvironmentValue.TOMCAT_8_JAVA_8_OPENJ9;
+import static com.microsoft.applicationinsights.smoketest.EnvironmentValue.WILDFLY_13_JAVA_8;
+import static com.microsoft.applicationinsights.smoketest.EnvironmentValue.WILDFLY_13_JAVA_8_OPENJ9;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.data.MapEntry.entry;
+
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.RegisterExtension;
+
+// Verifies that ${file:...} connection string lookup works correctly when
+// the APPLICATIONINSIGHTS_CONNECTION_STRING env var is NOT set.
+@UseAgent
+abstract class ConnectionStringFromFileTest {
+
+  @RegisterExtension
+  static final SmokeTestExtension testing =
+      SmokeTestExtension.builder()
+          .doNotSetConnectionString()
+          .addFile("connection_string.txt", "/app/secrets/connection_string.txt")
+          .build();
+
+  @Test
+  @TargetUri("/test")
+  void shouldReadConnectionStringFromFile() throws Exception {
+    Telemetry telemetry = testing.getTelemetry(0);
+
+    assertThat(telemetry.rd.getName()).isEqualTo("GET /ConnectionStringFromFile/*");
+    assertThat(telemetry.rd.getResponseCode()).isEqualTo("200");
+    assertThat(telemetry.rd.getSuccess()).isTrue();
+    assertThat(telemetry.rd.getProperties())
+        .containsExactly(entry("_MS.ProcessedByMetricExtractors", "True"));
+    assertThat(telemetry.rd.getMeasurements()).isEmpty();
+  }
+
+  @Environment(TOMCAT_8_JAVA_8)
+  static class Tomcat8Java8Test extends ConnectionStringFromFileTest {}
+
+  @Environment(TOMCAT_8_JAVA_8_OPENJ9)
+  static class Tomcat8Java8OpenJ9Test extends ConnectionStringFromFileTest {}
+
+  @Environment(TOMCAT_8_JAVA_11)
+  static class Tomcat8Java11Test extends ConnectionStringFromFileTest {}
+
+  @Environment(TOMCAT_8_JAVA_11_OPENJ9)
+  static class Tomcat8Java11OpenJ9Test extends ConnectionStringFromFileTest {}
+
+  @Environment(TOMCAT_8_JAVA_17)
+  static class Tomcat8Java17Test extends ConnectionStringFromFileTest {}
+
+  @Environment(TOMCAT_8_JAVA_17_OPENJ9)
+  static class Tomcat8Java17OpenJ9Test extends ConnectionStringFromFileTest {}
+
+  @Environment(TOMCAT_8_JAVA_21)
+  static class Tomcat8Java21Test extends ConnectionStringFromFileTest {}
+
+  @Environment(TOMCAT_8_JAVA_21_OPENJ9)
+  static class Tomcat8Java21OpenJ9Test extends ConnectionStringFromFileTest {}
+
+  @Environment(TOMCAT_8_JAVA_25)
+  static class Tomcat8Java23Test extends ConnectionStringFromFileTest {}
+
+  @Environment(TOMCAT_8_JAVA_25_OPENJ9)
+  static class Tomcat8Java23OpenJ9Test extends ConnectionStringFromFileTest {}
+
+  @Environment(WILDFLY_13_JAVA_8)
+  static class Wildfly13Java8Test extends ConnectionStringFromFileTest {}
+
+  @Environment(WILDFLY_13_JAVA_8_OPENJ9)
+  static class Wildfly13Java8OpenJ9Test extends ConnectionStringFromFileTest {}
+}

smoke-tests/apps/ConnectionStringFromFile/src/smokeTest/resources/applicationinsights.json

@@ -0,0 +1,7 @@
+{
+  "connectionString": "${file:/app/secrets/connection_string.txt}",
+  "role": {
+    "name": "testrolename",
+    "instance": "testroleinstance"
+  }
+}

smoke-tests/apps/ConnectionStringFromFile/src/smokeTest/resources/connection_string.txt

@@ -0,0 +1 @@
+InstrumentationKey=00000000-0000-0000-0000-0FEEDDADBEEF;IngestionEndpoint=http://host.testcontainers.internal:6060/;LiveEndpoint=http://host.testcontainers.internal:6060/