Support async profiler feature #720

zhengziyi0117 · 2024-09-29T15:14:05Z

Integrate async profiler performance analysis function in Java

If this pull request closes/resolves/fixes an existing issue, replace the issue number. Closes #.
Update the CHANGES log.

apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/conf/Config.java

apm-sniffer/config/agent.config

...core/src/main/java/org/apache/skywalking/apm/agent/core/asyncprofiler/AsyncProfilerTask.java

...rc/main/java/org/apache/skywalking/apm/agent/core/asyncprofiler/AsyncProfilerDataSender.java

lujiajing1126 · 2024-10-25T06:57:51Z

Pls fix CI

…/apm/agent/core/conf/Config.java Co-authored-by: Jiajing LU <[email protected]>

lujiajing1126 · 2024-10-28T10:31:22Z

...rc/main/java/org/apache/skywalking/apm/agent/core/asyncprofiler/AsyncProfilerDataSender.java

+         * Wait briefly to see if the server can receive the jfr. If the server cannot receive it, onError will be triggered.
+         * Then we wait for a while (waiting for the server to send onError) and then decide whether to send the jfr file.
+         */
+        Thread.sleep(500);


@wu-sheng In the current design, the Java Agent has to wait after the first request in order to determine whether the OAP accepts the following data transfer. This is a kind of negotiation.

WDYT?

If you need to wait for the server, you should change AsyncProfilerTask service to dual streaming, and the response needs to be a kind of ack

500ms can't guarantee anything AFAIK.

If you need to wait for the server, you should change AsyncProfilerTask service to dual streaming, and the response needs to be a kind of ack

Sure. Let's use dual streaming @zhengziyi0117

Note, in this case, Command should not be the response anymore.

wu-sheng · 2024-10-28T12:40:14Z

...rc/main/java/org/apache/skywalking/apm/agent/core/asyncprofiler/AsyncProfilerDataSender.java

+         */
+        Thread.sleep(500);
+
+        // Is it possible to upload jfr?


What does this comment mean?

wu-sheng · 2024-10-28T12:49:14Z

pom.xml

+            <dependency>
+                <groupId>tools.profiler</groupId>
+                <artifactId>async-profiler</artifactId>
+                <version>${async-profiler.version}</version>
+            </dependency>


Please move this the last one rather than the first.

wu-sheng · 2024-10-28T12:49:17Z

pom.xml

@@ -87,6 +87,7 @@

        <!-- core lib dependency -->
        <bytebuddy.version>1.14.9</bytebuddy.version>
+        <async-profiler.version>3.0</async-profiler.version>


Please move this the last one rather than the first.

lujiajing1126

LGTM if all the current comments are resolved

lujiajing1126 · 2024-10-30T01:02:30Z

...va/org/apache/skywalking/apm/agent/core/asyncprofiler/AsyncProfilerTaskExecutionService.java

