Copilot commented on code in PR #13514:
URL: https://github.com/apache/skywalking/pull/13514#discussion_r2374076058


##########
oap-server/analyzer/agent-analyzer/src/main/java/org/apache/skywalking/oap/server/analyzer/provider/trace/parser/listener/vservice/VirtualDatabaseProcessor.java:
##########
@@ -65,18 +66,36 @@ public void prepareVSIfNecessary(SpanObject span, 
SegmentObject segmentObject) {
         recordList.add(toDatabaseAccess(span, serviceName, timeBucket, 
latency));
 
         readStatementIfSlow(span.getTagsList(), latency).ifPresent(statement 
-> {
-            DatabaseSlowStatement dbSlowStat = new DatabaseSlowStatement();
-            dbSlowStat.setId(segmentObject.getTraceSegmentId() + "-" + 
span.getSpanId());
-            dbSlowStat.setTraceId(segmentObject.getTraceId());
-            
dbSlowStat.setDatabaseServiceId(IDManager.ServiceID.buildId(serviceName, 
false));
-            dbSlowStat.setStatement(statement);
-            dbSlowStat.setLatency(latency);
-            
dbSlowStat.setTimeBucket(TimeBucket.getRecordTimeBucket(span.getStartTime()));
-            dbSlowStat.setTimestamp(span.getStartTime());
-            recordList.add(dbSlowStat);
+            recordList.add(buildDatabaseSlowStatement(span, segmentObject, 
statement, serviceName, latency));
+            recordList.add(buildServiceDatabaseSlowStatement(span, 
segmentObject, statement, serviceName, latency));

Review Comment:
   The recordList.add() calls are duplicated from the refactored code. The 
original DatabaseSlowStatement creation should be removed from the lambda to 
avoid creating duplicate records.
   ```suggestion
               // recordList.add(buildDatabaseSlowStatement(span, 
segmentObject, statement, serviceName, latency));
               // recordList.add(buildServiceDatabaseSlowStatement(span, 
segmentObject, statement, serviceName, latency));
   ```



##########
oap-server/analyzer/agent-analyzer/src/main/java/org/apache/skywalking/oap/server/analyzer/provider/trace/parser/listener/vservice/VirtualDatabaseProcessor.java:
##########
@@ -65,18 +66,36 @@ public void prepareVSIfNecessary(SpanObject span, 
SegmentObject segmentObject) {
         recordList.add(toDatabaseAccess(span, serviceName, timeBucket, 
latency));
 
         readStatementIfSlow(span.getTagsList(), latency).ifPresent(statement 
-> {
-            DatabaseSlowStatement dbSlowStat = new DatabaseSlowStatement();
-            dbSlowStat.setId(segmentObject.getTraceSegmentId() + "-" + 
span.getSpanId());
-            dbSlowStat.setTraceId(segmentObject.getTraceId());
-            
dbSlowStat.setDatabaseServiceId(IDManager.ServiceID.buildId(serviceName, 
false));
-            dbSlowStat.setStatement(statement);
-            dbSlowStat.setLatency(latency);
-            
dbSlowStat.setTimeBucket(TimeBucket.getRecordTimeBucket(span.getStartTime()));
-            dbSlowStat.setTimestamp(span.getStartTime());
-            recordList.add(dbSlowStat);
+            recordList.add(buildDatabaseSlowStatement(span, segmentObject, 
statement, serviceName, latency));
+            recordList.add(buildServiceDatabaseSlowStatement(span, 
segmentObject, statement, serviceName, latency));
         });
     }
 
+    private DatabaseSlowStatement buildDatabaseSlowStatement(SpanObject span, 
SegmentObject segmentObject, String statement, String serviceName, int latency) 
{
+        DatabaseSlowStatement dbSlowStat = new DatabaseSlowStatement();
+        dbSlowStat.setId(segmentObject.getTraceSegmentId() + "-" + 
span.getSpanId());
+        dbSlowStat.setTraceId(segmentObject.getTraceId());
+        
dbSlowStat.setDatabaseServiceId(IDManager.ServiceID.buildId(serviceName, 
false));
+        dbSlowStat.setStatement(statement);
+        dbSlowStat.setLatency(latency);
+        
dbSlowStat.setTimeBucket(TimeBucket.getRecordTimeBucket(span.getStartTime()));
+        dbSlowStat.setTimestamp(span.getStartTime());
+        return dbSlowStat;
+    }
+
+    private Source buildServiceDatabaseSlowStatement(SpanObject span, 
SegmentObject segmentObject, String statement, String serviceName, int latency) 
{

Review Comment:
   The return type should be ServiceDatabaseSlowStatement instead of Source for 
better type safety and clarity, matching the pattern used in 
buildDatabaseSlowStatement.
   ```suggestion
       private ServiceDatabaseSlowStatement 
buildServiceDatabaseSlowStatement(SpanObject span, SegmentObject segmentObject, 
String statement, String serviceName, int latency) {
   ```



-- 
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]

Reply via email to