Configuration file related changes (#1459)

* Capturing all exceptions even if config file failed to load * fix spotbugs issues * fix json file used in datascrubbing smoke test
microsoft · Jan 26, 2021 · 93ee092 · 93ee092
1 parent 5c7ed36
commit 93ee092
Show file tree

Hide file tree

Showing 7 changed files with 94 additions and 20 deletions.
diff --git a/agent/agent-bootstrap/spotbugs.exclude.xml b/agent/agent-bootstrap/spotbugs.exclude.xml
@@ -22,6 +22,11 @@
                 <Class name="com.microsoft.applicationinsights.agent.bootstrap.MainEntryPoint" />
                 <Method name="start"/>
             </And>
+            <And>
+                <!-- False Positive. Path is not from an external source. -->
+                <Class name="com.microsoft.applicationinsights.agent.bootstrap.MainEntryPoint" />
+                <Method name="logErrorMessage"/>
+            </And>
         </Or>
     </Match>
     <Match>

diff --git a/...strap/src/main/java/com/microsoft/applicationinsights/agent/bootstrap/MainEntryPoint.java b/...strap/src/main/java/com/microsoft/applicationinsights/agent/bootstrap/MainEntryPoint.java
@@ -87,9 +87,9 @@ public static void start(Instrumentation instrumentation, URL bootstrapURL) {
             FriendlyException friendlyException = getFriendlyException(t);
             String banner = "ApplicationInsights Java Agent " + version + " failed to start";
             if (friendlyException != null) {
-                logErrorMessage(startupLogger, friendlyException.getMessageWithBanner(banner), true, t);
+                logErrorMessage(startupLogger, friendlyException.getMessageWithBanner(banner), true, t, bootstrapURL);
             } else {
-                logErrorMessage(startupLogger, banner, false, t);
+                logErrorMessage(startupLogger, banner, false, t, bootstrapURL);
             }
 
         } finally {
@@ -118,18 +118,31 @@ static FriendlyException getFriendlyException(Throwable t) {
         return getFriendlyException(cause);
     }
 
-    private static void logErrorMessage(Logger startupLogger, String message, boolean isFriendlyException, Throwable t) {
+    private static void logErrorMessage(Logger startupLogger, String message, boolean isFriendlyException, Throwable t, URL bootstrapURL) {
+
         if (startupLogger != null) {
             if (isFriendlyException) {
                 startupLogger.error(message);
             } else {
                 startupLogger.error(message, t);
             }
         } else {
-            if (isFriendlyException) {
-                System.err.println(message);
-            } else {
-                t.printStackTrace();
+            try {
+                // IF the startupLogger failed to be initialized due to configuration syntax error, try initializing it here
+                Path agentPath = new File(bootstrapURL.toURI()).toPath();
+                startupLogger = configureLogging(new SelfDiagnostics(), agentPath);
+                if (isFriendlyException) {
+                    startupLogger.error(message);
+                } else {
+                    startupLogger.error(message, t);
+                }
+            } catch (Throwable e) {
+                // If the startupLogger still have some issues being initialized, just print the error stack trace
+                if (isFriendlyException) {
+                    System.err.println(message);
+                } else {
+                    t.printStackTrace();
+                }
             }
         }
     }

diff --git a/...com/microsoft/applicationinsights/agent/bootstrap/configuration/ConfigurationBuilder.java b/...com/microsoft/applicationinsights/agent/bootstrap/configuration/ConfigurationBuilder.java
@@ -288,7 +288,7 @@ public static Configuration loadJsonConfigFile(Path configPath) throws IOExcepti
         long lastModifiedTime = attributes.lastModifiedTime().toMillis();
         try (InputStream in = Files.newInputStream(configPath)) {
             Moshi moshi = MoshiBuilderFactory.createBuilderWithAdaptor();
-            JsonAdapter<Configuration> jsonAdapter = moshi.adapter(Configuration.class);
+            JsonAdapter<Configuration> jsonAdapter = moshi.adapter(Configuration.class).failOnUnknown();
             Buffer buffer = new Buffer();
             buffer.readFrom(in);
             try {

diff --git a/...va/com/microsoft/applicationinsights/agent/bootstrap/configuration/ConfigurationTest.java b/...va/com/microsoft/applicationinsights/agent/bootstrap/configuration/ConfigurationTest.java
@@ -15,6 +15,7 @@
 import com.microsoft.applicationinsights.agent.bootstrap.configuration.Configuration.ProcessorMatchType;
 import com.microsoft.applicationinsights.agent.bootstrap.configuration.Configuration.ProcessorType;
 import com.squareup.moshi.JsonAdapter;
+import com.squareup.moshi.JsonDataException;
 import com.squareup.moshi.JsonReader;
 import com.squareup.moshi.Moshi;
 import com.squareup.moshi.Types;
@@ -36,7 +37,7 @@ private static Configuration loadConfiguration() throws IOException {
     private static Configuration loadConfiguration(String resourceName) throws IOException {
         CharSource json = Resources.asCharSource(Resources.getResource(resourceName), Charsets.UTF_8);
         Moshi moshi = MoshiBuilderFactory.createBuilderWithAdaptor();
-        JsonAdapter<Configuration> jsonAdapter = moshi.adapter(Configuration.class);
+        JsonAdapter<Configuration> jsonAdapter = moshi.adapter(Configuration.class).failOnUnknown();
         return jsonAdapter.fromJson(json.read());
     }
 
@@ -71,12 +72,8 @@ public void shouldParse() throws IOException {
     @Test
     public void shouldParseProcessorConfiguration() throws IOException {
 
-        CharSource json = Resources.asCharSource(Resources.getResource("ApplicationInsights_SpanProcessor.json"), Charsets.UTF_8);
-        Moshi moshi = MoshiBuilderFactory.createBuilderWithAdaptor();
-        JsonAdapter<Configuration> jsonAdapter = moshi.adapter(Configuration.class);
-        Configuration configuration = jsonAdapter.fromJson(json.read());
+        Configuration configuration = loadConfiguration("ApplicationInsights_SpanProcessor.json");
         PreviewConfiguration preview = configuration.preview;
-
         assertEquals("InstrumentationKey=00000000-0000-0000-0000-000000000000", configuration.connectionString);
         assertEquals(8, preview.processors.size());
         // insert config test
@@ -282,6 +279,11 @@ public void shouldOverrideJmxMetrics() throws IOException {
         assertEquals(configuration.jmxMetrics.get(2).name, "Current Thread Count");
     }
 
+    @Test(expected = JsonDataException.class)
+    public void shouldNotParseFaultyJson() throws IOException {
+        Configuration configuration = loadConfiguration("applicationinsights_faulty.json");
+    }
+
     private List<JmxMetric> parseJmxMetricsJson(String json) throws IOException {
         Moshi moshi = MoshiBuilderFactory.createBasicBuilder();
         Type listOfJmxMetrics = Types.newParameterizedType(List.class, JmxMetric.class);

diff --git a/agent/agent-bootstrap/src/test/resources/ApplicationInsights_SpanProcessor.json b/agent/agent-bootstrap/src/test/resources/ApplicationInsights_SpanProcessor.json
@@ -1,6 +1,5 @@
 {
     "connectionString": "InstrumentationKey=00000000-0000-0000-0000-000000000000",
-    "roleName": "Something Good",
     "preview": {
       "processors": [
         {

diff --git a/agent/agent-bootstrap/src/test/resources/applicationinsights_faulty.json b/agent/agent-bootstrap/src/test/resources/applicationinsights_faulty.json
@@ -0,0 +1,59 @@
+{
+  "connectionString": "InstrumentationKey=00000000-0000-0000-0000-000000000000",
+  "role": {
+    "name": "Something Good",
+    "instance": "xyz123"
+  },
+  "customDimensions": {
+    "some key": "abc",
+    "another key": "def"
+  },
+  "sampling": {
+    "percentage": 10
+  },
+  "instrumentation": {
+    "jdbc": {
+      "enabled": false
+    },
+    "logging": {
+      "enabled": true,
+      "level": "error"
+    },
+    "micrometer": {
+      "enabled": true
+    }
+  },
+  "jmxMetrics": [
+    {
+      "name": "Thread Count",
+      "objectName": "java.lang:type=Threading",
+      "attribute": "ThreadCount"
+    },
+    {
+      "name": "Initial Heap Memory",
+      "objectName": "java.lang:type=Memory",
+      "attribute": "HeapMemoryUsage.init"
+    },
+    {
+      "name": "Current Class Loaded Count",
+      "objectName": "java.lang:type=ClassLoading",
+      "attribute": "LoadedClassCount"
+    }
+  ],
+  "heartbeat": {
+    "intervalSeconds": 60
+  },
+  "httpProxy": {
+    "host": "myproxy",
+    "port": 8080
+  },
+  "selfDiagnostics": {
+    "level": "debug",
+    "destination": "file",
+    "file": {
+      "path": "/var/log/applicationinsights/abc.log",
+      "maxSizeMb": 10,
+      "maxHistory": 2
+    }
+  }
+}
diff --git a/test/smoke/appServers/global-resources/spanprocessor_applicationinsights.json b/test/smoke/appServers/global-resources/spanprocessor_applicationinsights.json
@@ -1,7 +1,6 @@
 {
     "connectionString": "InstrumentationKey=00000000-0000-0000-0000-0FEEDDADBEEF;IngestionEndpoint=http://fakeingestion:60606/",
     "preview": {
-      "roleName": "Something Good",
       "processors" : [
         {
           "type": "attribute",
@@ -79,10 +78,7 @@
             }
           ]
         }
-      ],
-      "selfDiagnostics": {
-        "level": "debug"
-      }
+      ]
     }
   }