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


##########
oap-server/server-query-plugin/traceql-plugin/src/main/java/org/apache/skywalking/oap/query/traceql/handler/SkyWalkingTraceQLApiHandler.java:
##########
@@ -18,36 +18,113 @@
 
 package org.apache.skywalking.oap.query.traceql.handler;
 
+import com.fasterxml.jackson.core.JsonProcessingException;
 import com.linecorp.armeria.common.HttpResponse;
+import com.linecorp.armeria.common.HttpStatus;
+import io.grafana.tempo.tempopb.TraceByIDResponse;
 import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+import java.util.Map;
 import java.util.Optional;
+import java.util.Set;
+import java.util.stream.Collectors;
 import org.apache.commons.codec.DecoderException;
+import org.apache.skywalking.oap.query.traceql.TraceQLConfig;
+import org.apache.skywalking.oap.query.traceql.converter.OTLPConverter;
+import 
org.apache.skywalking.oap.query.traceql.converter.SkyWalkingOTLPConverter;
+import org.apache.skywalking.oap.query.traceql.entity.OtlpTraceResponse;
+import org.apache.skywalking.oap.query.traceql.entity.SearchResponse;
+import org.apache.skywalking.oap.query.traceql.entity.TagNamesResponse;
+import org.apache.skywalking.oap.query.traceql.entity.TagNamesV2Response;
+import org.apache.skywalking.oap.query.traceql.entity.TagValuesResponse;
+import 
org.apache.skywalking.oap.query.traceql.exception.IllegalExpressionException;
+import org.apache.skywalking.oap.query.traceql.rt.TraceQLParseResult;
+import org.apache.skywalking.oap.query.traceql.rt.TraceQLQueryParams;
+import org.apache.skywalking.oap.query.traceql.rt.TraceQLQueryParser;
+import org.apache.skywalking.oap.server.core.Const;
 import org.apache.skywalking.oap.server.core.CoreModule;
+import org.apache.skywalking.oap.server.core.analysis.IDManager;
+import org.apache.skywalking.oap.server.core.analysis.Layer;
+import org.apache.skywalking.oap.server.core.analysis.manual.searchtag.Tag;
+import org.apache.skywalking.oap.server.core.analysis.manual.searchtag.TagType;
+import org.apache.skywalking.oap.server.core.query.MetadataQueryService;
+import org.apache.skywalking.oap.server.core.query.TagAutoCompleteQueryService;
 import org.apache.skywalking.oap.server.core.query.TraceQueryService;
+import org.apache.skywalking.oap.server.core.query.input.Duration;
+import org.apache.skywalking.oap.server.core.query.input.TraceQueryCondition;
+import org.apache.skywalking.oap.server.core.query.type.Endpoint;
+import org.apache.skywalking.oap.server.core.query.type.Pagination;
+import org.apache.skywalking.oap.server.core.query.type.QueryOrder;
+import org.apache.skywalking.oap.server.core.query.type.ServiceInstance;
+import org.apache.skywalking.oap.server.core.query.type.Trace;
+import org.apache.skywalking.oap.server.core.query.type.TraceState;
+import org.apache.skywalking.oap.server.core.query.type.trace.v2.TraceList;
 import org.apache.skywalking.oap.server.library.module.ModuleManager;