+
+    @Override
+    public void shutdown() throws Throwable {
+        scheduledFuture.cancel(true);


you have to check NULL!

wu-sheng · 2024-10-30T03:52:22Z

...java/org/apache/skywalking/apm/agent/core/asyncprofiler/AsyncProfilerTaskChannelService.java

+            Commands commands = asyncProfilerTaskBlockingStub.withDeadlineAfter(GRPC_UPSTREAM_TIMEOUT, TimeUnit.SECONDS)
+                    .getAsyncProfilerTaskCommands(query);
+            ServiceManager.INSTANCE.findService(CommandService.class).receiveCommand(commands);


You should check service not implemented error of gRPC, you could try that by connecting v < 10.1 OAP.

Once it is not implemented, the agent should shutdown this part automatically.

wu-sheng · 2024-10-30T03:54:08Z

apm-sniffer/config/agent.config

@@ -164,6 +164,12 @@ profile.duration=${SW_AGENT_PROFILE_DURATION:10}
 profile.dump_max_stack_depth=${SW_AGENT_PROFILE_DUMP_MAX_STACK_DEPTH:500}
 # Snapshot transport to backend buffer size
 profile.snapshot_transport_buffer_size=${SW_AGENT_PROFILE_SNAPSHOT_TRANSPORT_BUFFER_SIZE:4500}
+# If true, Async Profiler will be enabled when user creates a new async profiler task, Otherwise it is disabled.


This comment is not accurate. You hard coded this value as true, so, the default value is TRUE, not disabled. Please update your default value in the codes.

...java/org/apache/skywalking/apm/network/trace/component/command/AsyncProfilerTaskCommand.java

wu-sheng · 2024-10-30T06:48:10Z

...rc/main/java/org/apache/skywalking/apm/agent/core/asyncprofiler/AsyncProfilerDataSender.java

+@DefaultImplementor
+public class AsyncProfilerDataSender implements BootService, GRPCChannelListener {
+    private static final ILog LOGGER = LogManager.getLogger(ProfileSnapshotSender.class);
+    private static final int DATA_CHUNK_SIZE = 1024 * 1024;


This should move into config. Don't have to expose through agent.config, but we don't need to hard code this.

wu-sheng · 2024-10-30T06:50:59Z

...rc/main/java/org/apache/skywalking/apm/agent/core/asyncprofiler/AsyncProfilerDataSender.java

+                status.finished();
+                ServiceManager.INSTANCE.findService(GRPCChannelManager.class).reportError(t);


When onError, we should output logs.

...rc/main/java/org/apache/skywalking/apm/agent/core/asyncprofiler/AsyncProfilerDataSender.java

wu-sheng · 2024-10-30T06:53:32Z

...va/org/apache/skywalking/apm/agent/core/asyncprofiler/AsyncProfilerTaskExecutionService.java

+
+    private static final ILog LOGGER = LogManager.getLogger(AsyncProfilerTaskChannelService.class);
+
+    private static final AsyncProfiler ASYNC_PROFILER = AsyncProfiler.getInstance();


Please use lazy initialization as much as possible. I am not sure how much effects would this getInstance takes, but as service list is loaded before agent instrumentation, we should load as less code as possible. Otherwise, the instrumentation could fail due to classes have been loaded.

wu-sheng · 2024-10-30T06:53:59Z

...va/org/apache/skywalking/apm/agent/core/asyncprofiler/AsyncProfilerTaskExecutionService.java

+    private static final String SUCCESS_RESULT = "Profiling started\n";
+
+    // profile executor thread pool, only running one thread
+    private static final ScheduledExecutorService ASYNC_PROFILER_EXECUTOR = Executors.newSingleThreadScheduledExecutor(


Same here, please initialize later in the process.

…/apm/agent/core/asyncprofiler/AsyncProfilerDataSender.java Co-authored-by: 吴晟 Wu Sheng <[email protected]>

...va/org/apache/skywalking/apm/agent/core/asyncprofiler/AsyncProfilerTaskExecutionService.java

…/apm/agent/core/asyncprofiler/AsyncProfilerTaskExecutionService.java Co-authored-by: 吴晟 Wu Sheng <[email protected]>

lujiajing1126

LGTM

wu-sheng · 2024-10-30T08:10:34Z

...java/org/apache/skywalking/apm/agent/core/asyncprofiler/AsyncProfilerTaskChannelService.java

+                    return;
+                }
+                final StatusRuntimeException statusRuntimeException = (StatusRuntimeException) t;
+                if (statusRuntimeException.getStatus().getCode() == Status.Code.UNIMPLEMENTED) {


Please use Status.Code.UNIMPLEMENTED.equals. It is safer.

lujiajing1126 mentioned this pull request Sep 30, 2024

[OSPP] Async Profiler integration with Java Agent apache/skywalking#12673

Closed

7 tasks

feat: support async-profiler

5adec9b

zhengziyi0117 force-pushed the async-profiler branch from 4f73937 to 5adec9b Compare September 30, 2024 05:35

zhengziyi added 2 commits October 11, 2024 14:50

fix: reduce the default max_duration of async-profiler

5a88a8f

fix: Cooperate with proto modification

e5243b2

lujiajing1126 requested changes Oct 25, 2024

View reviewed changes

apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/conf/Config.java Show resolved Hide resolved

lujiajing1126 requested changes Oct 25, 2024

View reviewed changes

...core/src/main/java/org/apache/skywalking/apm/agent/core/asyncprofiler/AsyncProfilerTask.java Outdated Show resolved Hide resolved

lujiajing1126 requested changes Oct 25, 2024

View reviewed changes

...rc/main/java/org/apache/skywalking/apm/agent/core/asyncprofiler/AsyncProfilerDataSender.java Outdated Show resolved Hide resolved

lujiajing1126 added the enhancement New feature or request label Oct 25, 2024

lujiajing1126 assigned zhengziyi0117 Oct 25, 2024

lujiajing1126 added this to the 9.4.0 milestone Oct 25, 2024

zhengziyi0117 and others added 7 commits October 25, 2024 15:12

Update apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking…

c236129

…/apm/agent/core/conf/Config.java Co-authored-by: Jiajing LU <[email protected]>

Update apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking…

7417103

…/apm/agent/core/conf/Config.java Co-authored-by: Jiajing LU <[email protected]>

fix: proto git sha

d327dd2

Update apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking…

1a417ac

…/apm/agent/core/conf/Config.java Co-authored-by: Jiajing LU <[email protected]>

Update apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking…

3b59da7

…/apm/agent/core/conf/Config.java Co-authored-by: Jiajing LU <[email protected]>

Merge remote-tracking branch 'origin/async-profiler' into async-profiler

fdbb15c

fix: comment

faa8732

lujiajing1126 self-requested a review October 28, 2024 02:12

zhengziyi added 2 commits October 28, 2024 10:25

fix: comment & data sender

fbc24ef

fix: data sender and server communication

dbb7bff

lujiajing1126 reviewed Oct 28, 2024

View reviewed changes

Merge branch 'main' into async-profiler

9aa2ec4

wu-sheng reviewed Oct 28, 2024

View reviewed changes

zhengziyi added 2 commits October 28, 2024 23:06

fix: data sender

2f1cdd8

fix: data sender

bcc5f30

lujiajing1126 requested changes Oct 30, 2024

View reviewed changes

fix: some issue

e66546a

wu-sheng reviewed Oct 30, 2024

View reviewed changes

...java/org/apache/skywalking/apm/network/trace/component/command/AsyncProfilerTaskCommand.java Show resolved Hide resolved

zhengziyi added 4 commits October 30, 2024 11:56

fix: unit test

2f28e5f

fix: comment

17990e0

fix: unit test

d6dcbff

fix: comment

e8aef4c

wu-sheng reviewed Oct 30, 2024

View reviewed changes

...rc/main/java/org/apache/skywalking/apm/agent/core/asyncprofiler/AsyncProfilerDataSender.java Outdated Show resolved Hide resolved

wu-sheng reviewed Oct 30, 2024

View reviewed changes

zhengziyi and others added 2 commits October 30, 2024 15:06

fix: lazy init && config

b3fe849

Update apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking…

4a6d7f5

…/apm/agent/core/asyncprofiler/AsyncProfilerDataSender.java Co-authored-by: 吴晟 Wu Sheng <[email protected]>

wu-sheng reviewed Oct 30, 2024

View reviewed changes

...va/org/apache/skywalking/apm/agent/core/asyncprofiler/AsyncProfilerTaskExecutionService.java Outdated Show resolved Hide resolved

wu-sheng reviewed Oct 30, 2024

View reviewed changes

...va/org/apache/skywalking/apm/agent/core/asyncprofiler/AsyncProfilerTaskExecutionService.java Outdated Show resolved Hide resolved

zhengziyi and others added 6 commits October 30, 2024 15:13

fix: volatile

599fb98

Merge remote-tracking branch 'origin/async-profiler' into async-profiler

2ccbd23

Update apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking…

0435607

…/apm/agent/core/asyncprofiler/AsyncProfilerTaskExecutionService.java Co-authored-by: 吴晟 Wu Sheng <[email protected]>

Update apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking…

efa9ce9

…/apm/agent/core/asyncprofiler/AsyncProfilerTaskExecutionService.java Co-authored-by: 吴晟 Wu Sheng <[email protected]>

fix: log grammar

bbd987f

Merge remote-tracking branch 'origin/async-profiler' into async-profiler

071d062

lujiajing1126 previously approved these changes Oct 30, 2024

View reviewed changes

wu-sheng reviewed Oct 30, 2024

View reviewed changes

fix: log && code format

d09acd4

zhengziyi0117 dismissed lujiajing1126’s stale review via d09acd4 October 30, 2024 08:25

wu-sheng approved these changes Oct 30, 2024

View reviewed changes

wu-sheng merged commit 2027a98 into apache:main Oct 30, 2024
191 of 192 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support async profiler feature #720

Support async profiler feature #720

zhengziyi0117 commented Sep 29, 2024

lujiajing1126 commented Oct 25, 2024

lujiajing1126 Oct 28, 2024

wu-sheng Oct 28, 2024

wu-sheng Oct 28, 2024

lujiajing1126 Oct 28, 2024

wu-sheng Oct 28, 2024

wu-sheng Oct 28, 2024

wu-sheng Oct 28, 2024

wu-sheng Oct 28, 2024

lujiajing1126 left a comment

lujiajing1126 Oct 30, 2024

wu-sheng Oct 30, 2024

wu-sheng Oct 30, 2024

wu-sheng Oct 30, 2024

wu-sheng Oct 30, 2024

wu-sheng Oct 30, 2024

wu-sheng Oct 30, 2024

wu-sheng Oct 30, 2024

lujiajing1126 left a comment

wu-sheng Oct 30, 2024

		status.finished();
		ServiceManager.INSTANCE.findService(GRPCChannelManager.class).reportError(t);


		private static final ILog LOGGER = LogManager.getLogger(AsyncProfilerTaskChannelService.class);

		private static final AsyncProfiler ASYNC_PROFILER = AsyncProfiler.getInstance();

Support async profiler feature #720

Support async profiler feature #720

Conversation

zhengziyi0117 commented Sep 29, 2024

lujiajing1126 commented Oct 25, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lujiajing1126 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lujiajing1126 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment