From d5b0ade6119a33687a81b594df11f94a40777f87 Mon Sep 17 00:00:00 2001 From: Adi Muraru Date: Thu, 18 Apr 2019 07:35:59 +0000 Subject: [PATCH 1/2] Allow dynamic update of agent parameters Made the agent profilers re-entrant allowing to attach and update profiling parameters at runtime This is useful when the VM is not started with the agent but can be later attached and controlled, e.g: - update profiling interval - enabling/disabling profilers For time being only profilers parameters can be updated. Transformers, reporter not --- attach-client.sh | 8 + pom.xml | 8 +- .../java/com/uber/profiling/AgentImpl.java | 300 ++++++++++-------- .../java/com/uber/profiling/Arguments.java | 7 +- .../java/com/uber/profiling/Profiler.java | 43 ++- .../com/uber/profiling/ProfilerGroup.java | 38 --- .../com/uber/profiling/ProfilerRunner.java | 48 --- .../uber/profiling/ShutdownHookRunner.java | 4 + .../profilers/CpuAndMemoryProfiler.java | 4 +- .../uber/profiling/profilers/IOProfiler.java | 3 +- .../profilers/MethodArgumentCollector.java | 1 + .../profilers/MethodArgumentProfiler.java | 3 +- .../profilers/MethodDurationProfiler.java | 3 +- .../profilers/ProcessInfoProfiler.java | 12 +- .../profiling/profilers/ProfilerBase.java | 3 +- .../StacktraceCollectorProfiler.java | 6 +- .../profilers/StacktraceReporterProfiler.java | 7 +- .../reporters/ConsoleOutputReporter.java | 2 - .../uber/profiling/tools/AttachClient.java | 20 ++ .../com/uber/profiling/util/StringUtils.java | 3 - .../java/com/uber/profiling/AgentITCase.java | 97 +++++- ...lerRunnableTest.java => ProfilerTest.java} | 10 +- .../profilers/ProcessInfoProfilerTest.java | 2 +- 23 files changed, 375 insertions(+), 257 deletions(-) create mode 100755 attach-client.sh delete mode 100644 src/main/java/com/uber/profiling/ProfilerGroup.java delete mode 100644 src/main/java/com/uber/profiling/ProfilerRunner.java create mode 100644 src/main/java/com/uber/profiling/tools/AttachClient.java rename src/test/java/com/uber/profiling/{ProfilerRunnableTest.java => ProfilerTest.java} (86%) diff --git a/attach-client.sh b/attach-client.sh new file mode 100755 index 0000000..671dce2 --- /dev/null +++ b/attach-client.sh @@ -0,0 +1,8 @@ +#!/bin/sh + +DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" >/dev/null 2>&1 && pwd )" +AGENT_PATH=`ls -1 $DIR/target/jvm-profiler-*.jar` +echo $AGENT_PATH + +java -cp $AGENT_PATH:$JAVA_HOME/lib/tools.jar com.uber.profiling.tools.AttachClient $AGENT_PATH $* + diff --git a/pom.xml b/pom.xml index 5c84fbb..b03f83f 100644 --- a/pom.xml +++ b/pom.xml @@ -81,7 +81,13 @@ 4.8.1 test - + + + + com.github.olivergondza + maven-jdk-tools-wrapper + 0.1 + diff --git a/src/main/java/com/uber/profiling/AgentImpl.java b/src/main/java/com/uber/profiling/AgentImpl.java index 3315330..c8e6d75 100644 --- a/src/main/java/com/uber/profiling/AgentImpl.java +++ b/src/main/java/com/uber/profiling/AgentImpl.java @@ -32,15 +32,14 @@ import com.uber.profiling.util.ClassMethodArgumentMetricBuffer; import com.uber.profiling.util.SparkUtils; import com.uber.profiling.util.StacktraceMetricBuffer; - import java.lang.instrument.Instrumentation; -import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; -import java.util.List; +import java.util.Map; import java.util.UUID; -import java.util.concurrent.Executors; -import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ScheduledFuture; +import java.util.concurrent.ScheduledThreadPoolExecutor; import java.util.concurrent.TimeUnit; public class AgentImpl { @@ -50,6 +49,14 @@ public class AgentImpl { private static final int MAX_THREAD_POOL_SIZE = 2; + private Map profilers = new ConcurrentHashMap<>(); + private String processUuid; + private String appId; + private JavaAgentFileTransformer transformer; + private Thread shutdownHook; + // use ScheduledThreadPoolExecutor instead of executor service + // so we can set setRemoveOnCancelPolicy + private ScheduledThreadPoolExecutor scheduledExecutorService; private boolean started = false; public void run(Arguments arguments, Instrumentation instrumentation, Collection objectsToCloseOnShutdown) { @@ -60,173 +67,190 @@ public void run(Arguments arguments, Instrumentation instrumentation, Collection Reporter reporter = arguments.getReporter(); - String processUuid = UUID.randomUUID().toString(); - - String appId = null; - - String appIdVariable = arguments.getAppIdVariable(); - if (appIdVariable != null && !appIdVariable.isEmpty()) { - appId = System.getenv(appIdVariable); - } - - if (appId == null || appId.isEmpty()) { - appId = SparkUtils.probeAppId(arguments.getAppIdRegex()); - } - - if (!arguments.getDurationProfiling().isEmpty() - || !arguments.getArgumentProfiling().isEmpty()) { - instrumentation.addTransformer(new JavaAgentFileTransformer(arguments.getDurationProfiling(), arguments.getArgumentProfiling())); - } - - List profilers = createProfilers(reporter, arguments, processUuid, appId); - - ProfilerGroup profilerGroup = startProfilers(profilers); - - Thread shutdownHook = new Thread(new ShutdownHookRunner(profilerGroup.getPeriodicProfilers(), Arrays.asList(reporter), objectsToCloseOnShutdown)); - Runtime.getRuntime().addShutdownHook(shutdownHook); - } - - public ProfilerGroup startProfilers(Collection profilers) { - if (started) { - logger.warn("Profilers already started, do not start it again"); - return new ProfilerGroup(new ArrayList<>(), new ArrayList<>()); + if (processUuid == null) { + processUuid = UUID.randomUUID().toString(); } - List oneTimeProfilers = new ArrayList<>(); - List periodicProfilers = new ArrayList<>(); - - for (Profiler profiler : profilers) { - if (profiler.getIntervalMillis() == 0) { - oneTimeProfilers.add(profiler); - } else if (profiler.getIntervalMillis() > 0) { - periodicProfilers.add(profiler); - } else { - logger.log(String.format("Ignored profiler %s due to its invalid interval %s", profiler, profiler.getIntervalMillis())); + if (appId == null){ + String appIdVariable = arguments.getAppIdVariable(); + if (appIdVariable != null && !appIdVariable.isEmpty()) { + appId = System.getenv(appIdVariable); } - } - for (Profiler profiler : oneTimeProfilers) { - try { - profiler.profile(); - logger.info("Finished one time profiler: " + profiler); - } catch (Throwable ex) { - logger.warn("Failed to run one time profiler: " + profiler, ex); + if (appId == null || appId.isEmpty()) { + appId = SparkUtils.probeAppId(arguments.getAppIdRegex()); } } - for (Profiler profiler : periodicProfilers) { - try { - profiler.profile(); - logger.info("Ran periodic profiler (first run): " + profiler); - } catch (Throwable ex) { - logger.warn("Failed to run periodic profiler (first run): " + profiler, ex); + if (transformer == null) { + if (!arguments.getDurationProfiling().isEmpty() + || !arguments.getArgumentProfiling().isEmpty()) { + transformer = new JavaAgentFileTransformer( + arguments.getDurationProfiling(), + arguments.getArgumentProfiling()); + instrumentation.addTransformer(transformer); } } - - scheduleProfilers(periodicProfilers); - started = true; - return new ProfilerGroup(oneTimeProfilers, periodicProfilers); + createProfilers(reporter, arguments, processUuid, appId); + runProfilers(); + scheduleProfilers(); + + // set set/update shutdown hook + if (shutdownHook != null) { + // cancel previous, in case new profilers are added at runtime + Runtime.getRuntime().removeShutdownHook(shutdownHook); + } + shutdownHook = new Thread(new ShutdownHookRunner(profilers.values(), + Arrays.asList(reporter), objectsToCloseOnShutdown)); + Runtime.getRuntime().addShutdownHook(shutdownHook); } - private List createProfilers(Reporter reporter, Arguments arguments, String processUuid, String appId) { + /** + * Create or update the schedule interval for the profilers + */ + private void createProfilers(Reporter reporter, Arguments arguments, String processUuid, String appId) { String tag = arguments.getTag(); String cluster = arguments.getCluster(); long metricInterval = arguments.getMetricInterval(); - List profilers = new ArrayList<>(); - - CpuAndMemoryProfiler cpuAndMemoryProfiler = new CpuAndMemoryProfiler(reporter); - cpuAndMemoryProfiler.setTag(tag); - cpuAndMemoryProfiler.setCluster(cluster); - cpuAndMemoryProfiler.setIntervalMillis(metricInterval); - cpuAndMemoryProfiler.setProcessUuid(processUuid); - cpuAndMemoryProfiler.setAppId(appId); - - profilers.add(cpuAndMemoryProfiler); - - ProcessInfoProfiler processInfoProfiler = new ProcessInfoProfiler(reporter); - processInfoProfiler.setTag(tag); - processInfoProfiler.setCluster(cluster); - processInfoProfiler.setProcessUuid(processUuid); - processInfoProfiler.setAppId(appId); - - profilers.add(processInfoProfiler); + // create once the profiler, + // but update interval period in case this is changed at runtime + profilers.computeIfAbsent(CpuAndMemoryProfiler.PROFILER_NAME, k -> { + CpuAndMemoryProfiler cpuAndMemoryProfiler = new CpuAndMemoryProfiler(reporter); + cpuAndMemoryProfiler.setTag(tag); + cpuAndMemoryProfiler.setCluster(cluster); + cpuAndMemoryProfiler.setProcessUuid(processUuid); + cpuAndMemoryProfiler.setAppId(appId); + return cpuAndMemoryProfiler; + }).setIntervalMillis(metricInterval); + + profilers.computeIfAbsent(ProcessInfoProfiler.PROFILER_NAME, k -> { + ProcessInfoProfiler processInfoProfiler = new ProcessInfoProfiler(reporter); + processInfoProfiler.setTag(tag); + processInfoProfiler.setCluster(cluster); + processInfoProfiler.setProcessUuid(processUuid); + processInfoProfiler.setAppId(appId); + processInfoProfiler.setIntervalMillis(-1); + return processInfoProfiler; + }); if (!arguments.getDurationProfiling().isEmpty()) { - ClassAndMethodLongMetricBuffer classAndMethodMetricBuffer = new ClassAndMethodLongMetricBuffer(); - - MethodDurationProfiler methodDurationProfiler = new MethodDurationProfiler(classAndMethodMetricBuffer, reporter); - methodDurationProfiler.setTag(tag); - methodDurationProfiler.setCluster(cluster); - methodDurationProfiler.setIntervalMillis(metricInterval); - methodDurationProfiler.setProcessUuid(processUuid); - methodDurationProfiler.setAppId(appId); - - MethodDurationCollector methodDurationCollector = new MethodDurationCollector(classAndMethodMetricBuffer); - MethodProfilerStaticProxy.setCollector(methodDurationCollector); - - profilers.add(methodDurationProfiler); + profilers.computeIfAbsent(MethodDurationProfiler.PROFILER_NAME, k -> { + ClassAndMethodLongMetricBuffer classAndMethodMetricBuffer = new ClassAndMethodLongMetricBuffer(); + + MethodDurationProfiler methodDurationProfiler = new MethodDurationProfiler( + classAndMethodMetricBuffer, reporter); + methodDurationProfiler.setTag(tag); + methodDurationProfiler.setCluster(cluster); + methodDurationProfiler.setProcessUuid(processUuid); + methodDurationProfiler.setAppId(appId); + + MethodDurationCollector methodDurationCollector = new MethodDurationCollector( + classAndMethodMetricBuffer); + MethodProfilerStaticProxy.setCollector(methodDurationCollector); + return methodDurationProfiler; + }).setIntervalMillis(metricInterval); } if (!arguments.getArgumentProfiling().isEmpty()) { - ClassMethodArgumentMetricBuffer classAndMethodArgumentBuffer = new ClassMethodArgumentMetricBuffer(); - - MethodArgumentProfiler methodArgumentProfiler = new MethodArgumentProfiler(classAndMethodArgumentBuffer, reporter); - methodArgumentProfiler.setTag(tag); - methodArgumentProfiler.setCluster(cluster); - methodArgumentProfiler.setIntervalMillis(metricInterval); - methodArgumentProfiler.setProcessUuid(processUuid); - methodArgumentProfiler.setAppId(appId); - - MethodArgumentCollector methodArgumentCollector = new MethodArgumentCollector(classAndMethodArgumentBuffer); - MethodProfilerStaticProxy.setArgumentCollector(methodArgumentCollector); + profilers.computeIfAbsent(MethodArgumentCollector.PROFILER_NAME, k -> { + ClassMethodArgumentMetricBuffer classAndMethodArgumentBuffer = new ClassMethodArgumentMetricBuffer(); + + MethodArgumentProfiler methodArgumentProfiler = new MethodArgumentProfiler( + classAndMethodArgumentBuffer, reporter); + methodArgumentProfiler.setTag(tag); + methodArgumentProfiler.setCluster(cluster); + methodArgumentProfiler.setProcessUuid(processUuid); + methodArgumentProfiler.setAppId(appId); + + MethodArgumentCollector methodArgumentCollector = new MethodArgumentCollector( + classAndMethodArgumentBuffer); + MethodProfilerStaticProxy.setArgumentCollector(methodArgumentCollector); + return methodArgumentProfiler; + }).setIntervalMillis(metricInterval); - profilers.add(methodArgumentProfiler); } - + if (arguments.getSampleInterval() > 0) { - StacktraceMetricBuffer stacktraceMetricBuffer = new StacktraceMetricBuffer(); - - StacktraceCollectorProfiler stacktraceCollectorProfiler = new StacktraceCollectorProfiler(stacktraceMetricBuffer, AgentThreadFactory.NAME_PREFIX); - stacktraceCollectorProfiler.setIntervalMillis(arguments.getSampleInterval()); - - StacktraceReporterProfiler stacktraceReporterProfiler = new StacktraceReporterProfiler(stacktraceMetricBuffer, reporter); - stacktraceReporterProfiler.setTag(tag); - stacktraceReporterProfiler.setCluster(cluster); - stacktraceReporterProfiler.setIntervalMillis(metricInterval); - stacktraceReporterProfiler.setProcessUuid(processUuid); - stacktraceReporterProfiler.setAppId(appId); - - profilers.add(stacktraceCollectorProfiler); - profilers.add(stacktraceReporterProfiler); + Profiler reporterProfiler = profilers + .computeIfAbsent(StacktraceReporterProfiler.PROFILER_NAME, k -> { + StacktraceMetricBuffer stacktraceMetricBuffer = new StacktraceMetricBuffer(); + StacktraceReporterProfiler stacktraceReporterProfiler = new StacktraceReporterProfiler( + stacktraceMetricBuffer, reporter); + stacktraceReporterProfiler.setTag(tag); + stacktraceReporterProfiler.setCluster(cluster); + stacktraceReporterProfiler.setProcessUuid(processUuid); + stacktraceReporterProfiler.setAppId(appId); + return stacktraceReporterProfiler; + }); + reporterProfiler.setIntervalMillis(metricInterval); + + profilers.computeIfAbsent(StacktraceCollectorProfiler.PROFILER_NAME, k -> { + StacktraceCollectorProfiler stacktraceCollectorProfiler = new StacktraceCollectorProfiler( + ((StacktraceReporterProfiler) reporterProfiler).getBuffer(), + AgentThreadFactory.NAME_PREFIX); + return stacktraceCollectorProfiler; + }).setIntervalMillis(arguments.getSampleInterval()); } if (arguments.isIoProfiling()) { - IOProfiler ioProfiler = new IOProfiler(reporter); - ioProfiler.setTag(tag); - ioProfiler.setCluster(cluster); - ioProfiler.setIntervalMillis(metricInterval); - ioProfiler.setProcessUuid(processUuid); - ioProfiler.setAppId(appId); - - profilers.add(ioProfiler); + profilers.computeIfAbsent(IOProfiler.PROFILER_NAME, k -> { + IOProfiler ioProfiler = new IOProfiler(reporter); + ioProfiler.setTag(tag); + ioProfiler.setCluster(cluster); + ioProfiler.setProcessUuid(processUuid); + ioProfiler.setAppId(appId); + + return ioProfiler; + }).setIntervalMillis(metricInterval); } - - return profilers; } - private void scheduleProfilers(Collection profilers) { - int threadPoolSize = Math.min(profilers.size(), MAX_THREAD_POOL_SIZE); - ScheduledExecutorService scheduledExecutorService = Executors.newScheduledThreadPool(threadPoolSize, new AgentThreadFactory()); + /** + * Run profilers at start-up, once + */ + private void runProfilers() { + if (started) { + logger.info("Profilers already started, do not run them again"); + return; + } + for (Profiler profiler : profilers.values()) { + try { + profiler.profile(); + logger.info("Ran profiler: " + profiler); + } catch (Throwable ex) { + logger.warn("Failed to run profiler: " + profiler, ex); + } + } + started = true; + } - for (Profiler profiler : profilers) { + /** + * (Re)schedule periodic profilers + */ + private void scheduleProfilers() { + if (scheduledExecutorService == null) { + int threadPoolSize = Math.min(profilers.size(), MAX_THREAD_POOL_SIZE); + scheduledExecutorService = new ScheduledThreadPoolExecutor(threadPoolSize, new AgentThreadFactory()); + scheduledExecutorService.setRemoveOnCancelPolicy(true); + } + + for (Profiler profiler : profilers.values()) { + if (profiler.getScheduleHandler() != null) { + //cancel previous task if already scheduled + profiler.getScheduleHandler().cancel(false); + } + if (profiler.getIntervalMillis() <= 0){ + //one time profiler, don't schedule + continue; + } if (profiler.getIntervalMillis() < Arguments.MIN_INTERVAL_MILLIS) { throw new RuntimeException("Interval too short for profiler: " + profiler + ", must be at least " + Arguments.MIN_INTERVAL_MILLIS); } - - ProfilerRunner worker = new ProfilerRunner(profiler); - scheduledExecutorService.scheduleAtFixedRate(worker, 0, profiler.getIntervalMillis(), TimeUnit.MILLISECONDS); + ScheduledFuture handler = scheduledExecutorService.scheduleAtFixedRate(profiler, 0, profiler.getIntervalMillis(), TimeUnit.MILLISECONDS); + // save the scheduled handler. so we can cancel if needed + profiler.setScheduleHandler(handler); logger.info(String.format("Scheduled profiler %s with interval %s millis", profiler, profiler.getIntervalMillis())); } } diff --git a/src/main/java/com/uber/profiling/Arguments.java b/src/main/java/com/uber/profiling/Arguments.java index 8052355..2eda09f 100644 --- a/src/main/java/com/uber/profiling/Arguments.java +++ b/src/main/java/com/uber/profiling/Arguments.java @@ -23,7 +23,6 @@ import com.uber.profiling.util.DummyConfigProvider; import com.uber.profiling.util.JsonUtils; import com.uber.profiling.util.ReflectionUtils; - import java.lang.reflect.Constructor; import java.util.ArrayList; import java.util.HashMap; @@ -59,6 +58,7 @@ public class Arguments { private boolean noop = false; private Constructor reporterConstructor; + private Reporter reporter; private Constructor configProviderConstructor; private String configFile; @@ -260,11 +260,14 @@ public Map> getRawArgValues() { } public Reporter getReporter() { + if (reporter != null) { + return reporter; + } if (reporterConstructor == null) { return new ConsoleOutputReporter(); } else { try { - Reporter reporter = reporterConstructor.newInstance(); + reporter = reporterConstructor.newInstance(); reporter.updateArguments(getRawArgValues()); return reporter; } catch (Throwable e) { diff --git a/src/main/java/com/uber/profiling/Profiler.java b/src/main/java/com/uber/profiling/Profiler.java index 464b8a0..a212641 100644 --- a/src/main/java/com/uber/profiling/Profiler.java +++ b/src/main/java/com/uber/profiling/Profiler.java @@ -16,10 +16,45 @@ package com.uber.profiling; -public interface Profiler { - long getIntervalMillis(); +import com.uber.profiling.util.AgentLogger; +import java.util.concurrent.ScheduledFuture; +import java.util.concurrent.atomic.AtomicLong; - void setReporter(Reporter reporter); +public abstract class Profiler implements Runnable { - void profile(); + private static final AgentLogger logger = AgentLogger.getLogger(Profiler.class.getName()); + private static final int MAX_ERROR_COUNT_TO_LOG = 100; + + private final AtomicLong errorCounter = new AtomicLong(0); + private ScheduledFuture scheduleHandler; + + public abstract long getIntervalMillis(); + + public abstract void setIntervalMillis(long millis); + + public abstract void setReporter(Reporter reporter); + + public abstract void profile(); + + @Override + public void run() { + try { + this.profile(); + } catch (Throwable e) { + long errorCountValue = errorCounter.incrementAndGet(); + if (errorCountValue <= MAX_ERROR_COUNT_TO_LOG) { + logger.warn("Failed to run profile: " + this, e); + } else { + e.printStackTrace(); + } + } + } + + public ScheduledFuture getScheduleHandler() { + return scheduleHandler; + } + + public void setScheduleHandler(ScheduledFuture handler) { + this.scheduleHandler = handler; + } } diff --git a/src/main/java/com/uber/profiling/ProfilerGroup.java b/src/main/java/com/uber/profiling/ProfilerGroup.java deleted file mode 100644 index 5e4b36d..0000000 --- a/src/main/java/com/uber/profiling/ProfilerGroup.java +++ /dev/null @@ -1,38 +0,0 @@ -/* - * Copyright (c) 2018 Uber Technologies, Inc. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package com.uber.profiling; - -import java.util.ArrayList; -import java.util.List; - -public class ProfilerGroup { - private List oneTimeProfilers; - private List periodicProfilers; - - public ProfilerGroup(List oneTimeProfilers, List periodicProfilers) { - this.oneTimeProfilers = new ArrayList<>(oneTimeProfilers); - this.periodicProfilers = new ArrayList<>(periodicProfilers); - } - - public List getOneTimeProfilers() { - return oneTimeProfilers; - } - - public List getPeriodicProfilers() { - return periodicProfilers; - } -} diff --git a/src/main/java/com/uber/profiling/ProfilerRunner.java b/src/main/java/com/uber/profiling/ProfilerRunner.java deleted file mode 100644 index bcd6db3..0000000 --- a/src/main/java/com/uber/profiling/ProfilerRunner.java +++ /dev/null @@ -1,48 +0,0 @@ -/* - * Copyright (c) 2018 Uber Technologies, Inc. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package com.uber.profiling; - -import com.uber.profiling.util.AgentLogger; - -import java.util.concurrent.atomic.AtomicLong; - -public class ProfilerRunner implements Runnable { - private static final AgentLogger logger = AgentLogger.getLogger(ProfilerRunner.class.getName()); - - private static final int MAX_ERROR_COUNT_TO_LOG = 100; - - private final Profiler profiler; - private final AtomicLong errorCounter = new AtomicLong(0); - - public ProfilerRunner(Profiler profiler) { - this.profiler = profiler; - } - - @Override - public void run() { - try { - profiler.profile(); - } catch (Throwable e) { - long errorCountValue = errorCounter.incrementAndGet(); - if (errorCountValue <= MAX_ERROR_COUNT_TO_LOG) { - logger.warn("Failed to run profile: " + profiler, e); - } else { - e.printStackTrace(); - } - } - } -} diff --git a/src/main/java/com/uber/profiling/ShutdownHookRunner.java b/src/main/java/com/uber/profiling/ShutdownHookRunner.java index 26eaca6..0201872 100644 --- a/src/main/java/com/uber/profiling/ShutdownHookRunner.java +++ b/src/main/java/com/uber/profiling/ShutdownHookRunner.java @@ -42,6 +42,10 @@ public void run() { for (Profiler profiler : profilers) { try { + if (profiler.getIntervalMillis() <=0 ){ + //one time profiler, skip + continue; + } logShutdownMessage("Running periodic profiler (last run): " + profiler); profiler.profile(); logShutdownMessage("Ran periodic profiler (last run): " + profiler); diff --git a/src/main/java/com/uber/profiling/profilers/CpuAndMemoryProfiler.java b/src/main/java/com/uber/profiling/profilers/CpuAndMemoryProfiler.java index 22448f4..d87a9a5 100644 --- a/src/main/java/com/uber/profiling/profilers/CpuAndMemoryProfiler.java +++ b/src/main/java/com/uber/profiling/profilers/CpuAndMemoryProfiler.java @@ -16,7 +16,6 @@ package com.uber.profiling.profilers; -import com.uber.profiling.Profiler; import com.uber.profiling.Reporter; import com.uber.profiling.util.AgentLogger; import com.uber.profiling.util.ProcFileUtils; @@ -36,7 +35,7 @@ import java.util.List; import java.util.Map; -public class CpuAndMemoryProfiler extends ProfilerBase implements Profiler { +public class CpuAndMemoryProfiler extends ProfilerBase { public final static String PROFILER_NAME = "CpuAndMemory"; private static final AgentLogger logger = AgentLogger.getLogger(CpuAndMemoryProfiler.class.getName()); @@ -70,6 +69,7 @@ public long getIntervalMillis() { return intervalMillis; } + @Override public void setIntervalMillis(long intervalMillis) { this.intervalMillis = intervalMillis; } diff --git a/src/main/java/com/uber/profiling/profilers/IOProfiler.java b/src/main/java/com/uber/profiling/profilers/IOProfiler.java index c1ac93d..211912d 100644 --- a/src/main/java/com/uber/profiling/profilers/IOProfiler.java +++ b/src/main/java/com/uber/profiling/profilers/IOProfiler.java @@ -16,7 +16,6 @@ package com.uber.profiling.profilers; -import com.uber.profiling.Profiler; import com.uber.profiling.Reporter; import com.uber.profiling.util.ProcFileUtils; @@ -24,7 +23,7 @@ import java.util.List; import java.util.Map; -public class IOProfiler extends ProfilerBase implements Profiler { +public class IOProfiler extends ProfilerBase { public final static String PROFILER_NAME = "IO"; private long intervalMillis = Constants.DEFAULT_METRIC_INTERVAL; diff --git a/src/main/java/com/uber/profiling/profilers/MethodArgumentCollector.java b/src/main/java/com/uber/profiling/profilers/MethodArgumentCollector.java index c7d3688..7011670 100644 --- a/src/main/java/com/uber/profiling/profilers/MethodArgumentCollector.java +++ b/src/main/java/com/uber/profiling/profilers/MethodArgumentCollector.java @@ -19,6 +19,7 @@ import com.uber.profiling.util.ClassMethodArgumentMetricBuffer; public class MethodArgumentCollector { + public static final String PROFILER_NAME = "MethodArgumentCollector"; private ClassMethodArgumentMetricBuffer buffer; public MethodArgumentCollector(ClassMethodArgumentMetricBuffer buffer) { diff --git a/src/main/java/com/uber/profiling/profilers/MethodArgumentProfiler.java b/src/main/java/com/uber/profiling/profilers/MethodArgumentProfiler.java index a20e384..bf94444 100644 --- a/src/main/java/com/uber/profiling/profilers/MethodArgumentProfiler.java +++ b/src/main/java/com/uber/profiling/profilers/MethodArgumentProfiler.java @@ -16,7 +16,6 @@ package com.uber.profiling.profilers; -import com.uber.profiling.Profiler; import com.uber.profiling.Reporter; import com.uber.profiling.reporters.ConsoleOutputReporter; import com.uber.profiling.util.ClassAndMethodMetricKey; @@ -26,7 +25,7 @@ import java.util.Map; import java.util.concurrent.atomic.AtomicLong; -public class MethodArgumentProfiler extends ProfilerBase implements Profiler { +public class MethodArgumentProfiler extends ProfilerBase { public static final String PROFILER_NAME = "MethodArgument"; private ClassMethodArgumentMetricBuffer buffer; diff --git a/src/main/java/com/uber/profiling/profilers/MethodDurationProfiler.java b/src/main/java/com/uber/profiling/profilers/MethodDurationProfiler.java index 344ab11..21b79e9 100644 --- a/src/main/java/com/uber/profiling/profilers/MethodDurationProfiler.java +++ b/src/main/java/com/uber/profiling/profilers/MethodDurationProfiler.java @@ -16,7 +16,6 @@ package com.uber.profiling.profilers; -import com.uber.profiling.Profiler; import com.uber.profiling.Reporter; import com.uber.profiling.reporters.ConsoleOutputReporter; import com.uber.profiling.util.ClassAndMethodLongMetricBuffer; @@ -26,7 +25,7 @@ import java.util.HashMap; import java.util.Map; -public class MethodDurationProfiler extends ProfilerBase implements Profiler { +public class MethodDurationProfiler extends ProfilerBase { public static final String PROFILER_NAME = "MethodDuration"; private ClassAndMethodLongMetricBuffer buffer; diff --git a/src/main/java/com/uber/profiling/profilers/ProcessInfoProfiler.java b/src/main/java/com/uber/profiling/profilers/ProcessInfoProfiler.java index fe22962..b8ab3e9 100644 --- a/src/main/java/com/uber/profiling/profilers/ProcessInfoProfiler.java +++ b/src/main/java/com/uber/profiling/profilers/ProcessInfoProfiler.java @@ -17,7 +17,6 @@ package com.uber.profiling.profilers; import com.uber.profiling.AgentImpl; -import com.uber.profiling.Profiler; import com.uber.profiling.Reporter; import com.uber.profiling.util.AgentLogger; import com.uber.profiling.util.ProcFileUtils; @@ -30,7 +29,7 @@ import java.util.List; import java.util.Map; -public class ProcessInfoProfiler extends ProfilerBase implements Profiler { +public class ProcessInfoProfiler extends ProfilerBase { public final static String PROFILER_NAME = "ProcessInfo"; private static final AgentLogger logger = AgentLogger.getLogger(ProcessInfoProfiler.class.getName()); @@ -50,7 +49,14 @@ public ProcessInfoProfiler(Reporter reporter) { @Override public long getIntervalMillis() { - return 0; + return -1; + } + + @Override + public void setIntervalMillis(long millis) { + if (millis > 0) { + throw new IllegalStateException("Cannot schedule " + PROFILER_NAME + " profiler"); + } } @Override diff --git a/src/main/java/com/uber/profiling/profilers/ProfilerBase.java b/src/main/java/com/uber/profiling/profilers/ProfilerBase.java index 267e353..a063d0b 100644 --- a/src/main/java/com/uber/profiling/profilers/ProfilerBase.java +++ b/src/main/java/com/uber/profiling/profilers/ProfilerBase.java @@ -16,6 +16,7 @@ package com.uber.profiling.profilers; +import com.uber.profiling.Profiler; import com.uber.profiling.util.NetworkUtils; import com.uber.profiling.util.ProcFileUtils; import com.uber.profiling.util.ProcessUtils; @@ -23,7 +24,7 @@ import java.util.UUID; -public class ProfilerBase { +public abstract class ProfilerBase extends Profiler { private String tag = null; private String cluster = null; private String hostName = null; diff --git a/src/main/java/com/uber/profiling/profilers/StacktraceCollectorProfiler.java b/src/main/java/com/uber/profiling/profilers/StacktraceCollectorProfiler.java index 09ede1d..547b433 100644 --- a/src/main/java/com/uber/profiling/profilers/StacktraceCollectorProfiler.java +++ b/src/main/java/com/uber/profiling/profilers/StacktraceCollectorProfiler.java @@ -31,7 +31,8 @@ /** * This class collects stacktraces by getting thread dump via JMX, and stores the stacktraces into the given buffer. */ -public class StacktraceCollectorProfiler implements Profiler { +public class StacktraceCollectorProfiler extends Profiler { + public static final String PROFILER_NAME = "StacktraceCollector"; private long intervalMillis; private StacktraceMetricBuffer buffer; private String ignoreThreadNamePrefix = ""; @@ -63,6 +64,9 @@ public void setReporter(Reporter reporter) { @Override public void profile() { + if (getIntervalMillis() <=0){ + return; + } ThreadInfo[] threadInfos = threadMXBean.dumpAllThreads(false, false); if (threadInfos == null) { return; diff --git a/src/main/java/com/uber/profiling/profilers/StacktraceReporterProfiler.java b/src/main/java/com/uber/profiling/profilers/StacktraceReporterProfiler.java index 25c39e2..e6218f0 100644 --- a/src/main/java/com/uber/profiling/profilers/StacktraceReporterProfiler.java +++ b/src/main/java/com/uber/profiling/profilers/StacktraceReporterProfiler.java @@ -16,7 +16,6 @@ package com.uber.profiling.profilers; -import com.uber.profiling.Profiler; import com.uber.profiling.Reporter; import com.uber.profiling.reporters.ConsoleOutputReporter; import com.uber.profiling.util.ClassAndMethod; @@ -32,7 +31,7 @@ /** * This class reads the stacktraces from the given buffer and send out via given reporter. */ -public class StacktraceReporterProfiler extends ProfilerBase implements Profiler { +public class StacktraceReporterProfiler extends ProfilerBase { public static final String PROFILER_NAME = "Stacktrace"; private StacktraceMetricBuffer buffer; @@ -41,6 +40,10 @@ public class StacktraceReporterProfiler extends ProfilerBase implements Profiler private long intervalMillis = Constants.DEFAULT_METRIC_INTERVAL; + public StacktraceMetricBuffer getBuffer() { + return buffer; + } + public StacktraceReporterProfiler(StacktraceMetricBuffer buffer, Reporter reporter) { this.buffer = buffer; this.reporter = reporter; diff --git a/src/main/java/com/uber/profiling/reporters/ConsoleOutputReporter.java b/src/main/java/com/uber/profiling/reporters/ConsoleOutputReporter.java index 37b7c11..306ad56 100644 --- a/src/main/java/com/uber/profiling/reporters/ConsoleOutputReporter.java +++ b/src/main/java/com/uber/profiling/reporters/ConsoleOutputReporter.java @@ -18,8 +18,6 @@ import com.uber.profiling.Reporter; import com.uber.profiling.util.JsonUtils; - -import java.util.List; import java.util.Map; public class ConsoleOutputReporter implements Reporter { diff --git a/src/main/java/com/uber/profiling/tools/AttachClient.java b/src/main/java/com/uber/profiling/tools/AttachClient.java new file mode 100644 index 0000000..4feaf1e --- /dev/null +++ b/src/main/java/com/uber/profiling/tools/AttachClient.java @@ -0,0 +1,20 @@ +package com.uber.profiling.tools; + +import com.sun.tools.attach.VirtualMachine; + +public class AttachClient { + + public static void main(String[] args) throws Exception { + if (args.length != 3) { + System.out.println("Usage: com.uber.profiling.tools.AttachClient "); + return; + } + String agent_path = args[0]; + String agent_args = args[1]; + String pid = args[2]; + VirtualMachine vm = VirtualMachine.attach(pid); + System.out.println("vm = " + vm); + vm.loadAgent(agent_path, agent_args); + vm.detach(); + } +} diff --git a/src/main/java/com/uber/profiling/util/StringUtils.java b/src/main/java/com/uber/profiling/util/StringUtils.java index 1d96a4c..defda12 100644 --- a/src/main/java/com/uber/profiling/util/StringUtils.java +++ b/src/main/java/com/uber/profiling/util/StringUtils.java @@ -17,11 +17,8 @@ package com.uber.profiling.util; import org.apache.commons.lang3.math.NumberUtils; - import java.util.ArrayList; -import java.util.HashSet; import java.util.List; -import java.util.Set; import java.util.regex.Matcher; import java.util.regex.Pattern; diff --git a/src/test/java/com/uber/profiling/AgentITCase.java b/src/test/java/com/uber/profiling/AgentITCase.java index e221699..5823b2b 100644 --- a/src/test/java/com/uber/profiling/AgentITCase.java +++ b/src/test/java/com/uber/profiling/AgentITCase.java @@ -16,6 +16,7 @@ package com.uber.profiling; +import java.lang.reflect.Field; import org.junit.Assert; import org.junit.Test; @@ -25,9 +26,7 @@ import java.nio.file.Paths; import java.util.Arrays; import java.util.Comparator; -import java.util.HashMap; import java.util.List; -import java.util.Map; import java.util.stream.Collectors; public class AgentITCase { @@ -252,6 +251,82 @@ public void runAgent_appIdVariable() throws InterruptedException, IOException { System.out.println(jsonProcessInfo); Assert.assertTrue(jsonProcessInfo.contains("TEST_APP_ID_123_ABC")); } + + @Test + public void runAgent_updateAgentParametersAfterStart() throws InterruptedException, IOException { + String javaHome = System.getProperty("java.home"); + System.out.println(javaHome); + String javaBin = Paths.get(javaHome, "bin/java").toAbsolutePath().toString(); + + String agentJar = getAgentJarPath(); + + String outputDir = Files.createTempDirectory("jvm_profiler_test_output").toString(); + System.out.println("outputDir: " + outputDir); + + ProcessBuilder pb = new ProcessBuilder( + javaBin, + "-cp", + agentJar, + "-javaagent:" + agentJar + "=configProvider=com.uber.profiling.util.DummyConfigProvider,reporter=com.uber.profiling.reporters.FileOutputReporter,outputDir=" + outputDir + ",tag=mytag,metricInterval=200,durationProfiling=com.uber.profiling.examples.HelloWorldApplication.publicSleepMethod,argumentProfiling=com.uber.profiling.examples.HelloWorldApplication.publicSleepMethod.0", + "com.uber.profiling.examples.HelloWorldApplication", + "2000" + ); + + pb.redirectOutput(ProcessBuilder.Redirect.INHERIT); + pb.redirectError(ProcessBuilder.Redirect.INHERIT); + + Process process = pb.start(); + + // update the agent params using com.uber.profiling.tools.AttachClient + long pid = tryGetPid(process); + Assert.assertTrue(pid > 0); + + ProcessBuilder ac = new ProcessBuilder( + javaBin, + "-cp", + String.format("%s:%s/../lib/tools.jar", agentJar, javaHome), + "com.uber.profiling.tools.AttachClient", + agentJar, + "configProvider=com.uber.profiling.util.DummyConfigProvider,reporter=com.uber.profiling.reporters.FileOutputReporter,outputDir=" + outputDir + ",tag=mytag,metricInterval=300,durationProfiling=com.uber.profiling.examples.HelloWorldApplication.publicSleepMethod,argumentProfiling=com.uber.profiling.examples.HelloWorldApplication.publicSleepMethod.0", + String.valueOf(pid) + ); + ac.redirectOutput(ProcessBuilder.Redirect.INHERIT); + ac.redirectError(ProcessBuilder.Redirect.INHERIT); + Process attachClientProcess = ac.start(); + attachClientProcess.waitFor(); + + process.waitFor(); + + File[] files = new File(outputDir).listFiles(); + Assert.assertEquals(4, files.length); + + List fileNames = Arrays.asList(files).stream().map(t->t.getName()).sorted().collect(Collectors.toList()); + + Assert.assertEquals("CpuAndMemory.json", fileNames.get(0)); + String jsonCpuAndMemory = new String(Files.readAllBytes(Paths.get(outputDir, fileNames.get(0)))); + System.out.println("-----CpuAndMemory-----"); + System.out.println(jsonCpuAndMemory); + Assert.assertTrue(jsonCpuAndMemory.contains("bufferPool")); + + Assert.assertEquals("MethodArgument.json", fileNames.get(1)); + String jsonMethodArgument = new String(Files.readAllBytes(Paths.get(outputDir, fileNames.get(1)))); + System.out.println("-----MethodArgument-----"); + System.out.println(jsonMethodArgument); + Assert.assertTrue(jsonMethodArgument.contains("arg.0")); + + Assert.assertEquals("MethodDuration.json", fileNames.get(2)); + String jsonMethodDuration = new String(Files.readAllBytes(Paths.get(outputDir, fileNames.get(2)))); + System.out.println("-----MethodDuration-----"); + System.out.println(jsonMethodDuration); + Assert.assertTrue(jsonMethodDuration.contains("duration.sum")); + + Assert.assertEquals("ProcessInfo.json", fileNames.get(3)); + String jsonProcessInfo = new String(Files.readAllBytes(Paths.get(outputDir, fileNames.get(3)))); + System.out.println("-----ProcessInfo-----"); + System.out.println(jsonProcessInfo); + Assert.assertTrue(jsonProcessInfo.contains("jvmClassPath")); + Assert.assertTrue(jsonProcessInfo.contains(agentJar)); + } private String getAgentJarPath() throws IOException { // Find jar file with largest size under target directory, which should be the packaged agent jar file @@ -269,4 +344,22 @@ private String getAgentJarPath() throws IOException { System.out.println("agentJar: " + agentJar); return agentJar; } + + private int tryGetPid(Process process) + { + if (process.getClass().getName().equals("java.lang.UNIXProcess")) + { + try + { + Field f = process.getClass().getDeclaredField("pid"); + f.setAccessible(true); + return f.getInt(process); + } + catch (IllegalAccessException | IllegalArgumentException | NoSuchFieldException | SecurityException e) + { + } + } + + return 0; + } } diff --git a/src/test/java/com/uber/profiling/ProfilerRunnableTest.java b/src/test/java/com/uber/profiling/ProfilerTest.java similarity index 86% rename from src/test/java/com/uber/profiling/ProfilerRunnableTest.java rename to src/test/java/com/uber/profiling/ProfilerTest.java index 8f81dd9..3e74448 100644 --- a/src/test/java/com/uber/profiling/ProfilerRunnableTest.java +++ b/src/test/java/com/uber/profiling/ProfilerTest.java @@ -21,17 +21,21 @@ import java.util.concurrent.atomic.AtomicInteger; -public class ProfilerRunnableTest { +public class ProfilerTest { @Test public void invokeRunnable() { final AtomicInteger i = new AtomicInteger(10); - ProfilerRunner profilerRunnable = new ProfilerRunner(new Profiler() { + Profiler profilerRunnable = new Profiler() { @Override public long getIntervalMillis() { return 0; } + @Override + public void setIntervalMillis(long millis) { + } + @Override public void setReporter(Reporter reporter) { } @@ -40,7 +44,7 @@ public void setReporter(Reporter reporter) { public void profile() { i.incrementAndGet(); } - }); + }; profilerRunnable.run(); diff --git a/src/test/java/com/uber/profiling/profilers/ProcessInfoProfilerTest.java b/src/test/java/com/uber/profiling/profilers/ProcessInfoProfilerTest.java index 3494376..da08bb1 100644 --- a/src/test/java/com/uber/profiling/profilers/ProcessInfoProfilerTest.java +++ b/src/test/java/com/uber/profiling/profilers/ProcessInfoProfilerTest.java @@ -44,7 +44,7 @@ public void close() { } }); - Assert.assertEquals(0L, profiler.getIntervalMillis()); + Assert.assertEquals(-1L, profiler.getIntervalMillis()); profiler.profile(); profiler.profile(); From fa975e6b96ba2fd33462eee68597aeef30385590 Mon Sep 17 00:00:00 2001 From: Adi Muraru Date: Fri, 24 May 2019 22:48:05 +0300 Subject: [PATCH 2/2] implemented review --- src/main/java/com/uber/profiling/AgentImpl.java | 4 ++-- src/main/java/com/uber/profiling/Profiler.java | 10 ++++++++-- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/src/main/java/com/uber/profiling/AgentImpl.java b/src/main/java/com/uber/profiling/AgentImpl.java index c8e6d75..88304db 100644 --- a/src/main/java/com/uber/profiling/AgentImpl.java +++ b/src/main/java/com/uber/profiling/AgentImpl.java @@ -237,9 +237,9 @@ private void scheduleProfilers() { } for (Profiler profiler : profilers.values()) { - if (profiler.getScheduleHandler() != null) { + if (profiler.isRunning()) { //cancel previous task if already scheduled - profiler.getScheduleHandler().cancel(false); + profiler.cancel(); } if (profiler.getIntervalMillis() <= 0){ //one time profiler, don't schedule diff --git a/src/main/java/com/uber/profiling/Profiler.java b/src/main/java/com/uber/profiling/Profiler.java index a212641..394ef37 100644 --- a/src/main/java/com/uber/profiling/Profiler.java +++ b/src/main/java/com/uber/profiling/Profiler.java @@ -50,8 +50,14 @@ public void run() { } } - public ScheduledFuture getScheduleHandler() { - return scheduleHandler; + public boolean isRunning() { + return scheduleHandler != null; + } + + public void cancel() { + if (isRunning()) { + scheduleHandler.cancel(false); + } } public void setScheduleHandler(ScheduledFuture handler) {