Merge pull request #1177 from microsoft/littleaj/lgtm_fixes

littleaj · web-flow · commit c243b8600dd9 · 2019-12-12T16:10:22.000-08:00
Fix bugs from static analysis
diff --git a/agent/src/main/java/com/microsoft/applicationinsights/agent/internal/config/builder/XmlAgentConfigurationBuilder.java b/agent/src/main/java/com/microsoft/applicationinsights/agent/internal/config/builder/XmlAgentConfigurationBuilder.java
@@ -239,20 +239,22 @@ private void addMethods(ClassInstrumentationData classData, Element classNode) {
 
     private Element getTopTag(File configurationFile) throws ParserConfigurationException, IOException, SAXException {
         DocumentBuilder builder = createDocumentBuilder();
-        Document doc = builder.parse(new FileInputStream(configurationFile));
-        doc.getDocumentElement().normalize();
+        try (final FileInputStream fis = new FileInputStream(configurationFile)) {
+            Document doc = builder.parse(fis);
+            doc.getDocumentElement().normalize();
 
-        NodeList topTags = doc.getElementsByTagName(MAIN_TAG);
-        if (topTags == null || topTags.getLength() == 0) {
-            return null;
-        }
+            NodeList topTags = doc.getElementsByTagName(MAIN_TAG);
+            if (topTags == null || topTags.getLength() == 0) {
+                return null;
+            }
 
-        Node topNodeTag = topTags.item(0);
-        if (topNodeTag.getNodeType() != Node.ELEMENT_NODE) {
-            return null;
-        }
+            Node topNodeTag = topTags.item(0);
+            if (topNodeTag.getNodeType() != Node.ELEMENT_NODE) {
+                return null;
+            }
 
-        return (Element) topNodeTag;
+            return (Element) topNodeTag;
+        }
     }
 
     private DocumentBuilder createDocumentBuilder() throws ParserConfigurationException {
diff --git a/azure-application-insights-spring-boot-starter/src/main/java/com/microsoft/applicationinsights/autoconfigure/HeartBeatProvider/SpringBootHeartBeatProvider.java b/azure-application-insights-spring-boot-starter/src/main/java/com/microsoft/applicationinsights/autoconfigure/HeartBeatProvider/SpringBootHeartBeatProvider.java
@@ -88,8 +88,9 @@ public Boolean call() {
             }
           }
           catch (Exception e) {
-            InternalLogger.INSTANCE.warn("Failed to obtain heartbeat property, stack trace"
-                + "is: %s", ExceptionUtils.getStackTrace(e));
+            if (InternalLogger.INSTANCE.isWarnEnabled()) {
+              InternalLogger.INSTANCE.warn("Failed to obtain heartbeat property: %s", ExceptionUtils.getStackTrace(e));
+            }
           }
         }
         return hasSetValues;
