Copilot commented on code in PR #791:
URL:
https://github.com/apache/skywalking-banyandb/pull/791#discussion_r2378130183
##########
banyand/trace/trace.go:
##########
@@ -182,7 +182,12 @@ func (t *trace) querySidxForTraceIDs(ctx context.Context,
sidxInstances []sidx.S
for i, data := range response.Data {
if len(data) > 0 {
- traceID := string(data)
+ var traceID string
+ if len(data) > 0 && idFormat(data[0]) == idFormatV1 {
Review Comment:
The condition `len(data) > 0` is checked twice - once on line 184 and again
on line 186. The second check is redundant since it's already within the first
condition's block.
```suggestion
if idFormat(data[0]) == idFormatV1 {
```
##########
banyand/trace/trace.go:
##########
@@ -182,7 +182,12 @@ func (t *trace) querySidxForTraceIDs(ctx context.Context,
sidxInstances []sidx.S
for i, data := range response.Data {
if len(data) > 0 {
- traceID := string(data)
+ var traceID string
+ if len(data) > 0 && idFormat(data[0]) == idFormatV1 {
+ traceID = string(data[1:])
Review Comment:
The logic doesn't handle legacy trace IDs without control bits. According to
the PR description, this should maintain backward compatibility, but legacy
trace IDs (without control bits) will trigger a panic. Consider adding an
else-if branch to handle legacy format by treating the entire data as the trace
ID.
```suggestion
traceID = string(data[1:])
} else if len(data) > 0 {
// Legacy trace ID format: treat entire data as
trace ID
traceID = string(data)
```
--
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]
