From 1788635c6f3eb6c497ffe73dc8b39eacfbe8d16d Mon Sep 17 00:00:00 2001 From: littleaj <1690572+littleaj@users.noreply.github.com> Date: Fri, 3 May 2019 09:58:22 -0700 Subject: [PATCH 1/7] added short circuit condition --- .../perfcounter/AbstractWindowsPerformanceCounter.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/core/src/main/java/com/microsoft/applicationinsights/internal/perfcounter/AbstractWindowsPerformanceCounter.java b/core/src/main/java/com/microsoft/applicationinsights/internal/perfcounter/AbstractWindowsPerformanceCounter.java index 21f0059c8c2..ea7d6c3b497 100644 --- a/core/src/main/java/com/microsoft/applicationinsights/internal/perfcounter/AbstractWindowsPerformanceCounter.java +++ b/core/src/main/java/com/microsoft/applicationinsights/internal/perfcounter/AbstractWindowsPerformanceCounter.java @@ -8,6 +8,10 @@ public abstract class AbstractWindowsPerformanceCounter implements PerformanceCounter { protected void reportError(double value, String displayName) { + if (!InternalLogger.INSTANCE.isErrorEnabled()) { + return; + } + if (value == -1) { InternalLogger.INSTANCE.error("Native code exception in wrapper while fetching counter value '%s'", displayName); } else if (value == -4) { From a7d9df540421f797ad3bb060c95d3d6f95654265 Mon Sep 17 00:00:00 2001 From: littleaj <1690572+littleaj@users.noreply.github.com> Date: Fri, 3 May 2019 10:04:24 -0700 Subject: [PATCH 2/7] added log level checks to skip argument processing. using StringUtils instead of guava Strings. --- .../internal/perfcounter/JniPCConnector.java | 30 +++++++++++++------ 1 file changed, 21 insertions(+), 9 deletions(-) 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 index 70f10ba2b64..854f2f76081 100644 --- a/core/src/main/java/com/microsoft/applicationinsights/internal/perfcounter/JniPCConnector.java +++ b/core/src/main/java/com/microsoft/applicationinsights/internal/perfcounter/JniPCConnector.java @@ -24,12 +24,11 @@ import com.microsoft.applicationinsights.internal.logger.InternalLogger; import com.microsoft.applicationinsights.internal.system.SystemInformation; -import com.google.common.base.Preconditions; -import com.google.common.base.Strings; import com.microsoft.applicationinsights.internal.util.LocalFileSystemUtils; import com.microsoft.applicationinsights.internal.util.PropertyHelper; import org.apache.commons.io.FileUtils; import org.apache.commons.io.IOUtils; +import org.apache.commons.lang3.StringUtils; import java.io.File; import java.io.IOException; @@ -99,13 +98,24 @@ public static boolean initialize() { * @param category The category must be non null non empty value. * @param counter The counter must be non null non empty value. * @param instance The instance. - * @return True on success. + * @return The key for retrieving counter data. */ public static String addPerformanceCounter(String category, String counter, String instance) { - Preconditions.checkArgument(!Strings.isNullOrEmpty(category), "category must be non-null non empty string."); - Preconditions.checkArgument(!Strings.isNullOrEmpty(counter), "counter must be non-null non empty string."); + if (StringUtils.isEmpty(category)) { + throw new IllegalArgumentException("category must be non-null non empty string."); + } + if (StringUtils.isEmpty(counter)) { + throw new IllegalArgumentException("counter must be non-null non empty string."); + } - return addCounter(category, counter, instance); + if (InternalLogger.INSTANCE.isTraceEnabled()) { + InternalLogger.INSTANCE.trace("Registering performance counter: %s\\%s [%s]", category, counter, StringUtils.trimToEmpty(instance)); + } + final String s = addCounter(category, counter, instance); + if (StringUtils.isEmpty(s) && InternalLogger.INSTANCE.isWarnEnabled()) { + InternalLogger.INSTANCE.warn("Performance coutner registration failed for %s\\%s [%s]", category, counter, StringUtils.trimToEmpty(instance)); + } + return s; } /** @@ -118,7 +128,7 @@ public static String addPerformanceCounter(String category, String counter, Stri */ public static String translateInstanceName(String instanceName) throws Exception { if (PROCESS_SELF_INSTANCE_NAME.equals(instanceName)) { - if (Strings.isNullOrEmpty(currentInstanceName)) { + if (StringUtils.isEmpty(currentInstanceName)) { throw new Exception("Cannot translate instance name: Unknown current instance name"); } @@ -134,7 +144,9 @@ public static String translateInstanceName(String instanceName) throws Exception * @return The current value. */ public static double getValueOfPerformanceCounter(String name) { - Preconditions.checkArgument(!Strings.isNullOrEmpty(name), "name must be non-null non empty value."); + if(StringUtils.isEmpty(name)) { + throw new IllegalArgumentException("name must be non-null non empty value."); + } return getPerformanceCounterValue(name); } @@ -143,7 +155,7 @@ private static void initNativeCode() { int processId = Integer.parseInt(SystemInformation.INSTANCE.getProcessId()); currentInstanceName = getInstanceName(processId); - if (Strings.isNullOrEmpty(currentInstanceName)) { + if (StringUtils.isEmpty(currentInstanceName)) { InternalLogger.INSTANCE.error("Failed to fetch current process instance name, process counters for for the process level will not be activated."); } else { InternalLogger.INSTANCE.trace("Java process name is set to '%s'", currentInstanceName); From 3fa6a658a4555fbc85b6379788c6109dbf287421 Mon Sep 17 00:00:00 2001 From: littleaj <1690572+littleaj@users.noreply.github.com> Date: Fri, 3 May 2019 10:06:03 -0700 Subject: [PATCH 3/7] if registering pc that already exists, just return the key; not an error prevent null dereference --- core/src/windows/cpp/CppPerfCounters.cpp | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/core/src/windows/cpp/CppPerfCounters.cpp b/core/src/windows/cpp/CppPerfCounters.cpp index 46536bbf10f..3b938c629ac 100644 --- a/core/src/windows/cpp/CppPerfCounters.cpp +++ b/core/src/windows/cpp/CppPerfCounters.cpp @@ -65,7 +65,7 @@ ref class PerfCountersUtils String^ key = performanceCounterCategoryName + performanceCounterCounterName + performanceCounterInstanceName; if (pcDictionary->ContainsKey(key)) { - return nullptr; + return key; } PerformanceCounter^ pc = gcnew PerformanceCounter(performanceCounterCategoryName, performanceCounterCounterName, performanceCounterInstanceName); @@ -213,10 +213,14 @@ JNIEXPORT jstring JNICALL Java_com_microsoft_applicationinsights_internal_perfco env->ReleaseStringUTFChars(rawPerformanceCounterCounterName, performanceCounterCounterName); env->ReleaseStringUTFChars(rawPerformanceCounterInstance, performanceCounterInstance); - std::string a; - MarshalString(value, a); - jstring r = env->NewStringUTF(a.c_str()); - return r; + if (value == nullptr) { + return nullptr; + } else { + std::string a; + MarshalString(value, a); + jstring r = env->NewStringUTF(a.c_str()); + return r; + } } catch (...) { From 1a9f06baea1c2be94eeb336590341da5bcd5bf8d Mon Sep 17 00:00:00 2001 From: littleaj <1690572+littleaj@users.noreply.github.com> Date: Fri, 3 May 2019 10:08:34 -0700 Subject: [PATCH 4/7] remove guava references; replaced with apache commons lang3 --- .../WindowsPerformanceCounterAsMetric.java | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) 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 index 55b1603b684..b17fe36d1bc 100644 --- a/core/src/main/java/com/microsoft/applicationinsights/internal/perfcounter/WindowsPerformanceCounterAsMetric.java +++ b/core/src/main/java/com/microsoft/applicationinsights/internal/perfcounter/WindowsPerformanceCounterAsMetric.java @@ -29,8 +29,7 @@ import com.microsoft.applicationinsights.internal.system.SystemInformation; import com.microsoft.applicationinsights.telemetry.MetricTelemetry; -import com.google.common.base.Preconditions; -import com.google.common.base.Strings; +import org.apache.commons.lang3.StringUtils; import org.apache.commons.lang3.exception.ExceptionUtils; /** @@ -51,8 +50,12 @@ public final class WindowsPerformanceCounterAsMetric extends AbstractWindowsPerf * connect to the native code. or Exception if the constructor is not called under Windows OS. */ public WindowsPerformanceCounterAsMetric(Iterable pcsData) throws Throwable { - Preconditions.checkState(SystemInformation.INSTANCE.isWindows(), "Must be used under Windows OS."); - Preconditions.checkNotNull(pcsData, "pcsData must be non-null value."); + if (!SystemInformation.INSTANCE.isWindows()) { + throw new IllegalStateException("Must be used under Windows OS."); + } + if (pcsData == null) { + throw new IllegalArgumentException("pcsData must be non-null value."); + } // indicate that this is used for performance counters, not custom metrics. telemetry.markAsCustomPerfCounter(); @@ -107,7 +110,7 @@ private void register(Iterable pcsData) { for (WindowsPerformanceCounterData data : pcsData) { try { String key = JniPCConnector.addPerformanceCounter(data.categoryName, data.counterName, data.instanceName); - if (!Strings.isNullOrEmpty(key)) { + if (!StringUtils.isEmpty(key)) { keyToDisplayName.put(key, data.displayName); } } catch (ThreadDeath td) { From bc867770fbdef63742160d4b8d75bc88025a4250 Mon Sep 17 00:00:00 2001 From: littleaj <1690572+littleaj@users.noreply.github.com> Date: Fri, 3 May 2019 10:09:12 -0700 Subject: [PATCH 5/7] simplified log messages, added log level checks --- .../perfcounter/WindowsPerformanceCounterAsPC.java | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/core/src/main/java/com/microsoft/applicationinsights/internal/perfcounter/WindowsPerformanceCounterAsPC.java b/core/src/main/java/com/microsoft/applicationinsights/internal/perfcounter/WindowsPerformanceCounterAsPC.java index 4a9984ad6d4..a04fe01f116 100644 --- a/core/src/main/java/com/microsoft/applicationinsights/internal/perfcounter/WindowsPerformanceCounterAsPC.java +++ b/core/src/main/java/com/microsoft/applicationinsights/internal/perfcounter/WindowsPerformanceCounterAsPC.java @@ -81,8 +81,9 @@ public void report(TelemetryClient telemetryClient) { throw td; } catch (Throwable e) { try { - InternalLogger.INSTANCE.error("Failed to send performance counter for '%s': '%s'", entry.getValue().displayName, e.toString()); - InternalLogger.INSTANCE.trace("Stack trace generated is %s", ExceptionUtils.getStackTrace(e)); + if (InternalLogger.INSTANCE.isErrorEnabled()) { + InternalLogger.INSTANCE.error("Failed to send performance counter for '%s': %s", entry.getValue().displayName, ExceptionUtils.getStackTrace(e)); + } } catch (ThreadDeath td) { throw td; } catch (Throwable t2) { @@ -123,8 +124,9 @@ private void register(String category, String counter, String instance) { throw td; } catch (Throwable e) { try { - InternalLogger.INSTANCE.error("Exception while registering windows performance counter as PC"); - InternalLogger.INSTANCE.trace("Stack trace generated is %s", ExceptionUtils.getStackTrace(e)); + if (InternalLogger.INSTANCE.isErrorEnabled()) { + InternalLogger.INSTANCE.error("Exception while registering windows performance counter as PC: %s", ExceptionUtils.getStackTrace(e)); + } } catch (ThreadDeath td) { throw td; } catch (Throwable t2) { From cc8ea3c19386e4ae98312cef3af9ae7baf941f9a Mon Sep 17 00:00:00 2001 From: littleaj <1690572+littleaj@users.noreply.github.com> Date: Fri, 3 May 2019 10:40:31 -0700 Subject: [PATCH 6/7] clean jni tmp dir --- core/build.gradle | 1 + 1 file changed, 1 insertion(+) diff --git a/core/build.gradle b/core/build.gradle index 628436f6117..bea94e1b99f 100644 --- a/core/build.gradle +++ b/core/build.gradle @@ -83,6 +83,7 @@ sourceSets { clean { delete 'src/main/generated' + delete "${System.properties['java.io.tmpdir']}/AISDK/native" // see JniPCConnector.java for jni directory } import org.apache.tools.ant.taskdefs.condition.Os From d7944071bdcf47ca71ce219a7dea40ceec61955e Mon Sep 17 00:00:00 2001 From: littleaj <1690572+littleaj@users.noreply.github.com> Date: Fri, 3 May 2019 11:02:01 -0700 Subject: [PATCH 7/7] using npe instead --- .../internal/perfcounter/WindowsPerformanceCounterAsMetric.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 index b17fe36d1bc..2681bb50bc8 100644 --- a/core/src/main/java/com/microsoft/applicationinsights/internal/perfcounter/WindowsPerformanceCounterAsMetric.java +++ b/core/src/main/java/com/microsoft/applicationinsights/internal/perfcounter/WindowsPerformanceCounterAsMetric.java @@ -54,7 +54,7 @@ public WindowsPerformanceCounterAsMetric(Iterable throw new IllegalStateException("Must be used under Windows OS."); } if (pcsData == null) { - throw new IllegalArgumentException("pcsData must be non-null value."); + throw new NullPointerException("pcsData must be non-null value."); } // indicate that this is used for performance counters, not custom metrics.