diff --git a/core/src/main/java/com/microsoft/applicationinsights/TelemetryClient.java b/core/src/main/java/com/microsoft/applicationinsights/TelemetryClient.java
@@ -520,16 +520,19 @@ private TelemetryContext createInitializedContext() {
             ctx.getCloud().setRole(roleName);
         }
         for (ContextInitializer init : configuration.getContextInitializers()) {
+            if (init == null) { // since collection reference is exposed, we need a null check here
+                InternalLogger.INSTANCE.warn("Found null ContextInitializer in configuration. Skipping...");
+                continue;
+            }
+
             try {
                 init.initialize(ctx);
             } catch (ThreadDeath td) {
                 throw td;
             } catch (Throwable t) {
                 try {
                     if (InternalLogger.INSTANCE.isErrorEnabled()) {
-                        InternalLogger.INSTANCE.error("Exception in context initializer%s: %s",
-                                init == null ? " (context initializer is null)" : ", "+init.getClass().getSimpleName(),
-                                ExceptionUtils.getStackTrace(t));
+                        InternalLogger.INSTANCE.error("Exception in context initializer, %s: %s", init.getClass().getSimpleName(), ExceptionUtils.getStackTrace(t));
                     }
                 } catch (ThreadDeath td) {
                     throw td;
diff --git a/core/src/main/java/com/microsoft/applicationinsights/internal/channel/common/SenderThreadLocalBackOffData.java b/core/src/main/java/com/microsoft/applicationinsights/internal/channel/common/SenderThreadLocalBackOffData.java
@@ -53,9 +53,9 @@ final class SenderThreadLocalBackOffData {
      * @param addMilliseconds The amount of seconds that will be added to the 'large' intervals to distinct between sender threads.
      */
     public SenderThreadLocalBackOffData(long[] backOffTimeoutsInMillis, long addMilliseconds) {
-        Preconditions.checkNotNull(backOffTimeoutsInMillis, "backOffTimeoutsInSeconds must be not null");
-        Preconditions.checkArgument(backOffTimeoutsInMillis.length > 0, "backOffTimeoutsInSeconds must not be empty");
-        Preconditions.checkArgument(addMilliseconds >= 0, "addSeconds must not be >= 0");
+        Preconditions.checkNotNull(backOffTimeoutsInMillis, "backOffTimeoutsInMillis must be not null");
+        Preconditions.checkArgument(backOffTimeoutsInMillis.length > 0, "backOffTimeoutsInMillis must not be empty");
+        Preconditions.checkArgument(addMilliseconds >= 0, "addMilliseconds must not be >= 0");
 
         currentBackOffIndex = -1;
         instanceIsActive = true;
diff --git a/core/src/main/java/com/microsoft/applicationinsights/internal/channel/common/SenderThreadsBackOffManager.java b/core/src/main/java/com/microsoft/applicationinsights/internal/channel/common/SenderThreadsBackOffManager.java
@@ -83,8 +83,7 @@ public synchronized void stopAllSendersBackOffActivities() {
 
     @Override
     protected SenderThreadLocalBackOffData initialValue() {
-        int addSeconds = threadsSecondsDifference.incrementAndGet();
-        senderThreadLocalData = new SenderThreadLocalBackOffData(backOffTimeoutsInMilliseconds, addSeconds * 1000);
+        senderThreadLocalData = new SenderThreadLocalBackOffData(backOffTimeoutsInMilliseconds, threadsSecondsDifference.incrementAndGet() * 1000L);
         registerSenderData(senderThreadLocalData);
         return senderThreadLocalData;
     }
diff --git a/core/src/main/java/com/microsoft/applicationinsights/internal/channel/common/TransmissionFileSystemOutput.java b/core/src/main/java/com/microsoft/applicationinsights/internal/channel/common/TransmissionFileSystemOutput.java
@@ -109,7 +109,7 @@ public TransmissionFileSystemOutput(String folderPath, String maxTransmissionSto
                                                                         DEFAULT_CAPACITY_MEGABYTES,
                                                                         MAX_TRANSMISSION_STORAGE_CAPACITY_NAME,
                                                                         maxTransmissionStorageCapacity);
-        capacityInBytes = capacityEnforcer.getCurrentValue() * 1024 * 1024;
+        capacityInBytes = capacityEnforcer.getCurrentValue() * 1024L * 1024L;
 
         folder = new File(folderPath);
 
@@ -212,7 +212,7 @@ public Transmission fetchOldestFile() {
     }
 
     public void setCapacity(int suggestedCapacity) {
-        this.capacityInBytes = capacityEnforcer.normalizeValue(suggestedCapacity) * 1024 * 1024;
+        this.capacityInBytes = capacityEnforcer.normalizeValue(suggestedCapacity) * 1024L * 1024L;
     }
 
     private List<File> sortOldestLastAndTrim(Collection<File> transmissions, int limit) {
diff --git a/core/src/main/java/com/microsoft/applicationinsights/internal/channel/sampling/FixedRateTelemetrySampler.java b/core/src/main/java/com/microsoft/applicationinsights/internal/channel/sampling/FixedRateTelemetrySampler.java
@@ -89,7 +89,7 @@ public void setSamplingPercentage(Double samplingPercentage) {
 
     @Override
     public boolean isSampledIn(Telemetry telemetry) {
-        Double currentSamplingPercentage = samplingPercentage.get();
+        double currentSamplingPercentage = samplingPercentage.get();
 
         if (currentSamplingPercentage < 100.0 - 1.0E-12) {
             if (telemetry instanceof SupportSampling) {
diff --git a/core/src/main/java/com/microsoft/applicationinsights/internal/heartbeat/DefaultHeartBeatPropertyProvider.java b/core/src/main/java/com/microsoft/applicationinsights/internal/heartbeat/DefaultHeartBeatPropertyProvider.java
@@ -97,8 +97,9 @@ public Boolean call() {
             }
           }
           catch (Exception e) {
-           InternalLogger.INSTANCE.warn("Failed to obtain heartbeat property, stack trace"
-               + "is: %s", ExceptionUtils.getStackTrace(e));
+            if (InternalLogger.INSTANCE.isWarnEnabled()) {
+              InternalLogger.INSTANCE.warn("Failed to obtain heartbeat property: %s", ExceptionUtils.getStackTrace(e));
+            }
           }
         }
         return hasSetValues;
diff --git a/core/src/main/java/com/microsoft/applicationinsights/internal/heartbeat/WebAppsHeartbeatProvider.java b/core/src/main/java/com/microsoft/applicationinsights/internal/heartbeat/WebAppsHeartbeatProvider.java
@@ -107,8 +107,9 @@ public Boolean call() {
             }
           }
           catch (Exception e) {
-            InternalLogger.INSTANCE.warn("Failed to obtain heartbeat property, stack trace"
-                + "is: %s", ExceptionUtils.getStackTrace(e));
+            if (InternalLogger.INSTANCE.isWarnEnabled()) {
+              InternalLogger.INSTANCE.warn("Failed to obtain heartbeat property: %s", ExceptionUtils.getStackTrace(e));
+            }
           }
         }
         return hasSetValues;
diff --git a/core/src/main/java/com/microsoft/applicationinsights/internal/perfcounter/JniPCConnector.java b/core/src/main/java/com/microsoft/applicationinsights/internal/perfcounter/JniPCConnector.java
@@ -209,12 +209,10 @@ private static void extractToLocalFolder(File dllOnDisk, String libraryToLoad) t
 
             InternalLogger.INSTANCE.trace("Successfully extracted '%s' to local folder", libraryToLoad);
         } finally {
-            if (in != null) {
-                try {
-                    in.close();
-                } catch (IOException e) {
-                    InternalLogger.INSTANCE.error("Failed to close input stream for dll extraction: %s", e.toString());
-                }
+            try {
+                in.close();
+            } catch (IOException e) {
+                InternalLogger.INSTANCE.error("Failed to close input stream for dll extraction: %s", e.toString());
             }
             if (out != null) {
                 try {
diff --git a/core/src/main/java/com/microsoft/applicationinsights/internal/shutdown/SDKShutdownActivity.java b/core/src/main/java/com/microsoft/applicationinsights/internal/shutdown/SDKShutdownActivity.java
@@ -24,7 +24,6 @@
 import java.io.Closeable;
 import java.util.List;
 import java.util.ArrayList;
-import java.util.concurrent.Executor;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.TimeUnit;
 
@@ -214,15 +213,17 @@ public void stopAll() {
 
     private SDKShutdownAction getShutdownAction() {
         if (shutdownAction == null) {
-            synchronized (this) {
+            synchronized (INSTANCE) {
                 if (shutdownAction == null) {
                     try {
-                        shutdownAction = new SDKShutdownAction();
-                        Thread t = new Thread(shutdownAction, SDKShutdownActivity.class.getSimpleName()+"-ShutdownHook");
+                        SDKShutdownAction action = new SDKShutdownAction();
+                        Thread t = new Thread(action, SDKShutdownActivity.class.getSimpleName()+"-ShutdownHook");
                         Runtime.getRuntime().addShutdownHook(t);
+                        shutdownAction = action;
                     } catch (Exception e) {
-                        InternalLogger.INSTANCE.error("Error while adding shutdown hook in getShutDownThread call");
-                        InternalLogger.INSTANCE.trace("Stack trace generated is %s", ExceptionUtils.getStackTrace(e));
+                        if (InternalLogger.INSTANCE.isErrorEnabled()) {
+                            InternalLogger.INSTANCE.error("Error while adding shutdown hook in getShutDownThread call: %s", ExceptionUtils.getStackTrace(e));
+                        }
                     }
                 }
             }
diff --git a/core/src/main/java/com/microsoft/applicationinsights/internal/system/SystemInformation.java b/core/src/main/java/com/microsoft/applicationinsights/internal/system/SystemInformation.java
@@ -36,10 +36,9 @@ public enum SystemInformation {
 
     private final static String DEFAULT_PROCESS_NAME = "Java_Process";
 
-    private String processId;
+    private final String processId = initializeProcessId();
 
     public String getProcessId() {
-        setProcessId();
         return processId;
     }
 
@@ -51,27 +50,22 @@ public boolean isUnix() {
         return SystemUtils.IS_OS_UNIX;
     }
 
-    private synchronized void setProcessId() {
-        if (!Strings.isNullOrEmpty(processId)) {
-            return;
-        }
-
+    private String initializeProcessId() {
         String rawName = ManagementFactory.getRuntimeMXBean().getName();
         if (!Strings.isNullOrEmpty(rawName)) {
             int i = rawName.indexOf("@");
             if (i != -1) {
                 String processIdAsString = rawName.substring(0, i);
                 try {
                     Integer.parseInt(processIdAsString);
-                    processId = processIdAsString;
-                    return;
+                    return processIdAsString;
                 } catch (Exception e) {
                     InternalLogger.INSTANCE.error("Failed to fetch process id: '%s'", e.toString());
                 }
             }
         }
 
         // Default
-        processId = DEFAULT_PROCESS_NAME;
+        return DEFAULT_PROCESS_NAME;
     }
 }
diff --git a/core/src/main/java/com/microsoft/applicationinsights/telemetry/Duration.java b/core/src/main/java/com/microsoft/applicationinsights/telemetry/Duration.java
@@ -127,10 +127,10 @@ public int getMilliseconds() {
      * @return The total milliseconds of the duration.
      */
     public long getTotalMilliseconds() {
-        return  (days * SECONDS_IN_ONE_DAY * 1000) +
-                (hours * SECONDS_IN_ONE_HOUR * 1000) +
-                (minutes * SECONDS_IN_ONE_MINUTE * 1000) +
-                (seconds * 1000) + milliseconds;
+        return  (days * SECONDS_IN_ONE_DAY * 1000L) +
+                (hours * SECONDS_IN_ONE_HOUR * 1000L) +
+                (minutes * SECONDS_IN_ONE_MINUTE * 1000L) +
+                (seconds * 1000L) + milliseconds;
     }
 
     @Override
diff --git a/web/src/main/java/com/microsoft/applicationinsights/web/extensibility/initializers/WebOperationNameTelemetryInitializer.java b/web/src/main/java/com/microsoft/applicationinsights/web/extensibility/initializers/WebOperationNameTelemetryInitializer.java
@@ -50,14 +50,14 @@ protected void onInitializeTelemetry(Telemetry telemetry) {
     // region Private
 
     private void updateRequestNameIfRequestTelemetry(Telemetry telemetry, String operationName) {
-        if (!(telemetry instanceof RequestTelemetry)) {
+        if (!(telemetry instanceof RequestTelemetry)) { // null instanceof Object == false
             return;
         }
 
         RequestTelemetry requestTelemetry = (RequestTelemetry)telemetry;
 
         // We only update the request telemetry name if not already provided by the user.
-        if (requestTelemetry != null && CommonUtils.isNullOrEmpty(requestTelemetry.getName())) {
+        if (CommonUtils.isNullOrEmpty(requestTelemetry.getName())) {
             requestTelemetry.setName(operationName);
         }
     }

Original file line number	Diff line number	Diff line change
`@@ -88,8 +88,9 @@ public Boolean call() {`
`88`	`88`	`}`
`89`	`89`	`}`
`90`	`90`	`catch (Exception e) {`
`91`		`- InternalLogger.INSTANCE.warn("Failed to obtain heartbeat property, stack trace"`
`92`		`- + "is: %s", ExceptionUtils.getStackTrace(e));`
	`91`	`+ if (InternalLogger.INSTANCE.isWarnEnabled()) {`
	`92`	`+ InternalLogger.INSTANCE.warn("Failed to obtain heartbeat property: %s", ExceptionUtils.getStackTrace(e));`
	`93`	`+ }`
`93`	`94`	`}`
`94`	`95`	`}`
`95`	`96`	`return hasSetValues;`
Original file line number	Diff line number	Diff line change
`@@ -83,8 +83,7 @@ public synchronized void stopAllSendersBackOffActivities() {`
`83`	`83`
`84`	`84`	`@Override`
`85`	`85`	`protected SenderThreadLocalBackOffData initialValue() {`
`86`		`- int addSeconds = threadsSecondsDifference.incrementAndGet();`
`87`		`- senderThreadLocalData = new SenderThreadLocalBackOffData(backOffTimeoutsInMilliseconds, addSeconds * 1000);`
	`86`	`+ senderThreadLocalData = new SenderThreadLocalBackOffData(backOffTimeoutsInMilliseconds, threadsSecondsDifference.incrementAndGet() * 1000L);`
`88`	`87`	`registerSenderData(senderThreadLocalData);`
`89`	`88`	`return senderThreadLocalData;`
`90`	`89`	`}`
Original file line number	Diff line number	Diff line change
`@@ -109,7 +109,7 @@ public TransmissionFileSystemOutput(String folderPath, String maxTransmissionSto`
`109`	`109`	`DEFAULT_CAPACITY_MEGABYTES,`
`110`	`110`	`MAX_TRANSMISSION_STORAGE_CAPACITY_NAME,`
`111`	`111`	`maxTransmissionStorageCapacity);`
`112`		`- capacityInBytes = capacityEnforcer.getCurrentValue() * 1024 * 1024;`
	`112`	`+ capacityInBytes = capacityEnforcer.getCurrentValue() * 1024L * 1024L;`
`113`	`113`
`114`	`114`	`folder = new File(folderPath);`
`115`	`115`
`@@ -212,7 +212,7 @@ public Transmission fetchOldestFile() {`
`212`	`212`	`}`
`213`	`213`
`214`	`214`	`public void setCapacity(int suggestedCapacity) {`
`215`		`- this.capacityInBytes = capacityEnforcer.normalizeValue(suggestedCapacity) * 1024 * 1024;`
	`215`	`+ this.capacityInBytes = capacityEnforcer.normalizeValue(suggestedCapacity) * 1024L * 1024L;`
`216`	`216`	`}`
`217`	`217`
`218`	`218`	`private List<File> sortOldestLastAndTrim(Collection<File> transmissions, int limit) {`
Original file line number	Diff line number	Diff line change
`@@ -97,8 +97,9 @@ public Boolean call() {`
`97`	`97`	`}`
`98`	`98`	`}`
`99`	`99`	`catch (Exception e) {`
`100`		`- InternalLogger.INSTANCE.warn("Failed to obtain heartbeat property, stack trace"`
`101`		`- + "is: %s", ExceptionUtils.getStackTrace(e));`
	`100`	`+ if (InternalLogger.INSTANCE.isWarnEnabled()) {`
	`101`	`+ InternalLogger.INSTANCE.warn("Failed to obtain heartbeat property: %s", ExceptionUtils.getStackTrace(e));`
	`102`	`+ }`
`102`	`103`	`}`
`103`	`104`	`}`
`104`	`105`	`return hasSetValues;`
Original file line number	Diff line number	Diff line change
`@@ -107,8 +107,9 @@ public Boolean call() {`
`107`	`107`	`}`
`108`	`108`	`}`
`109`	`109`	`catch (Exception e) {`
`110`		`- InternalLogger.INSTANCE.warn("Failed to obtain heartbeat property, stack trace"`
`111`		`- + "is: %s", ExceptionUtils.getStackTrace(e));`
	`110`	`+ if (InternalLogger.INSTANCE.isWarnEnabled()) {`
	`111`	`+ InternalLogger.INSTANCE.warn("Failed to obtain heartbeat property: %s", ExceptionUtils.getStackTrace(e));`
	`112`	`+ }`
`112`	`113`	`}`
`113`	`114`	`}`
`114`	`115`	`return hasSetValues;`