Copilot commented on code in PR #100:
URL:
https://github.com/apache/skywalking-banyandb-java-client/pull/100#discussion_r2386522228
##########
src/main/java/org/apache/skywalking/banyandb/v1/client/TraceQueryResponse.java:
##########
@@ -43,14 +45,9 @@ public List<BanyandbTrace.Trace> getTraces() {
/**
* Get the trace query execution trace if enabled.
Review Comment:
The method documentation is incomplete. It should specify the return type
and describe what the returned Trace object contains, especially since the
method signature changed from String to Trace.
```suggestion
* Returns the execution trace of the trace query if enabled.
*
* @return a {@link Trace} object representing the execution trace of
the query,
* or {@code null} if execution tracing is not enabled. The
returned {@link Trace}
* contains details about the query execution, such as timing
and internal operations,
* which can be useful for debugging or performance analysis.
```
##########
src/main/java/org/apache/skywalking/banyandb/v1/client/TraceQueryResponse.java:
##########
@@ -27,9 +27,11 @@
*/
public class TraceQueryResponse {
private final BanyandbTrace.QueryResponse response;
+ private final Trace trace;
TraceQueryResponse(BanyandbTrace.QueryResponse response) {
this.response = response;
+ this.trace = Trace.convertFromProto(response.getTraceQueryResult());
Review Comment:
The trace field is initialized unconditionally, but the original
implementation only returned trace data when `response.hasTraceQueryResult()`
was true. This could cause issues if the trace query result is not present.
Consider adding a null check: `this.trace = response.hasTraceQueryResult() ?
Trace.convertFromProto(response.getTraceQueryResult()) : null;`
```suggestion
this.trace = response.hasTraceQueryResult() ?
Trace.convertFromProto(response.getTraceQueryResult()) : null;
```
--
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]
