Copilot commented on code in PR #13524: URL: https://github.com/apache/skywalking/pull/13524#discussion_r2477359906
########## test/e2e-v2/cases/profiling/trace/go/expected/profile-list-finished.yml: ########## @@ -0,0 +1,34 @@ +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You 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. + Review Comment: Line 15 contains trailing whitespace that should be removed. ```suggestion ``` ########## oap-server/server-receiver-plugin/skywalking-profile-receiver-plugin/src/main/java/org/apache/skywalking/oap/server/receiver/profile/provider/handler/ProfileTaskServiceHandler.java: ########## @@ -130,6 +139,92 @@ public void onCompleted() { }; } + @Override + public StreamObserver<GoProfileData> goProfileReport(StreamObserver<Commands> responseObserver) { + return new StreamObserver<GoProfileData>() { + private ByteArrayOutputStream profileDataBuffer = new ByteArrayOutputStream(); + private String currentTaskId = null; + + @Override + public void onNext(GoProfileData profileData) { + LOGGER.debug("receive go profile data: taskId='{}', payloadSize={}, isLast={}", + profileData.getTaskId(), + profileData.getPayload().size(), + profileData.getIsLast()); + + // Check if taskId is empty - this indicates a problem with Go agent + if (profileData.getTaskId() == null || profileData.getTaskId().isEmpty()) { + LOGGER.error("Go agent sent empty taskId! This indicates a problem with Go agent's profile task management. " + + "Please check Go agent's profile task creation and task.TaskID assignment."); + return; + } + + // Reset state if this is a new task + if (currentTaskId == null || !currentTaskId.equals(profileData.getTaskId())) { + currentTaskId = profileData.getTaskId(); + profileDataBuffer.reset(); + LOGGER.debug("Starting new task: {}", currentTaskId); + } + + // Collect profile data + try { + profileDataBuffer.write(profileData.getPayload().toByteArray()); + } catch (IOException e) { + LOGGER.error("Failed to write Go profile data", e); + return; + } + + // If this is the last data chunk, parse and store Review Comment: Line 177 has incorrect indentation - mixed spaces/tabs. Should align with surrounding code indentation. ```suggestion // If this is the last data chunk, parse and store ``` ########## oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/profiling/trace/ProfileLanguageType.java: ########## @@ -0,0 +1,47 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You 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 org.apache.skywalking.oap.server.core.profiling.trace; + +/** + * Language type for profile records. Stored as int in storage for compatibility. + */ +public enum ProfileLanguageType { + JAVA(0), + GO(1); + + private final int value; + + ProfileLanguageType(int value) { + this.value = value; + } + + public int getValue() { + return value; + } + + public static ProfileLanguageType fromValue(int value) { + for (ProfileLanguageType language : values()) { + if (language.value == value) { + return language; + } + } + return JAVA; // default to Java + } +} + + Review Comment: Remove trailing blank line at end of file. ```suggestion ``` ########## oap-server/server-receiver-plugin/skywalking-profile-receiver-plugin/src/main/java/org/apache/skywalking/oap/server/receiver/profile/provider/handler/ProfileTaskServiceHandler.java: ########## @@ -130,6 +139,92 @@ public void onCompleted() { }; } + @Override + public StreamObserver<GoProfileData> goProfileReport(StreamObserver<Commands> responseObserver) { + return new StreamObserver<GoProfileData>() { + private ByteArrayOutputStream profileDataBuffer = new ByteArrayOutputStream(); + private String currentTaskId = null; + + @Override + public void onNext(GoProfileData profileData) { + LOGGER.debug("receive go profile data: taskId='{}', payloadSize={}, isLast={}", + profileData.getTaskId(), + profileData.getPayload().size(), + profileData.getIsLast()); + + // Check if taskId is empty - this indicates a problem with Go agent + if (profileData.getTaskId() == null || profileData.getTaskId().isEmpty()) { + LOGGER.error("Go agent sent empty taskId! This indicates a problem with Go agent's profile task management. " + + "Please check Go agent's profile task creation and task.TaskID assignment."); + return; + } + + // Reset state if this is a new task + if (currentTaskId == null || !currentTaskId.equals(profileData.getTaskId())) { + currentTaskId = profileData.getTaskId(); + profileDataBuffer.reset(); + LOGGER.debug("Starting new task: {}", currentTaskId); + } + + // Collect profile data Review Comment: Line 169 has incorrect indentation - mixed spaces/tabs. Should align with surrounding code indentation. ```suggestion // Collect profile data ``` ########## oap-server/server-library/library-pprof-parser/src/main/java/org/apache/skywalking/oap/server/library/pprof/parser/PprofSegmentParser.java: ########## @@ -0,0 +1,327 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You 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 org.apache.skywalking.oap.server.library.pprof.parser; + +import com.google.perftools.profiles.ProfileProto; +import java.io.IOException; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.LinkedHashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.UUID; + +/** + * Parse pprof profile data and extract segment information + */ +public class PprofSegmentParser { + + /** + * Segment information extracted from pprof labels + */ + public static class SegmentInfo { + private String segmentId; + private String traceId; + private String spanId; + private String serviceInstanceId; + private List<String> stack; + private long count; + + public String getSegmentId() { + return segmentId; + } + + public void setSegmentId(String segmentId) { + this.segmentId = segmentId; + } + + public String getTraceId() { + return traceId; + } + + public void setTraceId(String traceId) { + this.traceId = traceId; + } + + public String getSpanId() { + return spanId; + } + + public void setSpanId(String spanId) { + this.spanId = spanId; + } + + public String getServiceInstanceId() { + return serviceInstanceId; + } + + public void setServiceInstanceId(String serviceInstanceId) { + this.serviceInstanceId = serviceInstanceId; + } + + public List<String> getStack() { + return stack; + } + + public void setStack(List<String> stack) { + this.stack = stack; + } + + public long getCount() { + return count; + } + + public void setCount(long count) { + this.count = count; + } + } + + /** + * Parse pprof profile and extract all segment information + */ + public static List<SegmentInfo> parseSegments(ProfileProto.Profile profile) throws IOException { + List<SegmentInfo> segments = new ArrayList<>(); + List<String> stringTable = profile.getStringTableList(); + + // Group samples by segmentId + Map<String, List<ProfileProto.Sample>> segmentSamples = new HashMap<>(); + + for (ProfileProto.Sample sample : profile.getSampleList()) { + String segmentId = extractSegmentIdFromLabels(sample.getLabelList(), stringTable); + if (segmentId != null) { + segmentSamples.computeIfAbsent(segmentId, k -> new ArrayList<>()).add(sample); + } + } + + // Create SegmentInfo for each segment + for (Map.Entry<String, List<ProfileProto.Sample>> entry : segmentSamples.entrySet()) { + String segmentId = entry.getKey(); + List<ProfileProto.Sample> samples = entry.getValue(); + + SegmentInfo segmentInfo = new SegmentInfo(); + segmentInfo.setSegmentId(segmentId); + + // Extract basic information from first sample + ProfileProto.Sample firstSample = samples.get(0); + segmentInfo.setTraceId(extractTraceIdFromLabels(firstSample.getLabelList(), stringTable)); + segmentInfo.setSpanId(extractSpanIdFromLabels(firstSample.getLabelList(), stringTable)); + segmentInfo.setServiceInstanceId(extractServiceInstanceIdFromLabels(firstSample.getLabelList(), stringTable)); + + // Merge call stacks from all samples + List<String> combinedStack = extractCombinedStackFromSamples(samples, profile); + segmentInfo.setStack(combinedStack); + + // Calculate total sample count + long totalCount = samples.stream() + .mapToLong(sample -> sample.getValueCount() > 0 ? sample.getValue(0) : 1) + .sum(); + segmentInfo.setCount(totalCount); + + segments.add(segmentInfo); + } + + return segments; + } + + /** + * Extract segmentId from labels + */ + public static String extractSegmentIdFromLabels(List<ProfileProto.Label> labels, List<String> stringTable) { + for (ProfileProto.Label label : labels) { + String key = getStringFromTable(label.getKey(), stringTable); + if (key != null && (key.equals("segment_id") || key.equals("trace_segment_id") || + key.equals("segmentId") || key.equals("traceSegmentId") || + key.equals("traceSegmentID"))) { + return getStringFromTable(label.getStr(), stringTable); + } + } + return null; + } + + /** + * Extract traceId from labels + */ + private static String extractTraceIdFromLabels(List<ProfileProto.Label> labels, List<String> stringTable) { + for (ProfileProto.Label label : labels) { + String key = getStringFromTable(label.getKey(), stringTable); + if (key != null && (key.equals("trace_id") || key.equals("traceId") || key.equals("traceID"))) { + return getStringFromTable(label.getStr(), stringTable); + } + } + return "go_trace_" + UUID.randomUUID().toString().replace("-", ""); + } + + /** + * Extract spanId from labels + */ + private static String extractSpanIdFromLabels(List<ProfileProto.Label> labels, List<String> stringTable) { + for (ProfileProto.Label label : labels) { + String key = getStringFromTable(label.getKey(), stringTable); + if (key != null && (key.equals("span_id") || key.equals("spanId") || key.equals("spanID"))) { + return getStringFromTable(label.getStr(), stringTable); + } + } + return null; // spanId is optional + } + + /** + * Extract serviceInstanceId from labels + */ + private static String extractServiceInstanceIdFromLabels(List<ProfileProto.Label> labels, List<String> stringTable) { + for (ProfileProto.Label label : labels) { + String key = getStringFromTable(label.getKey(), stringTable); + if (key != null && (key.equals("service_instance_id") || key.equals("serviceInstanceId") || + key.equals("instance_id") || key.equals("instanceId"))) { + return getStringFromTable(label.getStr(), stringTable); + } + } + return "go_instance_1"; + } + + /** + * Extract merged call stack from samples + */ + private static List<String> extractCombinedStackFromSamples(List<ProfileProto.Sample> samples, ProfileProto.Profile profile) { + Set<String> uniqueStack = new LinkedHashSet<>(); + + for (ProfileProto.Sample sample : samples) { + List<String> stack = extractStackFromSample(sample, profile); + uniqueStack.addAll(stack); + } + + return new ArrayList<>(uniqueStack); + } + + /** + * Extract call stack from a single sample + */ + public static List<String> extractStackFromSample(ProfileProto.Sample sample, ProfileProto.Profile profile) { + List<String> stack = new ArrayList<>(); + + // Traverse location_id from leaf to root + for (int i = sample.getLocationIdCount() - 1; i >= 0; i--) { + long locationId = sample.getLocationId(i); + + // Delegate signature resolution to PprofParser to avoid duplication + String signature = PprofParser.resolveSignature(locationId, profile); + if (signature != null && !signature.isEmpty()) { + stack.add(signature); + } + } + + return stack; + } + + /** + * Get string from string table + */ + public static String getStringFromTable(long index, List<String> stringTable) { + if (index >= 0 && index < stringTable.size()) { + return stringTable.get((int) index); + } + return null; + } + + /** + * Extract label value from sample labels + */ + public static String extractLabel(ProfileProto.Sample sample, List<String> stringTable, String... keys) { + for (ProfileProto.Label l : sample.getLabelList()) { + String k = getStringFromTable(l.getKey(), stringTable); + if (k == null) { + continue; + } + for (String expect : keys) { + if (k.equals(expect)) { + return getStringFromTable(l.getStr(), stringTable); + } + } + } + return null; + } + + /** + * Extract timestamp from sample labels + */ + public static long extractTimestamp(ProfileProto.Sample sample, List<String> stringTable, boolean isStart) { + String target = isStart ? "startTime" : "endTime"; + for (ProfileProto.Label l : sample.getLabelList()) { + String k = getStringFromTable(l.getKey(), stringTable); + if (k == null) { + continue; + } + if (!target.equalsIgnoreCase(k)) { + continue; + } + long v = l.getNum(); + if (v <= 0) { + try { + String sv = getStringFromTable(l.getStr(), stringTable); + if (sv != null) { + v = Long.parseLong(sv.trim()); + } + } catch (Exception ignored) { + // ignore + } + } + if (v > 0 && v < 1_000_000_000_000L) { + // looks like seconds -> millis + return v * 1000L; + } + return v; + } + return 0L; + } + + /** + * Resolve sampling period in milliseconds from pprof profile + */ + public static long resolvePeriodMillis(ProfileProto.Profile profile) { + try { + long period = profile.getPeriod(); + String unit = null; + if (profile.hasPeriodType()) { + unit = getStringFromTable(profile.getPeriodType().getUnit(), profile.getStringTableList()); + } + if (period > 0) { + if (unit == null || unit.isEmpty() || "nanoseconds".equals(unit) || "nanosecond".equals(unit) || "ns".equals(unit)) { + return Math.max(1L, period / 1_000_000L); + } + if ("microseconds".equals(unit) || "us".equals(unit)) { + return Math.max(1L, period / 1_000L); + } + if ("milliseconds".equals(unit) || "ms".equals(unit)) { + return Math.max(1L, period); + } + if ("seconds".equals(unit) || "s".equals(unit)) { + return Math.max(1L, period * 1000L); + } + if ("hz".equals(unit) || "HZ".equals(unit)) { + // samples per second + return period > 0 ? Math.max(1L, 1000L / Math.max(1L, period)) : 10L; Review Comment: Test is always true, because of [this condition](1). ```suggestion return Math.max(1L, 1000L / Math.max(1L, period)); ``` ########## oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/profiling/trace/analyze/GoProfileAnalyzer.java: ########## @@ -0,0 +1,251 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You 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 org.apache.skywalking.oap.server.core.profiling.trace.analyze; + +import com.google.perftools.profiles.ProfileProto; +import java.util.Collections; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.stream.Collectors; +import java.util.ArrayDeque; +import org.apache.skywalking.oap.server.core.profiling.trace.ProfileThreadSnapshotRecord; +import org.apache.skywalking.oap.server.core.query.input.SegmentProfileAnalyzeQuery; +import org.apache.skywalking.oap.server.core.query.type.ProfileAnalyzation; +import org.apache.skywalking.oap.server.core.query.type.ProfileStackElement; +import org.apache.skywalking.oap.server.core.query.type.ProfileStackTree; +import org.apache.skywalking.oap.server.library.pprof.parser.PprofSegmentParser; +import org.apache.skywalking.oap.server.library.pprof.parser.PprofParser; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * Analyzer for Go pprof samples. Builds a stack tree with total/self durations using sampling period. + * This works independently from ThreadSnapshot, for Go profiles only. + */ +public class GoProfileAnalyzer { + private static final Logger LOGGER = LoggerFactory.getLogger(GoProfileAnalyzer.class); + + /** + * Analyze a pprof profile for a specific segment and time window. + */ + public ProfileAnalyzation analyze(final String segmentId, + final long startTimeInclusive, Review Comment: The parameter 'startTimeInclusive' is never used. ```suggestion ``` ########## oap-server/server-library/library-pprof-parser/src/main/java/org/apache/skywalking/oap/server/library/pprof/parser/PprofSegmentParser.java: ########## @@ -0,0 +1,327 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You 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 org.apache.skywalking.oap.server.library.pprof.parser; + +import com.google.perftools.profiles.ProfileProto; +import java.io.IOException; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.LinkedHashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.UUID; + +/** + * Parse pprof profile data and extract segment information + */ +public class PprofSegmentParser { + + /** + * Segment information extracted from pprof labels + */ + public static class SegmentInfo { + private String segmentId; + private String traceId; + private String spanId; + private String serviceInstanceId; + private List<String> stack; + private long count; + + public String getSegmentId() { + return segmentId; + } + + public void setSegmentId(String segmentId) { + this.segmentId = segmentId; + } + + public String getTraceId() { + return traceId; + } + + public void setTraceId(String traceId) { + this.traceId = traceId; + } + + public String getSpanId() { + return spanId; + } + + public void setSpanId(String spanId) { + this.spanId = spanId; + } + + public String getServiceInstanceId() { + return serviceInstanceId; + } + + public void setServiceInstanceId(String serviceInstanceId) { + this.serviceInstanceId = serviceInstanceId; + } + + public List<String> getStack() { + return stack; + } + + public void setStack(List<String> stack) { + this.stack = stack; + } + + public long getCount() { + return count; + } + + public void setCount(long count) { + this.count = count; + } + } + + /** + * Parse pprof profile and extract all segment information + */ + public static List<SegmentInfo> parseSegments(ProfileProto.Profile profile) throws IOException { + List<SegmentInfo> segments = new ArrayList<>(); Review Comment: Variable 'List<SegmentInfo> segments' is never read. ########## oap-server/server-receiver-plugin/skywalking-profile-receiver-plugin/src/main/java/org/apache/skywalking/oap/server/receiver/profile/provider/handler/ProfileTaskServiceHandler.java: ########## @@ -130,6 +139,92 @@ public void onCompleted() { }; } + @Override + public StreamObserver<GoProfileData> goProfileReport(StreamObserver<Commands> responseObserver) { + return new StreamObserver<GoProfileData>() { + private ByteArrayOutputStream profileDataBuffer = new ByteArrayOutputStream(); + private String currentTaskId = null; + + @Override + public void onNext(GoProfileData profileData) { + LOGGER.debug("receive go profile data: taskId='{}', payloadSize={}, isLast={}", + profileData.getTaskId(), + profileData.getPayload().size(), + profileData.getIsLast()); + + // Check if taskId is empty - this indicates a problem with Go agent + if (profileData.getTaskId() == null || profileData.getTaskId().isEmpty()) { + LOGGER.error("Go agent sent empty taskId! This indicates a problem with Go agent's profile task management. " + + "Please check Go agent's profile task creation and task.TaskID assignment."); + return; + } + + // Reset state if this is a new task + if (currentTaskId == null || !currentTaskId.equals(profileData.getTaskId())) { Review Comment: Line 162 has incorrect indentation - mixed spaces/tabs. Should align with surrounding code indentation. ########## oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/profiling/trace/analyze/GoProfileAnalyzer.java: ########## @@ -0,0 +1,251 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You 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 org.apache.skywalking.oap.server.core.profiling.trace.analyze; + +import com.google.perftools.profiles.ProfileProto; +import java.util.Collections; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.stream.Collectors; +import java.util.ArrayDeque; +import org.apache.skywalking.oap.server.core.profiling.trace.ProfileThreadSnapshotRecord; +import org.apache.skywalking.oap.server.core.query.input.SegmentProfileAnalyzeQuery; +import org.apache.skywalking.oap.server.core.query.type.ProfileAnalyzation; +import org.apache.skywalking.oap.server.core.query.type.ProfileStackElement; +import org.apache.skywalking.oap.server.core.query.type.ProfileStackTree; +import org.apache.skywalking.oap.server.library.pprof.parser.PprofSegmentParser; +import org.apache.skywalking.oap.server.library.pprof.parser.PprofParser; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * Analyzer for Go pprof samples. Builds a stack tree with total/self durations using sampling period. + * This works independently from ThreadSnapshot, for Go profiles only. + */ +public class GoProfileAnalyzer { + private static final Logger LOGGER = LoggerFactory.getLogger(GoProfileAnalyzer.class); + + /** + * Analyze a pprof profile for a specific segment and time window. + */ + public ProfileAnalyzation analyze(final String segmentId, + final long startTimeInclusive, + final long endTimeInclusive, + final ProfileProto.Profile profile) { + final long periodMs = PprofSegmentParser.resolvePeriodMillis(profile); + + // Build ProfileStackElement directly (reuse FrameTreeBuilder's mergeSample logic) + Map<String, Integer> key2Id = new HashMap<>(); // "parentId|name" -> id + List<ProfileStackElement> elements = new ArrayList<>(); + + // Strict per-segment filtering + final List<String> stringTable = profile.getStringTableList(); + + for (ProfileProto.Sample sample : profile.getSampleList()) { + final String seg = PprofSegmentParser.extractSegmentIdFromLabels(sample.getLabelList(), stringTable); + if (seg == null || !seg.equals(segmentId)) { + continue; + } + long sampleCount = sample.getValueCount() > 0 ? sample.getValue(0) : 1L; + long weightMs = sampleCount * periodMs; + + // Build function stack then ensure root->leaf order for aggregation + List<String> stack = PprofSegmentParser.extractStackFromSample(sample, profile); + Collections.reverse(stack); + + // Aggregate along path (similar to FrameTreeBuilder.mergeSample) + int parentId = -1; // root + for (String fn : stack) { + String key = parentId + "|" + fn; + Integer nodeId = key2Id.get(key); + + if (nodeId == null) { + ProfileStackElement element = new ProfileStackElement(); + element.setId(elements.size()); + element.setParentId(parentId); + element.setCodeSignature(fn); + element.setDuration(0); + element.setDurationChildExcluded(0); + element.setCount(0); + elements.add(element); + nodeId = element.getId(); + key2Id.put(key, nodeId); + } + + ProfileStackElement element = elements.get(nodeId); + element.setDuration(element.getDuration() + (int) weightMs); + element.setCount(element.getCount() + (int) sampleCount); + + parentId = nodeId; + } + } + + int rootCount = 0; + for (ProfileStackElement e : elements) { + if (e.getParentId() == -1) { + rootCount++; + } + } + if (rootCount > 1) { + int virtualRootId = elements.size(); + ProfileStackElement virtualRoot = new ProfileStackElement(); + virtualRoot.setId(virtualRootId); + virtualRoot.setParentId(-1); + virtualRoot.setCodeSignature("root"); + virtualRoot.setDuration(0); + virtualRoot.setDurationChildExcluded(0); + virtualRoot.setCount(0); + elements.add(virtualRoot); + + for (ProfileStackElement e : elements) { + if (e.getId() == virtualRootId) { + continue; + } + if (e.getParentId() == -1) { + e.setParentId(virtualRootId); + virtualRoot.setDuration(virtualRoot.getDuration() + e.getDuration()); + virtualRoot.setCount(virtualRoot.getCount() + e.getCount()); + } + } + } + + // Calculate self = total - sum(immediate children) in O(n) + Map<Integer, Integer> childDurSum = new HashMap<>(); + for (ProfileStackElement child : elements) { + int pid = child.getParentId(); + if (pid != -1) { + childDurSum.put(pid, childDurSum.getOrDefault(pid, 0) + child.getDuration()); + } + } + for (ProfileStackElement elem : elements) { + int childrenSum = childDurSum.getOrDefault(elem.getId(), 0); + elem.setDurationChildExcluded(Math.max(0, elem.getDuration() - childrenSum)); + } + + // Reorder and reindex elements: ensure root first (id=0), parent before child + Integer rootId = null; + for (ProfileStackElement e : elements) { + if (e.getParentId() == -1) { + rootId = e.getId(); + break; + } + } + if (rootId != null) { + Map<Integer, List<ProfileStackElement>> childrenMap = new HashMap<>(); + for (ProfileStackElement e : elements) { + childrenMap.computeIfAbsent(e.getParentId(), k -> new ArrayList<>()).add(e); + } + + List<ProfileStackElement> ordered = new ArrayList<>(); + ArrayDeque<ProfileStackElement> queue = new ArrayDeque<>(); + // start from root + for (ProfileStackElement e : elements) { + if (e.getId() == rootId) { + queue.add(e); + break; + } + } + while (!queue.isEmpty()) { + ProfileStackElement cur = queue.removeFirst(); + ordered.add(cur); + List<ProfileStackElement> children = childrenMap.get(cur.getId()); + if (children != null) { + // sort children by duration desc to make primary path first + children.sort((a, b) -> Integer.compare(b.getDuration(), a.getDuration())); + queue.addAll(children); + } + } + + // reassign ids to ensure contiguous and root=0, fix parentId references + Map<Integer, Integer> idRemap = new HashMap<>(); + for (int i = 0; i < ordered.size(); i++) { + idRemap.put(ordered.get(i).getId(), i); + } + for (ProfileStackElement e : ordered) { + int newId = idRemap.get(e.getId()); + int oldId = e.getId(); Review Comment: Variable 'int oldId' is never read. ```suggestion ``` ########## oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/profiling/trace/analyze/GoProfileAnalyzer.java: ########## @@ -0,0 +1,251 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You 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 org.apache.skywalking.oap.server.core.profiling.trace.analyze; + +import com.google.perftools.profiles.ProfileProto; +import java.util.Collections; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.stream.Collectors; +import java.util.ArrayDeque; +import org.apache.skywalking.oap.server.core.profiling.trace.ProfileThreadSnapshotRecord; +import org.apache.skywalking.oap.server.core.query.input.SegmentProfileAnalyzeQuery; +import org.apache.skywalking.oap.server.core.query.type.ProfileAnalyzation; +import org.apache.skywalking.oap.server.core.query.type.ProfileStackElement; +import org.apache.skywalking.oap.server.core.query.type.ProfileStackTree; +import org.apache.skywalking.oap.server.library.pprof.parser.PprofSegmentParser; +import org.apache.skywalking.oap.server.library.pprof.parser.PprofParser; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * Analyzer for Go pprof samples. Builds a stack tree with total/self durations using sampling period. + * This works independently from ThreadSnapshot, for Go profiles only. + */ +public class GoProfileAnalyzer { + private static final Logger LOGGER = LoggerFactory.getLogger(GoProfileAnalyzer.class); + + /** + * Analyze a pprof profile for a specific segment and time window. + */ + public ProfileAnalyzation analyze(final String segmentId, + final long startTimeInclusive, + final long endTimeInclusive, Review Comment: The parameter 'endTimeInclusive' is never used. ```suggestion ``` ########## oap-server/server-receiver-plugin/skywalking-profile-receiver-plugin/src/main/java/org/apache/skywalking/oap/server/receiver/profile/provider/handler/ProfileTaskServiceHandler.java: ########## @@ -130,6 +139,92 @@ public void onCompleted() { }; } + @Override + public StreamObserver<GoProfileData> goProfileReport(StreamObserver<Commands> responseObserver) { + return new StreamObserver<GoProfileData>() { + private ByteArrayOutputStream profileDataBuffer = new ByteArrayOutputStream(); + private String currentTaskId = null; + + @Override + public void onNext(GoProfileData profileData) { + LOGGER.debug("receive go profile data: taskId='{}', payloadSize={}, isLast={}", + profileData.getTaskId(), + profileData.getPayload().size(), + profileData.getIsLast()); + + // Check if taskId is empty - this indicates a problem with Go agent + if (profileData.getTaskId() == null || profileData.getTaskId().isEmpty()) { + LOGGER.error("Go agent sent empty taskId! This indicates a problem with Go agent's profile task management. " + + "Please check Go agent's profile task creation and task.TaskID assignment."); + return; + } + + // Reset state if this is a new task + if (currentTaskId == null || !currentTaskId.equals(profileData.getTaskId())) { + currentTaskId = profileData.getTaskId(); + profileDataBuffer.reset(); + LOGGER.debug("Starting new task: {}", currentTaskId); + } + + // Collect profile data + try { + profileDataBuffer.write(profileData.getPayload().toByteArray()); + } catch (IOException e) { + LOGGER.error("Failed to write Go profile data", e); + return; + } + + // If this is the last data chunk, parse and store + if (profileData.getIsLast()) { + try { + // Parse Go profile data via library-pprof-parser (auto-detect gzip) + ProfileProto.Profile profile = PprofParser.parseProfile(profileDataBuffer.toByteArray()); + List<SegmentInfo> segments = PprofSegmentParser.parseSegments(profile); + + // Log parsed segments briefly for troubleshooting + if (CollectionUtils.isEmpty(segments)) { + LOGGER.debug("Parsed Go profile has no segments. taskId={}, hint=check labels segment_id/trace_id", currentTaskId); + } + + // Store ProfileThreadSnapshotRecord for each segment + for (SegmentInfo segmentInfo : segments) { + storeGoProfileSegment(segmentInfo, currentTaskId, profile); + } + + // Analyzer preview removed to reduce log noise after verification + + LOGGER.info("Processed Go profile data: taskId={}, segments={}", currentTaskId, segments.size()); + + } catch (Exception e) { + LOGGER.error("Failed to parse Go profile data for task: " + currentTaskId, e); + } finally { + // Reset state Review Comment: Line 201 has incorrect indentation - mixed spaces/tabs. Should align with surrounding code indentation. ```suggestion // Reset state ``` -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
