removed usage of -1 as a default value for some measurements (#651)

littleaj · web-flow · commit c3d5eacbefdc · 2018-04-27T17:06:48.000-07:00
* removed usage of -1 as a default value for measurements. This just causes confusion when seen in the cloud. If you can't measure something, don't report it as some garbage value

* fix log message; instead of waiting for N counters, just wait for each kind individually

* added support to delay before calling TargetUri. cleaned up test code
diff --git a/core/src/main/java/com/microsoft/applicationinsights/internal/perfcounter/AbstractUnixPerformanceCounter.java b/core/src/main/java/com/microsoft/applicationinsights/internal/perfcounter/AbstractUnixPerformanceCounter.java
@@ -45,11 +45,11 @@ protected AbstractUnixPerformanceCounter(String path) {
         this.path = path;
         processFile = new File(path);
         if (!processFile.canRead()) {
-            logError("Can not read");
+            logPerfCounterErrorError("Can not read");
         }
     }
 
-    protected void logError(String format, Object... args) {
+    protected void logPerfCounterErrorError(String format, Object... args) {
         format = "Performance Counter " + getId() + ": Error in file '" + path + "': " + format;
         InternalLogger.INSTANCE.error(format, args);
     }
diff --git a/core/src/main/java/com/microsoft/applicationinsights/internal/perfcounter/Constants.java b/core/src/main/java/com/microsoft/applicationinsights/internal/perfcounter/Constants.java
@@ -51,8 +51,6 @@ public final class Constants {
 
     public final static String PROCESS_CATEGORY = "Process";
 
-    public final static double DEFAULT_DOUBLE_VALUE = -1.0;
-
     private Constants() {
     }
 }
diff --git a/core/src/main/java/com/microsoft/applicationinsights/internal/perfcounter/CpuPerformanceCounterCalculator.java b/core/src/main/java/com/microsoft/applicationinsights/internal/perfcounter/CpuPerformanceCounterCalculator.java
@@ -47,8 +47,8 @@ public CpuPerformanceCounterCalculator() {
 
     }
 
-    public double getProcessCpuUsage() {
-        double processCpuUsage;
+    public Double getProcessCpuUsage() {
+        Double processCpuUsage = null;
         try {
             RuntimeMXBean runtimeMXBean = ManagementFactory.getRuntimeMXBean();
 
@@ -58,14 +58,12 @@ public double getProcessCpuUsage() {
             if (prevUpTime > 0L && upTime > prevUpTime) {
                 long elapsedCpu = processCpuTime - prevProcessCpuTime;
                 long elapsedTime = upTime - prevUpTime;
-                processCpuUsage = Math.min(99F, elapsedCpu / (elapsedTime * 10000F * numberOfCpus));
-            } else {
-                processCpuUsage = Constants.DEFAULT_DOUBLE_VALUE;
+                processCpuUsage = Math.min(99.999, elapsedCpu / (elapsedTime * 10_000.0 * numberOfCpus)); // if this looks weird, here's another way to write it: (elapsedCpu / 1000000.0) / elapsedTime / numberOfCpus * 100.0
             }
             prevUpTime = upTime;
             prevProcessCpuTime = processCpuTime;
         } catch (Exception e) {
-            processCpuUsage = Constants.DEFAULT_DOUBLE_VALUE;
+            processCpuUsage = null;
             InternalLogger.INSTANCE.error("Error in getProcessCPUUsage");
             InternalLogger.INSTANCE.trace("Stack trace generated is %s", ExceptionUtils.getStackTrace(e));
         }
diff --git a/core/src/main/java/com/microsoft/applicationinsights/internal/perfcounter/JmxPerformanceCounter.java b/core/src/main/java/com/microsoft/applicationinsights/internal/perfcounter/JmxPerformanceCounter.java
@@ -59,7 +59,10 @@ public JmxPerformanceCounter(String categoryName, String counterName, Map<String
         Preconditions.checkArgument(!objectToAttributes.isEmpty(), "objectToAttributes should be not be empty");
 
         id = categoryName + "." + counterName;
-        telemetry = new PerformanceCounterTelemetry(categoryName, counterName, SystemInformation.INSTANCE.getProcessId(), Constants.DEFAULT_DOUBLE_VALUE);
+        telemetry = new PerformanceCounterTelemetry();
+        telemetry.setCategoryName(categoryName);
+        telemetry.setCounterName(counterName);
+        telemetry.setInstanceName(SystemInformation.INSTANCE.getProcessId());
         this.objectToAttributes = objectToAttributes;
     }
 
diff --git a/core/src/main/java/com/microsoft/applicationinsights/internal/perfcounter/ProcessCpuPerformanceCounter.java b/core/src/main/java/com/microsoft/applicationinsights/internal/perfcounter/ProcessCpuPerformanceCounter.java
@@ -67,7 +67,10 @@ public void report(TelemetryClient telemetryClient) {
         if (cpuPerformanceCounterCalculator == null) {
             return;
         }
-        double processCpuUsage = cpuPerformanceCounterCalculator.getProcessCpuUsage();
+        Double processCpuUsage = cpuPerformanceCounterCalculator.getProcessCpuUsage();
+        if (processCpuUsage == null) {
+            return;
+        }
 
         InternalLogger.INSTANCE.trace("Performance Counter: %s %s: %s", getProcessCategoryName(), Constants.CPU_PC_COUNTER_NAME, processCpuUsage);
         Telemetry telemetry = new PerformanceCounterTelemetry(
diff --git a/core/src/main/java/com/microsoft/applicationinsights/internal/perfcounter/UnixProcessIOPerformanceCounter.java b/core/src/main/java/com/microsoft/applicationinsights/internal/perfcounter/UnixProcessIOPerformanceCounter.java
@@ -60,7 +60,10 @@ public String getId() {
     public void report(TelemetryClient telemetryClient) {
         long currentCollectionInNanos = System.nanoTime();
 
-        double processIO = getCurrentIOForCurrentProcess();
+        Double processIO = getCurrentIOForCurrentProcess();
+        if (processIO == null) {
+            return;
+        }
         if (lastCollectionInNanos != -1) {
             // Not the first time
 
@@ -83,10 +86,14 @@ public void report(TelemetryClient telemetryClient) {
         lastCollectionInNanos = currentCollectionInNanos;
     }
 
-    public double getCurrentIOForCurrentProcess() {
+    /**
+     *
+     * @return the current IO for current process, or null if the datum could not be measured.
+     */
+    public Double getCurrentIOForCurrentProcess() {
         BufferedReader bufferedReader = null;
 
-        double result = Constants.DEFAULT_DOUBLE_VALUE;
+        Double result = null;
         UnixProcessIOtParser parser = new UnixProcessIOtParser();
         try {
             bufferedReader = new BufferedReader(new FileReader(getProcessFile()));
@@ -97,15 +104,15 @@ public double getCurrentIOForCurrentProcess() {
 
             result = parser.getValue();
         } catch (Exception e) {
-            result = Constants.DEFAULT_DOUBLE_VALUE;
-            logError("Error while parsing file: '%s'", getId(), e.toString());
+            result = null;
+            logPerfCounterErrorError("Error while parsing file: '%s'", getId());
             InternalLogger.INSTANCE.trace("Stack trace generated is %s", ExceptionUtils.getStackTrace(e));
         } finally {
             if (bufferedReader != null ) {
                 try {
                     bufferedReader.close();
                 } catch (Exception e) {
-                    logError("Error while closing file : '%s'", e.toString());
+                    logPerfCounterErrorError("Error while closing file : '%s'", e.toString());
                     InternalLogger.INSTANCE.trace("Stack trace generated is %s", ExceptionUtils.getStackTrace(e));
                 }
             }
diff --git a/core/src/main/java/com/microsoft/applicationinsights/internal/perfcounter/UnixTotalCpuPerformanceCounter.java b/core/src/main/java/com/microsoft/applicationinsights/internal/perfcounter/UnixTotalCpuPerformanceCounter.java
@@ -99,14 +99,14 @@ private String getLineOfData() {
             bufferedReader = new BufferedReader(new FileReader(getProcessFile()));
             line = bufferedReader.readLine();
         } catch (Exception e) {
-            logError("Error while parsing file: '%s'", e.toString());
+            logPerfCounterErrorError("Error while parsing file: '%s'", e.toString());
             InternalLogger.INSTANCE.trace("Stack trace generated is %s", ExceptionUtils.getStackTrace(e));
         } finally {
             if (bufferedReader != null ) {
                 try {
                     bufferedReader.close();
                 } catch (Exception e) {
-                    logError("Error while closing file : '%s'", e.toString());
+                    logPerfCounterErrorError("Error while closing file : '%s'", e.toString());
                     InternalLogger.INSTANCE.trace("Stack trace generated is %s", ExceptionUtils.getStackTrace(e));
                 }
             }
diff --git a/core/src/main/java/com/microsoft/applicationinsights/internal/perfcounter/UnixTotalMemoryPerformanceCounter.java b/core/src/main/java/com/microsoft/applicationinsights/internal/perfcounter/UnixTotalMemoryPerformanceCounter.java
@@ -49,7 +49,10 @@ public String getId() {
 
     @Override
     public void report(TelemetryClient telemetryClient) {
-        double totalAvailableMemory = getTotalAvailableMemory();
+        Double totalAvailableMemory = getTotalAvailableMemory();
+        if (totalAvailableMemory == null) {
+            return;
+        }
 
         InternalLogger.INSTANCE.trace("Sending Performance Counter: %s %s: %s", Constants.TOTAL_MEMORY_PC_CATEGORY_NAME, Constants.TOTAL_MEMORY_PC_COUNTER_NAME, totalAvailableMemory);
         Telemetry telemetry = new PerformanceCounterTelemetry(
@@ -61,10 +64,10 @@ public void report(TelemetryClient telemetryClient) {
         telemetryClient.track(telemetry);
     }
 
-    private double getTotalAvailableMemory() {
+    private Double getTotalAvailableMemory() {
         BufferedReader bufferedReader = null;
 
-        double result = Constants.DEFAULT_DOUBLE_VALUE;
+        Double result = null;
         UnixTotalMemInfoParser reader = new UnixTotalMemInfoParser();
         try {
             bufferedReader = new BufferedReader(new FileReader(getProcessFile()));
@@ -76,14 +79,14 @@ private double getTotalAvailableMemory() {
             // The value we get is in KB so we need to translate that to bytes.
             result = reader.getValue() * KB;
         } catch (Exception e) {
-            result = Constants.DEFAULT_DOUBLE_VALUE;
-            logError("Error while parsing file: '%s'", e.toString());
+            result = null;
+            logPerfCounterErrorError("Error while parsing file: '%s'", e.toString());
         } finally {
             if (bufferedReader != null ) {
                 try {
                     bufferedReader.close();
                 } catch (Exception e) {
-                    logError("Error while closing file : '%s'", e.toString());
+                    logPerfCounterErrorError("Error while closing file : '%s'", e.toString());
                 }
             }
         }
diff --git a/core/src/main/java/com/microsoft/applicationinsights/internal/perfcounter/WindowsPerformanceCounterAsMetric.java b/core/src/main/java/com/microsoft/applicationinsights/internal/perfcounter/WindowsPerformanceCounterAsMetric.java
@@ -42,7 +42,7 @@ public final class WindowsPerformanceCounterAsMetric extends AbstractWindowsPerf
     private static final String ID = Constants.PERFORMANCE_COUNTER_PREFIX + "WindowsPerformanceCounterAsMetric";
 
     private final HashMap<String, String> keyToDisplayName = new HashMap<String, String>();
-    private final MetricTelemetry telemetry = new MetricTelemetry("placeholder", Constants.DEFAULT_DOUBLE_VALUE);
+    private final MetricTelemetry telemetry = new MetricTelemetry();
 
     /**
      * Registers the argument's data into performance counters.
diff --git a/core/src/test/java/com/microsoft/applicationinsights/internal/config/ConfigurationFileLocatorTest.java b/core/src/test/java/com/microsoft/applicationinsights/internal/config/ConfigurationFileLocatorTest.java
@@ -21,7 +21,7 @@
 
 package com.microsoft.applicationinsights.internal.config;
 
-import org.junit.Test;
+import org.junit.*;
 
 import java.io.File;
 import java.io.IOException;
@@ -39,6 +39,11 @@ public final class ConfigurationFileLocatorTest {
     private final static String MOCK_CONF_FILE = "MockApplicationInsights.xml";
     private final static String EXISTING_CONF_TEST_FILE = "ApplicationInsights.xml";
 
+    @Before
+    public void clearProp() {
+        System.clearProperty(ConfigurationFileLocator.CONFIG_DIR_PROPERTY);
+    }
+
     @Test(expected = IllegalArgumentException.class)
     public void testCtorWithNull() {
         new ConfigurationFileLocator(null);
diff --git a/test/smoke/framework/testCore/src/main/java/com/microsoft/applicationinsights/smoketest/AiSmokeTest.java b/test/smoke/framework/testCore/src/main/java/com/microsoft/applicationinsights/smoketest/AiSmokeTest.java
@@ -9,6 +9,7 @@
 import com.google.common.collect.Lists;
 import com.google.common.collect.Multimap;
 import com.google.common.io.Resources;
+import com.microsoft.applicationinsights.internal.schemav2.Data;
 import com.microsoft.applicationinsights.internal.schemav2.Domain;
 import com.microsoft.applicationinsights.internal.schemav2.Envelope;
 import com.microsoft.applicationinsights.smoketest.docker.AiDockerClient;
@@ -126,6 +127,7 @@ protected static void stopContainer(ContainerInfo info) throws Exception {
 	//region: application fields
 	protected String targetUri;
 	protected String httpMethod;
+	protected long targetUriDelayMs;
 	protected boolean expectSomeTelemetry = true;
 	//endregion
 
@@ -153,13 +155,14 @@ protected void starting(Description description) {
 			if (targetUri == null) {
 				thiz.targetUri = null;
 				thiz.httpMethod = null;
+				thiz.targetUriDelayMs = 0L;
 			} else {
 				thiz.targetUri = targetUri.value();
 				if (!thiz.targetUri.startsWith("/")) {
 					thiz.targetUri = "/"+thiz.targetUri;
 				}
-
 				thiz.httpMethod = targetUri.method().toUpperCase();
+				thiz.targetUriDelayMs = targetUri.delay();
 			}
 
 			ExpectSomeTelemetry expectSomeTelemetry = description.getAnnotation(ExpectSomeTelemetry.class);
@@ -271,7 +274,11 @@ protected void callTargetUriAndWaitForTelemetry() throws Exception {
 			System.out.println("targetUri==null: automated testapp request disabled");
 			return;
 		}
-
+		if (targetUriDelayMs > 0) {
+			System.out.printf("Waiting %.3fs before calling uri...%n", targetUriDelayMs/1000.0);
+			System.out.flush();
+			TimeUnit.MILLISECONDS.sleep(targetUriDelayMs);
+		}
 		System.out.println("Calling "+targetUri+" ...");
 		String url = getBaseUrl()+targetUri;
 		final String content;
@@ -287,7 +294,7 @@ protected void callTargetUriAndWaitForTelemetry() throws Exception {
 		assertNotNull(String.format("Null response from targetUri: '%s'. %s", targetUri, expectationMessage), content);
 		assertTrue(String.format("Empty response from targetUri: '%s'. %s", targetUri, expectationMessage), content.length() > 0);
 
-		System.out.printf("Waiting %ds for telemetry...", TELEMETRY_RECEIVE_TIMEOUT_SECONDS);
+		System.out.printf("Waiting %ds for telemetry...%n", TELEMETRY_RECEIVE_TIMEOUT_SECONDS);
 		TimeUnit.SECONDS.sleep(TELEMETRY_RECEIVE_TIMEOUT_SECONDS);
 		System.out.println("Finished waiting for telemetry.\nStarting validation...");
 
@@ -347,7 +354,7 @@ protected static void startDockerContainer() throws Exception {
 		System.out.println("Container started: "+containerId);
 
 		final int appServerDelayAfterStart_seconds = 5;
-		System.out.println("Waiting %d seconds for app server to startup...");
+		System.out.printf("Waiting %d seconds for app server to startup...%n", appServerDelayAfterStart_seconds);
 		TimeUnit.SECONDS.sleep(appServerDelayAfterStart_seconds);
 
 		currentContainerInfo = new ContainerInfo(containerId, currentImageName);
@@ -401,6 +408,10 @@ protected static String getProperty(String key) {
 		return rval;
 	}
 
+	protected static <T extends Domain> T getBaseData(Envelope envelope) {
+		return ((Data<T>)envelope.getData()).getBaseData();
+	}
+
 	protected static void waitForUrl(String url, long timeout, TimeUnit timeoutUnit, String appName) throws InterruptedException {
 		String rval = null;
 		Stopwatch watch = Stopwatch.createStarted();
diff --git a/test/smoke/framework/testCore/src/main/java/com/microsoft/applicationinsights/smoketest/TargetUri.java b/test/smoke/framework/testCore/src/main/java/com/microsoft/applicationinsights/smoketest/TargetUri.java
@@ -10,4 +10,9 @@
 public @interface TargetUri {
     String value();
     String method() default "GET";
+
+    /**
+     * The delay in milliseconds to wait before calling the target uri.
+     */
+    long delay() default 0L;
 }
diff --git a/test/smoke/testApps/AutoPerfCounters/src/smokeTest/java/com/microsoft/applicationinsights/smoketest/PerfCountersDataTest.java b/test/smoke/testApps/AutoPerfCounters/src/smokeTest/java/com/microsoft/applicationinsights/smoketest/PerfCountersDataTest.java

Original file line number	Diff line number	Diff line change
`@@ -45,11 +45,11 @@ protected AbstractUnixPerformanceCounter(String path) {`
`45`	`45`	`this.path = path;`
`46`	`46`	`processFile = new File(path);`
`47`	`47`	`if (!processFile.canRead()) {`
`48`		`- logError("Can not read");`
	`48`	`+ logPerfCounterErrorError("Can not read");`
`49`	`49`	`}`
`50`	`50`	`}`
`51`	`51`
`52`		`- protected void logError(String format, Object... args) {`
	`52`	`+ protected void logPerfCounterErrorError(String format, Object... args) {`
`53`	`53`	`format = "Performance Counter " + getId() + ": Error in file '" + path + "': " + format;`
`54`	`54`	`InternalLogger.INSTANCE.error(format, args);`
`55`	`55`	`}`
Original file line number	Diff line number	Diff line change
`@@ -51,8 +51,6 @@ public final class Constants {`
`51`	`51`
`52`	`52`	`public final static String PROCESS_CATEGORY = "Process";`
`53`	`53`
`54`		`- public final static double DEFAULT_DOUBLE_VALUE = -1.0;`
`55`		`-`
`56`	`54`	`private Constants() {`
`57`	`55`	`}`
`58`	`56`	`}`