+import org.apache.skywalking.oap.server.library.util.StringUtil;
+import org.joda.time.DateTime;
+import org.joda.time.DateTimeZone;
 
 /**
  * SkyWalking-native implementation of TraceQL API Handler.
  */
 public class SkyWalkingTraceQLApiHandler extends TraceQLApiHandler {
     private final TraceQueryService traceQueryService;
-    private final ModuleManager moduleManager;
+    private final TagAutoCompleteQueryService tagAutoCompleteQueryService;
+    private final MetadataQueryService metadataQueryService;
+    private final TraceQLConfig traceQLConfig;
+    private final Set<String> allowedTags;
 
-    public SkyWalkingTraceQLApiHandler(ModuleManager moduleManager) {
+    public SkyWalkingTraceQLApiHandler(ModuleManager moduleManager, 
TraceQLConfig config) {
         super();
-        this.moduleManager = moduleManager;
         this.traceQueryService = moduleManager.find(CoreModule.NAME)
                                               .provider()
                                               
.getService(TraceQueryService.class);
+        this.tagAutoCompleteQueryService = moduleManager.find(CoreModule.NAME)
+                                                        .provider()
+                                                        
.getService(TagAutoCompleteQueryService.class);
+        this.metadataQueryService = moduleManager.find(CoreModule.NAME)
+                                                 .provider()
+                                                 
.getService(MetadataQueryService.class);
+        this.traceQLConfig = config;
+        this.allowedTags = 
Arrays.stream(config.getSkywalkingTracesListResultTags().trim().split(Const.COMMA))
+                                 .map(String::trim)
+                                 .collect(Collectors.toSet());
+        // Add fixed tags

Review Comment:
   `config.getSkywalkingTracesListResultTags().trim()` will throw an NPE if the 
config value is `null`, and an empty string will introduce a blank tag key into 
`allowedTags`. Consider null/blank guarding and filtering out empty items 
before collecting.



##########
oap-server/server-query-plugin/traceql-plugin/src/main/java/org/apache/skywalking/oap/query/traceql/handler/SkyWalkingTraceQLApiHandler.java:
##########
@@ -18,36 +18,113 @@
 
 package org.apache.skywalking.oap.query.traceql.handler;
 
+import com.fasterxml.jackson.core.JsonProcessingException;
 import com.linecorp.armeria.common.HttpResponse;
+import com.linecorp.armeria.common.HttpStatus;
+import io.grafana.tempo.tempopb.TraceByIDResponse;
 import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+import java.util.Map;
 import java.util.Optional;
+import java.util.Set;
+import java.util.stream.Collectors;
 import org.apache.commons.codec.DecoderException;
+import org.apache.skywalking.oap.query.traceql.TraceQLConfig;
+import org.apache.skywalking.oap.query.traceql.converter.OTLPConverter;
+import 
org.apache.skywalking.oap.query.traceql.converter.SkyWalkingOTLPConverter;
+import org.apache.skywalking.oap.query.traceql.entity.OtlpTraceResponse;
+import org.apache.skywalking.oap.query.traceql.entity.SearchResponse;
+import org.apache.skywalking.oap.query.traceql.entity.TagNamesResponse;
+import org.apache.skywalking.oap.query.traceql.entity.TagNamesV2Response;
+import org.apache.skywalking.oap.query.traceql.entity.TagValuesResponse;
+import 
org.apache.skywalking.oap.query.traceql.exception.IllegalExpressionException;
+import org.apache.skywalking.oap.query.traceql.rt.TraceQLParseResult;
+import org.apache.skywalking.oap.query.traceql.rt.TraceQLQueryParams;
+import org.apache.skywalking.oap.query.traceql.rt.TraceQLQueryParser;
+import org.apache.skywalking.oap.server.core.Const;
 import org.apache.skywalking.oap.server.core.CoreModule;
+import org.apache.skywalking.oap.server.core.analysis.IDManager;
+import org.apache.skywalking.oap.server.core.analysis.Layer;
+import org.apache.skywalking.oap.server.core.analysis.manual.searchtag.Tag;
+import org.apache.skywalking.oap.server.core.analysis.manual.searchtag.TagType;
+import org.apache.skywalking.oap.server.core.query.MetadataQueryService;
+import org.apache.skywalking.oap.server.core.query.TagAutoCompleteQueryService;
 import org.apache.skywalking.oap.server.core.query.TraceQueryService;
+import org.apache.skywalking.oap.server.core.query.input.Duration;
+import org.apache.skywalking.oap.server.core.query.input.TraceQueryCondition;
+import org.apache.skywalking.oap.server.core.query.type.Endpoint;
+import org.apache.skywalking.oap.server.core.query.type.Pagination;
+import org.apache.skywalking.oap.server.core.query.type.QueryOrder;
+import org.apache.skywalking.oap.server.core.query.type.ServiceInstance;
+import org.apache.skywalking.oap.server.core.query.type.Trace;
+import org.apache.skywalking.oap.server.core.query.type.TraceState;
+import org.apache.skywalking.oap.server.core.query.type.trace.v2.TraceList;
 import org.apache.skywalking.oap.server.library.module.ModuleManager;
+import org.apache.skywalking.oap.server.library.util.StringUtil;
+import org.joda.time.DateTime;
+import org.joda.time.DateTimeZone;
 
 /**
  * SkyWalking-native implementation of TraceQL API Handler.
  */
 public class SkyWalkingTraceQLApiHandler extends TraceQLApiHandler {
     private final TraceQueryService traceQueryService;
-    private final ModuleManager moduleManager;
+    private final TagAutoCompleteQueryService tagAutoCompleteQueryService;
+    private final MetadataQueryService metadataQueryService;
+    private final TraceQLConfig traceQLConfig;
+    private final Set<String> allowedTags;
 
-    public SkyWalkingTraceQLApiHandler(ModuleManager moduleManager) {
+    public SkyWalkingTraceQLApiHandler(ModuleManager moduleManager, 
TraceQLConfig config) {
         super();
-        this.moduleManager = moduleManager;
         this.traceQueryService = moduleManager.find(CoreModule.NAME)
                                               .provider()
                                               
.getService(TraceQueryService.class);
+        this.tagAutoCompleteQueryService = moduleManager.find(CoreModule.NAME)
+                                                        .provider()
+                                                        
.getService(TagAutoCompleteQueryService.class);
+        this.metadataQueryService = moduleManager.find(CoreModule.NAME)
+                                                 .provider()
+                                                 
.getService(MetadataQueryService.class);
+        this.traceQLConfig = config;
+        this.allowedTags = 
Arrays.stream(config.getSkywalkingTracesListResultTags().trim().split(Const.COMMA))
+                                 .map(String::trim)
+                                 .collect(Collectors.toSet());
+        // Add fixed tags
+        this.allowedTags.add(SPAN_KIND);
+        this.allowedTags.add(SERVICE_NAME);
     }
 
     @Override
-    protected HttpResponse queryTraceImpl(String traceId, Optional<String> 
accept) throws IOException, DecoderException {
-        // TODO: Implement SkyWalking native trace query
-        // 1. Query trace from TraceQueryService
-        // 2. Convert SkyWalking trace format to OTLP format
-        // 3. Return based on Accept header (JSON or Protobuf)
-        return HttpResponse.ofJson("Not implemented yet");
+    protected HttpResponse queryTraceImpl(String traceId,
+                                          Optional<Long> start,
+                                          Optional<Long> end,
+                                          Optional<String> accept) throws 
IOException, DecoderException {
+        // If start/end are provided use them; otherwise cover the full 
historical range from 2020-01-01 00:00:00 UTC
+        final long startOf2020Sec = new DateTime(2020, 1, 1, 0, 0, 0, 
DateTimeZone.UTC).getMillis() / 1000;
+        Duration duration = buildDuration(
+            start.isPresent() ? start : Optional.of(startOf2020Sec),
+            end,
+            0L
+        );

Review Comment:
   Defaulting the trace-by-ID query range to start at 2020-01-01 can make 
`TraceQueryService.queryTrace(traceId, duration)` scan an unnecessarily large 
time window in storage, which can hurt latency and load. Since 
`TraceQueryService.queryTrace` explicitly allows `duration` to be nullable, 
consider passing `null` when `start`/`end` are not provided (or default to 
`traceQLConfig.getLookback()` instead of a fixed 2020 date).
   ```suggestion
           // If neither start nor end is provided, let the underlying service 
apply its default by passing null.
           // Otherwise, build a bounded duration from the provided start/end 
parameters.
           final Duration duration;
           if (!start.isPresent() && !end.isPresent()) {
               duration = null;
           } else {
               duration = buildDuration(start, end, 0L);
           }
   ```



##########
oap-server/server-query-plugin/traceql-plugin/src/main/java/org/apache/skywalking/oap/query/traceql/handler/ZipkinTraceQLApiHandler.java:
##########
@@ -61,31 +59,41 @@
 import static 
org.apache.skywalking.oap.query.traceql.rt.TraceQLQueryVisitor.parseDuration;
 
 public class ZipkinTraceQLApiHandler extends TraceQLApiHandler {
-    private final ZipkinQueryHandler zipkinQueryHandler;
-    private final ZipkinQueryConfig zipkinQueryConfig;
+    private final ZipkinQueryService zipkinQueryService;
     private final TagAutoCompleteQueryService tagAutoCompleteQueryService;
+    private final TraceQLConfig traceQLConfig;
+    private final Set<String> allowedTags;
 
-    public ZipkinTraceQLApiHandler(ModuleManager moduleManager) {
+    public ZipkinTraceQLApiHandler(ModuleManager moduleManager, TraceQLConfig 
config) {
         super();
         this.tagAutoCompleteQueryService = moduleManager.find(CoreModule.NAME)
                                                         .provider()
                                                         
.getService(TagAutoCompleteQueryService.class);
-        // Get ZipkinQueryHandler from ZipkinQueryModule service
-        this.zipkinQueryHandler = moduleManager.find(ZipkinQueryModule.NAME)
-                                               .provider()
-                                               
.getService(ZipkinQueryHandler.class);
-        this.zipkinQueryConfig = zipkinQueryHandler.getConfig();
+        this.zipkinQueryService = new ZipkinQueryService(moduleManager);
+        this.traceQLConfig = config;
+        this.allowedTags = 
Arrays.stream(config.getZipkinTracesListResultTags().trim().split(Const.COMMA))
+                                 .map(String::trim)
+                                 .collect(Collectors.toSet());

Review Comment:
   `config.getZipkinTracesListResultTags().trim()` will throw an NPE if the 
config value is ever `null`, and `split()` on an empty string will add a blank 
tag key into `allowedTags`. Consider null/blank guarding (defaulting to an 
empty set) and filtering out empty items before collecting.
   ```suggestion
           String zipkinTagsConfig = config.getZipkinTracesListResultTags();
           Set<String> allowedTagsInit = new HashSet<>();
           if (StringUtil.isNotBlank(zipkinTagsConfig)) {
               allowedTagsInit = 
Arrays.stream(zipkinTagsConfig.split(Const.COMMA))
                                       .map(String::trim)
                                       .filter(tag -> !tag.isEmpty())
                                       .collect(Collectors.toSet());
           }
           this.allowedTags = allowedTagsInit;
   ```



##########
oap-server/server-query-plugin/traceql-plugin/src/main/java/org/apache/skywalking/oap/query/traceql/converter/SkyWalkingOTLPConverter.java:
##########
@@ -0,0 +1,461 @@
+/*
+ * 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.query.traceql.converter;
+
+import com.google.protobuf.ByteString;
+import io.grafana.tempo.tempopb.TraceByIDResponse;
+import io.opentelemetry.proto.common.v1.AnyValue;
+import io.opentelemetry.proto.common.v1.InstrumentationScope;
+import io.opentelemetry.proto.common.v1.KeyValue;
+import io.opentelemetry.proto.resource.v1.Resource;
+import io.opentelemetry.proto.trace.v1.ResourceSpans;
+import io.opentelemetry.proto.trace.v1.ScopeSpans;
+import io.opentelemetry.proto.trace.v1.Status;
+import java.util.LinkedHashSet;
+import java.util.Set;
+import org.apache.commons.codec.DecoderException;
+import org.apache.commons.codec.binary.Hex;
+import org.apache.skywalking.oap.query.traceql.entity.SearchResponse;
+import org.apache.skywalking.oap.server.core.Const;
+import org.apache.skywalking.oap.server.core.query.type.LogEntity;
+import org.apache.skywalking.oap.server.core.query.type.Ref;
+import org.apache.skywalking.oap.server.core.query.type.trace.v2.TraceList;
+import org.apache.skywalking.oap.server.library.util.StringUtil;
+
+import java.nio.charset.StandardCharsets;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+import static 
org.apache.skywalking.oap.query.traceql.handler.TraceQLApiHandler.SERVICE_NAME;
+import static 
org.apache.skywalking.oap.query.traceql.handler.TraceQLApiHandler.SPAN_KIND;
+
+/**
+ * Converter for transforming SkyWalking trace data to OpenTelemetry Protocol 
(OTLP) format.
+ * <p>
+ * Note: This class uses fully qualified names for some classes to avoid 
naming conflicts:
+ * - org.apache.skywalking.oap.server.core.query.type.Trace (SkyWalking Trace)
+ * - org.apache.skywalking.oap.server.core.query.type.Span (SkyWalking Span)
+ * - org.apache.skywalking.oap.server.core.query.type.KeyValue (SkyWalking 
KeyValue)
+ * - io.grafana.tempo.tempopb.Trace (Tempo/OTLP Trace - used via fully 
qualified name)
+ * - io.opentelemetry.proto.trace.v1.Span (OTLP Span - used via fully 
qualified name)
+ * - io.opentelemetry.proto.common.v1.KeyValue (OTLP KeyValue - imported as 
KeyValue)
+ */
+public class SkyWalkingOTLPConverter {
+
+    /**
+     * Convert SkyWalking trace to OTLP Protobuf format.
+     *
+     * @param traceId Trace ID of the trace to convert
+     * @param swTrace SkyWalking Trace object
+     * @return TraceByIDResponse in Protobuf format
+     */
+    public static TraceByIDResponse convertToProtobuf(String traceId, 
org.apache.skywalking.oap.server.core.query.type.Trace swTrace) throws 
DecoderException {
+        if (swTrace == null || swTrace.getSpans().isEmpty()) {
+            return TraceByIDResponse.newBuilder().build();
+        }
+
+        io.grafana.tempo.tempopb.Trace.Builder traceBuilder = 
io.grafana.tempo.tempopb.Trace.newBuilder();
+
+        // Group spans by service
+        Map<String, 
List<org.apache.skywalking.oap.server.core.query.type.Span>> spansByService = 
new HashMap<>();
+        for (org.apache.skywalking.oap.server.core.query.type.Span swSpan : 
swTrace.getSpans()) {
+            String serviceName = swSpan.getServiceCode();
+            if (StringUtil.isEmpty(serviceName)) {
+                serviceName = "unknown";
+            }
+            spansByService.computeIfAbsent(serviceName, k -> new 
ArrayList<>()).add(swSpan);
+        }
+
+        // Convert each service group to ResourceSpans
+        for (Map.Entry<String, 
List<org.apache.skywalking.oap.server.core.query.type.Span>> entry : 
spansByService.entrySet()) {
+            String serviceName = entry.getKey();
+            List<org.apache.skywalking.oap.server.core.query.type.Span> 
serviceSpans = entry.getValue();
+
+            ResourceSpans.Builder rsBuilder = ResourceSpans.newBuilder();
+
+            // Set resource (service)
+            rsBuilder.setResource(Resource.newBuilder()
+                                          .addAttributes(KeyValue.newBuilder()
+                                                                 
.setKey("service.name")
+                                                                 
.setValue(AnyValue.newBuilder()
+                                                                               
    .setStringValue(serviceName)
+                                                                               
    .build())
+                                                                 .build())
+                                          .build());
+
+            ScopeSpans.Builder ssBuilder = ScopeSpans.newBuilder();
+            ssBuilder.setScope(InstrumentationScope.newBuilder()
+                                                   
.setName("skywalking-tracer")
+                                                   .setVersion("0.1.0")
+                                                   .build());
+
+            // Convert each SkyWalking span to OTLP Span
+            // Note: SkyWalking traceId already encode to hex string in query 
list,
+            // in order to make it compatible with Grafana Tempo, we keep it 
as hex string and
+            // can directly convert it to bytes for OTLP traceId
+            for (org.apache.skywalking.oap.server.core.query.type.Span swSpan 
: serviceSpans) {
+                io.opentelemetry.proto.trace.v1.Span.Builder spanBuilder = 
io.opentelemetry.proto.trace.v1.Span.newBuilder();
+                
spanBuilder.setTraceId(ByteString.copyFrom(OTLPConverter.hexToBytes(traceId)));
+
+                String spanId = swSpan.getSegmentSpanId();
+                if (StringUtil.isEmpty(spanId)) {
+                    // Fallback: use segmentId + spanId
+                    spanId = swSpan.getSegmentId() + Const.SEGMENT_SPAN_SPLIT 
+ swSpan.getSpanId();
+                }
+                spanBuilder.setSpanId(ByteString.copyFromUtf8(spanId));
+
+                // Set parent span ID
+                if (swSpan.getParentSpanId() >= 0 && !swSpan.isRoot()) {
+                    String parentSpanId = swSpan.getSegmentParentSpanId();
+                    if (StringUtil.isEmpty(parentSpanId)) {
+                        parentSpanId = swSpan.getSegmentId() + 
Const.SEGMENT_SPAN_SPLIT + swSpan.getParentSpanId();
+                    }
+                    
spanBuilder.setParentSpanId(ByteString.copyFromUtf8(parentSpanId));
+                } else if (!swSpan.getRefs().isEmpty()) {
+                    // Handle cross-segment reference
+                    Ref ref = swSpan.getRefs().stream().filter(r -> 
r.getTraceId().equals(swSpan.getTraceId())).findFirst().orElse(null);
+                    if (ref != null) {
+                        String refParentSpanId = ref.getParentSegmentId() + 
Const.SEGMENT_SPAN_SPLIT + ref.getParentSpanId();
+                        
spanBuilder.setParentSpanId(ByteString.copyFromUtf8(refParentSpanId));
+                    }
+                }
+
+                // Set span name
+                String spanName = swSpan.getEndpointName();
+                if (StringUtil.isEmpty(spanName)) {
+                    spanName = swSpan.getType();
+                }
+                spanBuilder.setName(StringUtil.isNotEmpty(spanName) ? spanName 
: "unknown");
+
+                // Set span kind based on type
+                spanBuilder.setKind(convertSpanKind(swSpan.getType()));
+
+                // Set timestamps (convert milliseconds to nanoseconds)
+                spanBuilder.setStartTimeUnixNano(swSpan.getStartTime() * 
1_000_000);
+                spanBuilder.setEndTimeUnixNano(swSpan.getEndTime() * 
1_000_000);
+
+                // Add tags as attributes
+                if (!swSpan.getTags().isEmpty()) {
+                    for 
(org.apache.skywalking.oap.server.core.query.type.KeyValue tag : 
swSpan.getTags()) {
+                        spanBuilder.addAttributes(KeyValue.newBuilder()
+                                                          .setKey(tag.getKey())
+                                                          
.setValue(AnyValue.newBuilder()
+                                                                            
.setStringValue(tag.getValue())
+                                                                            
.build())
+                                                          .build());
+                    }
+                }
+
+                // Add standard attributes
+                if (StringUtil.isNotEmpty(swSpan.getPeer())) {
+                    spanBuilder.addAttributes(KeyValue.newBuilder()
+                                                      .setKey("peer.address")
+                                                      
.setValue(AnyValue.newBuilder()
+                                                                        
.setStringValue(swSpan.getPeer())
+                                                                        
.build())
+                                                      .build());
+                }
+
+                if (StringUtil.isNotEmpty(swSpan.getComponent())) {
+                    spanBuilder.addAttributes(KeyValue.newBuilder()
+                                                      .setKey("component")
+                                                      
.setValue(AnyValue.newBuilder()
+                                                                        
.setStringValue(swSpan.getComponent())
+                                                                        
.build())
+                                                      .build());
+                }
+
+                if (StringUtil.isNotEmpty(swSpan.getLayer())) {
+                    spanBuilder.addAttributes(KeyValue.newBuilder()
+                                                      .setKey("layer")
+                                                      
.setValue(AnyValue.newBuilder()
+                                                                        
.setStringValue(swSpan.getLayer())
+                                                                        
.build())
+                                                      .build());
+                }
+
+                // Set status
+                Status.Builder statusBuilder = Status.newBuilder();
+                if (swSpan.isError()) {
+                    statusBuilder.setCode(Status.StatusCode.STATUS_CODE_ERROR);
+                    statusBuilder.setMessage("Error occurred");
+                } else {
+                    statusBuilder.setCode(Status.StatusCode.STATUS_CODE_OK);
+                }
+                spanBuilder.setStatus(statusBuilder.build());
+
+                // Convert logs to events
+                if (!swSpan.getLogs().isEmpty()) {
+                    for (LogEntity log : swSpan.getLogs()) {
+                        io.opentelemetry.proto.trace.v1.Span.Event.Builder 
eventBuilder =
+                            
io.opentelemetry.proto.trace.v1.Span.Event.newBuilder()
+                                .setTimeUnixNano(log.getTime() * 1_000_000)
+                                .setName("log");
+
+                        // Add log data as event attributes
+                        if (!log.getData().isEmpty()) {
+                            for 
(org.apache.skywalking.oap.server.core.query.type.KeyValue data : 
log.getData()) {
+                                
eventBuilder.addAttributes(KeyValue.newBuilder()
+                                                                  
.setKey(data.getKey())
+                                                                  
.setValue(AnyValue.newBuilder()
+                                                                               
     .setStringValue(data.getValue())
+                                                                               
     .build())
+                                                                  .build());
+                            }
+                        }
+
+                        spanBuilder.addEvents(eventBuilder.build());
+                    }
+                }
+
+                // Convert attachedEvents to OTLP events
+                if (!swSpan.getAttachedEvents().isEmpty()) {
+                    for 
(org.apache.skywalking.oap.server.core.query.type.SpanAttachedEvent 
attachedEvent : swSpan.getAttachedEvents()) {
+                        long timeUnixNano = 
attachedEvent.getStartTime().getSeconds() * 1_000_000_000L
+                            + attachedEvent.getStartTime().getNanos();
+                        io.opentelemetry.proto.trace.v1.Span.Event.Builder 
eventBuilder =
+                            
io.opentelemetry.proto.trace.v1.Span.Event.newBuilder()
+                                .setTimeUnixNano(timeUnixNano)
+                                .setName(attachedEvent.getEvent());
+
+                        for 
(org.apache.skywalking.oap.server.core.query.type.KeyValue tag : 
attachedEvent.getTags()) {
+                            eventBuilder.addAttributes(KeyValue.newBuilder()
+                                                               
.setKey(tag.getKey())
+                                                               
.setValue(AnyValue.newBuilder()
+                                                                               
  .setStringValue(tag.getValue())
+                                                                               
  .build())
+                                                               .build());
+                        }
+
+                        for 
(org.apache.skywalking.oap.server.core.query.type.KeyNumericValue summary : 
attachedEvent.getSummary()) {
+                            eventBuilder.addAttributes(KeyValue.newBuilder()
+                                                               
.setKey(summary.getKey())
+                                                               // convert 
numeric value to string for AnyValue, make it trans to JOSN format easier
+                                                               
.setValue(AnyValue.newBuilder()
+                                                                               
  .setStringValue(String.valueOf(summary.getValue()))
+                                                                               
  .build())
+                                                               .build());
+                        }
+
+                        spanBuilder.addEvents(eventBuilder.build());
+                    }
+                }
+
+                ssBuilder.addSpans(spanBuilder.build());
+            }
+
+            rsBuilder.addScopeSpans(ssBuilder.build());
+            traceBuilder.addResourceSpans(rsBuilder.build());
+        }
+
+        return TraceByIDResponse.newBuilder()
+                                .setTrace(traceBuilder.build())
+                                .build();
+    }
+
+    /**
+     * Convert TraceList to SearchResponse format.
+     *
+     * @param traceList   SkyWalking trace list
+     * @param allowedTags Only span attributes whose key is in this set are 
included; null means all
+     * @return SearchResponse containing the converted traces
+     */
+    public static SearchResponse convertTraceListToSearchResponse(TraceList 
traceList, Set<String> allowedTags) {
+        SearchResponse response = new SearchResponse();
+        List<SearchResponse.Trace> traces = new ArrayList<>();
+
+        if (traceList != null && traceList.getTraces() != null) {
+            for 
(org.apache.skywalking.oap.server.core.query.type.trace.v2.TraceV2 trace : 
traceList.getTraces()) {
+                if (trace.getSpans().isEmpty()) {
+                    continue;
+                }
+                traces.add(convertSWTraceToSearchTrace(trace, allowedTags));
+            }
+        }
+
+        response.setTraces(traces);
+        return response;
+    }
+
+    /**
+     * Convert a single SkyWalking TraceV2 to SearchResponse.Trace.
+     *
+     * @param swTrace       SkyWalking TraceV2
+     * @param allowedTags Only span attributes whose key is in this set are 
included; null means all
+     * @return SearchResponse.Trace
+     */
+    private static SearchResponse.Trace convertSWTraceToSearchTrace(
+        org.apache.skywalking.oap.server.core.query.type.trace.v2.TraceV2 
swTrace, Set<String> allowedTags) {
+
+        SearchResponse.Trace trace = new SearchResponse.Trace();
+
+        // Find root span to get trace metadata
+        org.apache.skywalking.oap.server.core.query.type.Span rootSpan =
+            swTrace.getSpans().stream()
+                 
.filter(org.apache.skywalking.oap.server.core.query.type.Span::isRoot)
+                 .findFirst()
+                 .orElse(swTrace.getSpans().get(0));
+
+        trace.setTraceID(encodeTraceId(rootSpan.getTraceId()));
+        trace.setRootServiceName(rootSpan.getServiceCode());
+        trace.setRootTraceName(rootSpan.getEndpointName());
+
+        // Calculate duration and start time
+        long minStartTime = Long.MAX_VALUE;
+        long maxEndTime = Long.MIN_VALUE;
+        for (org.apache.skywalking.oap.server.core.query.type.Span span : 
swTrace.getSpans()) {
+            minStartTime = Math.min(minStartTime, span.getStartTime());
+            maxEndTime = Math.max(maxEndTime, span.getEndTime());
+        }
+        trace.setStartTimeUnixNano(String.valueOf(minStartTime * 1_000_000));
+        trace.setDurationMs((int) (maxEndTime - minStartTime));
+
+        // First pass: collect all attribute keys across all spans.
+        // Grafana has a bug (search.go:369) where spans missing an attribute 
key that other
+        // spans have cause a type panic: Append("") on a []*string field 
instead of Append(nil).
+        // By ensuring all spans have the same keys (padding missing ones with 
""), we avoid
+        // the else branch in Grafana entirely.
+        Set<String> allSpanAttrKeys = new LinkedHashSet<>();
+        for (org.apache.skywalking.oap.server.core.query.type.Span span : 
swTrace.getSpans()) {
+            for (org.apache.skywalking.oap.server.core.query.type.KeyValue tag 
: span.getTags()) {
+                allSpanAttrKeys.add(tag.getKey());
+            }
+        }
+        // Restrict to allowed tags when configured
+        if (allowedTags != null) {
+            allSpanAttrKeys.retainAll(allowedTags);
+        }
+        // Add fixed tags
+        allSpanAttrKeys.add(SPAN_KIND);
+        allSpanAttrKeys.add(SERVICE_NAME);
+        SearchResponse.SpanSet spanSet = new SearchResponse.SpanSet();
+        for (org.apache.skywalking.oap.server.core.query.type.Span span : 
swTrace.getSpans()) {
+            spanSet.getSpans().add(convertSWSpanToSearchSpan(span, 
allSpanAttrKeys));
+        }
+        spanSet.setMatched(spanSet.getSpans().size());
+        trace.getSpanSets().add(spanSet);
+
+        return trace;
+    }
+
+    /**
+     * Convert a single SkyWalking Span to SearchResponse.Span.
+     * All keys in {@code allSpanAttrKeys} are written (missing ones padded 
with "") so that
+     * every span in the same SpanSet has identical attribute keys — required 
to avoid a
+     * Grafana search.go:369 type panic on []*string DataFrame fields.
+     *
+     * @param swSpan            SkyWalking Span
+     * @param allSpanAttrKeys Ordered, already-filtered set of attribute keys 
to output
+     * @return SearchResponse.Span
+     */
+    private static SearchResponse.Span convertSWSpanToSearchSpan(
+        org.apache.skywalking.oap.server.core.query.type.Span swSpan, 
Set<String> allSpanAttrKeys) {
+
+        SearchResponse.Span span = new SearchResponse.Span();
+        span.setSpanID(swSpan.getSegmentSpanId());
+        span.setStartTimeUnixNano(String.valueOf(swSpan.getStartTime() * 
1_000_000));
+        span.setDurationNanos(String.valueOf((swSpan.getEndTime() - 
swSpan.getStartTime()) * 1_000_000));
+
+        // Build attribute map for this span
+        Map<String, String> spanAttrMap = new java.util.LinkedHashMap<>();
+        if (swSpan.getServiceCode() != null) {
+            spanAttrMap.put(SERVICE_NAME, swSpan.getServiceCode());
+        }
+        if (swSpan.getType() != null) {
+            spanAttrMap.put(SPAN_KIND, 
convertSpanKind(swSpan.getType()).name());
+        }
+        for (org.apache.skywalking.oap.server.core.query.type.KeyValue tag : 
swSpan.getTags()) {
+            spanAttrMap.put(tag.getKey(), tag.getValue());
+        }
+
+        // Output all keys in consistent order, padding missing keys with "" 
to avoid
+        // the Grafana search.go:369 type panic on []*string fields
+        List<SearchResponse.Attribute> attributes = new ArrayList<>();
+        for (String key : allSpanAttrKeys) {
+            SearchResponse.Attribute attr = new SearchResponse.Attribute();
+            attr.setKey(key);
+            SearchResponse.Value value = new SearchResponse.Value();
+            value.setStringValue(spanAttrMap.getOrDefault(key, ""));
+            attr.setValue(value);
+            attributes.add(attr);
+        }
+
+        span.setAttributes(attributes);
+        return span;
+    }
+
+    /**
+     * Convert SkyWalking span type to OTLP span kind.
+     */
+    private static io.opentelemetry.proto.trace.v1.Span.SpanKind 
convertSpanKind(String type) {
+        if (StringUtil.isEmpty(type)) {
+            return 
io.opentelemetry.proto.trace.v1.Span.SpanKind.SPAN_KIND_UNSPECIFIED;
+        }
+
+        switch (type.toUpperCase()) {
+            case "ENTRY":
+                return 
io.opentelemetry.proto.trace.v1.Span.SpanKind.SPAN_KIND_SERVER;
+            case "EXIT":
+                return 
io.opentelemetry.proto.trace.v1.Span.SpanKind.SPAN_KIND_CLIENT;
+            case "LOCAL":
+                return 
io.opentelemetry.proto.trace.v1.Span.SpanKind.SPAN_KIND_INTERNAL;
+            default:
+                return 
io.opentelemetry.proto.trace.v1.Span.SpanKind.SPAN_KIND_INTERNAL;
+        }
+    }
+
+    /**
+     * Encode a SkyWalking trace ID to a lowercase hex string.
+     * SkyWalking trace IDs may contain letters, digits, '.' and '=',
+     * which are not valid in OTLP/Tempo trace IDs.
+     * Each UTF-8 byte of the trace ID is encoded as two hex characters,
+     * producing a hex-only string that Grafana accepts and that can be
+     * decoded back to the original trace ID without any loss.
+     *
+     * Examples:
+     *   "2a2e04e8d1114b14925c04a6321ca26c.38.17739924187687539"
+     *   → 
"326132653034653864313131346231343932356330346136333231636132366332653333382e31373733393932343138373638373533"
+     *   "abc123=.test"
+     *   → "61626331323333643265746573"  (encode)  → "abc123=.test"  (decode)

Review Comment:
   The Javadoc example for encoding `"abc123=.test"` appears to have an 
incorrect hex value (`61626331323333643265746573`). If the implementation is 
“UTF-8 bytes to lowercase hex”, the example should reflect the actual 
byte-to-hex output so readers can validate the behavior.
   ```suggestion
        *   → "6162633132333d2e74657374"  (encode)  → "abc123=.test"  (decode)
   ```



##########
oap-server/server-starter/src/main/resources/application.yml:
##########
@@ -509,6 +509,11 @@ traceQL:
     restContextPathSkywalking: 
${SW_TRACEQL_REST_CONTEXT_PATH_SKYWALKING:/skywalking}
     restIdleTimeOut: ${SW_TRACEQL_REST_IDLE_TIMEOUT:30000}
     restAcceptQueueSize: ${SW_TRACEQL_REST_QUEUE_SIZE:0}
+    # If query not provide the start time (the default end time is current), 
the default look back for traces and autocompleteTags, 1 day in millis

Review Comment:
   Grammar in the comment: "If query not provide" should be corrected for 
readability (e.g., "If the query does not provide").
   ```suggestion
       # If the query does not provide the start time (the default end time is 
current), the default lookback for traces and autocompleteTags is 1 day in 
milliseconds.
   ```



##########
docs/en/api/traceql-service.md:
##########
@@ -664,23 +668,172 @@ GET /zipkin/api/v2/traces/f321ebb45ffee8b5
 **Response (Protobuf)**:
 When `Accept: application/protobuf` header is set, the response will be in 
OpenTelemetry Protobuf format.
 
+## Zipkin Trace Conversion
+
+When using the Zipkin backend, the following conversions are applied:
+
+### Span Kind Mapping
+Zipkin span kinds are mapped to OTLP span kinds:
+
+| Zipkin Span Kind | OTLP Span Kind          |
+|------------------|-------------------------|
+| `CLIENT`         | `SPAN_KIND_CLIENT`      |
+| `SERVER`         | `SPAN_KIND_SERVER`      |
+| `PRODUCER`       | `SPAN_KIND_PRODUCER`    |
+| `CONSUMER`       | `SPAN_KIND_CONSUMER`    |
+| (absent)         | `SPAN_KIND_UNSPECIFIED` |
+| (other)          | `SPAN_KIND_INTERNAL`    |
+
+### Status Mapping
+Zipkin tags are used to derive the OTLP span status in the following priority 
order:
+
+1. If the `otel.status_code` tag is present, it is parsed directly as the OTLP 
`StatusCode` (e.g. `STATUS_CODE_OK`, `STATUS_CODE_ERROR`).
+2. Otherwise, if the `error` tag equals `true` (case-insensitive), the status 
is set to `STATUS_CODE_ERROR`.
+3. If neither tag is present, the status defaults to `STATUS_CODE_UNSET`.
+
+The `otel.status_description` tag, if present, is used as the status message.
+
+### Endpoint Mapping
+Zipkin endpoint fields are mapped to OTLP span attributes:
+
+| Zipkin Field                 | OTLP Attribute                  |
+|------------------------------|---------------------------------|
+| `localEndpoint.ipv4`         | `net.host.ip`                   |
+| `localEndpoint.ipv6`         | `net.host.ip`                   |
+| `localEndpoint.port`         | `net.host.port`                 |
+| `remoteEndpoint.serviceName` | `net.peer.name`, `peer.service` |
+| `remoteEndpoint.ipv4`        | `net.peer.ip`                   |
+| `remoteEndpoint.ipv6`        | `net.peer.ip`                   |
+| `remoteEndpoint.port`        | `net.peer.port`                 |
+
+### Annotations
+Zipkin annotations are converted to OTLP span events. Each annotation becomes 
a `Span.Event` with `timeUnixNano` (converted from microseconds) and `name` set 
to the annotation value.
+
+### Instrumentation Scope
+All spans within a Zipkin trace carry the instrumentation scope:
+```
+name: "zipkin-tracer"
+version: "0.1.0"
+```
+
+## SkyWalking Native Trace Conversion
+
+When using the SkyWalking native backend, the following conversions are 
applied:
+
+### Trace ID Encoding
+
+SkyWalking native trace IDs are arbitrary strings that may contain characters 
outside the hexadecimal alphabet
+(for example, `2a2e04e8d1114b14925c04a6321ca26c.38.17739924187687539` includes 
`.` separators).
+OTLP and Grafana Tempo require trace IDs to be pure hex strings.
+
+To satisfy this constraint, every SkyWalking trace ID is encoded by converting 
each UTF-8 byte of the
+original string to two lowercase hex characters:
+
+```
+Original: 2a2e04e8d1114b14925c04a6321ca26c.38.17739924187687539
+Encoded:  
32613265303465386431313134623134393235633034613633323163613236632e33382e3137373339393234313837363837353339
+```
+
+- **Encoding**: `traceId.getBytes(UTF-8)` → each byte formatted as `%02x` → 
concatenated lowercase hex string.
+- **Decoding**: hex string → byte array → `new String(bytes, UTF-8)` → 
original SkyWalking trace ID.
+
+The encoded trace ID is what appears in all API responses (e.g., the `traceID` 
field in [Search Traces](#search-traces)).
+When calling [Query Trace by ID](#query-trace-by-id-v1), use the encoded hex 
form as the `{traceId}` path parameter.
+
+### Span Kind Mapping
+SkyWalking span types are mapped to OTLP span kinds:
+
+| SkyWalking Span Type | OTLP Span Kind          |
+|----------------------|-------------------------|
+| `Entry`              | `SPAN_KIND_SERVER`      |
+| `Exit`               | `SPAN_KIND_CLIENT`      |
+| `Local`              | `SPAN_KIND_INTERNAL`    |
+| (absent)             | `SPAN_KIND_UNSPECIFIED` |
+| (other)              | `SPAN_KIND_UNSPECIFIED` |

Review Comment:
   The SkyWalking span kind mapping table says “(other) → 
`SPAN_KIND_UNSPECIFIED`”, but the implementation in 
`SkyWalkingOTLPConverter.convertSpanKind` defaults to `SPAN_KIND_INTERNAL`. 
Please align the documentation with the actual behavior (or adjust the code to 
match the documented mapping).
   ```suggestion
   | (other)              | `SPAN_KIND_INTERNAL`    |
   ```



##########
oap-server/server-query-plugin/traceql-plugin/src/main/java/org/apache/skywalking/oap/query/traceql/TraceQLConfig.java:
##########
@@ -33,4 +34,23 @@ public class TraceQLConfig extends ModuleConfig {
     private String restContextPathSkywalking;
     private long restIdleTimeOut = 30000;
     private int restAcceptQueueSize = 0;
+    private long lookback = 86400000L;
+    private String zipkinTracesListResultTags = ZIPKIN_TRACES_LIST_RESULT_TAGS;
+    private String skywalkingTracesListResultTags = 
SKYWALKING_TRACES_LIST_RESULT_TAGS;
+
+    private static final String ZIPKIN_TRACES_LIST_RESULT_TAGS = String.join(
+        Const.COMMA,
+        "http.method"

Review Comment:
   `zipkinTracesListResultTags` default here only includes `http.method`, but 
`application.yml`/docs default includes `http.method,error`. This inconsistency 
can lead to different behavior depending on whether config binding populates 
this field or not. Consider aligning `ZIPKIN_TRACES_LIST_RESULT_TAGS` with the 
documented default (or removing the field initializer and relying on the 
external config defaults consistently).
   ```suggestion
           "http.method",
           "error"
   ```



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