This is an automated email from the ASF dual-hosted git repository. wusheng pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/skywalking-banyandb.git
The following commit(s) were added to refs/heads/main by this push: new a287c38c feat: improve trace query functionality (#762) a287c38c is described below commit a287c38c758b11ea2d83014875af2e72acfaa017 Author: Gao Hongtao <hanahm...@gmail.com> AuthorDate: Tue Sep 9 16:24:33 2025 +0800 feat: improve trace query functionality (#762) --- api/proto/banyandb/trace/v1/query.proto | 10 ++- banyand/liaison/grpc/trace.go | 2 +- banyand/query/processor.go | 28 ++++++-- docs/api-reference.md | 18 ++++- test/cases/trace/data/data.go | 81 +++++++++++++++------- .../data/want/eq_endpoint_order_duration_asc.yml | 18 ++--- .../want/eq_service_instance_order_time_asc.yml | 30 ++++---- .../data/want/eq_service_order_timestamp_desc.yml | 33 +++++---- test/cases/trace/data/want/eq_trace_id.yml | 39 ++++++----- test/cases/trace/data/want/order_duration_desc.yml | 33 +++++---- .../cases/trace/data/want/order_timestamp_desc.yml | 33 +++++---- .../trace/data/want/order_timestamp_desc_limit.yml | 14 ++-- 12 files changed, 216 insertions(+), 123 deletions(-) diff --git a/api/proto/banyandb/trace/v1/query.proto b/api/proto/banyandb/trace/v1/query.proto index a0b8dabb..be453422 100644 --- a/api/proto/banyandb/trace/v1/query.proto +++ b/api/proto/banyandb/trace/v1/query.proto @@ -34,10 +34,16 @@ message Span { bytes span = 2; } +// Trace contains all spans that belong to a single trace ID. +message Trace { + // spans is the list of spans that belong to this trace. + repeated Span spans = 1; +} + // QueryResponse is the response of a query. message QueryResponse { - // spans is a list of spans that match the query. - repeated Span spans = 1; + // traces is a list of traces that match the query, with spans grouped by trace ID. + repeated Trace traces = 1; // trace_query_result contains the trace of the query execution if tracing is enabled. common.v1.Trace trace_query_result = 2; } diff --git a/banyand/liaison/grpc/trace.go b/banyand/liaison/grpc/trace.go index b2105ead..3b28f396 100644 --- a/banyand/liaison/grpc/trace.go +++ b/banyand/liaison/grpc/trace.go @@ -311,7 +311,7 @@ func (s *traceService) Write(stream tracev1.TraceService_WriteServer) error { } } -var emptyTraceQueryResponse = &tracev1.QueryResponse{Spans: make([]*tracev1.Span, 0)} +var emptyTraceQueryResponse = &tracev1.QueryResponse{Traces: make([]*tracev1.Trace, 0)} func (s *traceService) Query(ctx context.Context, req *tracev1.QueryRequest) (resp *tracev1.QueryResponse, err error) { for _, g := range req.Groups { diff --git a/banyand/query/processor.go b/banyand/query/processor.go index 3118f711..686eb7e9 100644 --- a/banyand/query/processor.go +++ b/banyand/query/processor.go @@ -534,7 +534,8 @@ func (p *traceQueryProcessor) executeQuery(ctx context.Context, queryCriteria *t } // Convert model.TraceResult iterator to tracev1.QueryResponse format - var spans []*tracev1.Span + // Each result contains spans from a single trace, so we can directly create traces + var traces []*tracev1.Trace // Check if trace ID tag should be included based on tag projection shouldIncludeTraceID := slices.Contains(queryCriteria.TagProjection, traceIDTagName) @@ -545,6 +546,16 @@ func (p *traceQueryProcessor) executeQuery(ctx context.Context, queryCriteria *t break } + if result.TID == "" { + // Skip spans without trace ID + continue + } + + // Create a trace for this result + trace := &tracev1.Trace{ + Spans: make([]*tracev1.Span, 0, len(result.Spans)), + } + // Convert each span in the trace result for i, spanBytes := range result.Spans { // Create trace tags from the result @@ -577,19 +588,26 @@ func (p *traceQueryProcessor) executeQuery(ctx context.Context, queryCriteria *t }) } - spans = append(spans, &tracev1.Span{ + span := &tracev1.Span{ Tags: traceTags, Span: spanBytes, - }) + } + trace.Spans = append(trace.Spans, span) } + + traces = append(traces, trace) } - resp = bus.NewMessage(bus.MessageID(now), &tracev1.QueryResponse{Spans: spans}) + resp = bus.NewMessage(bus.MessageID(now), &tracev1.QueryResponse{Traces: traces}) if !queryCriteria.Trace && p.slowQuery > 0 { latency := time.Since(n) if latency > p.slowQuery { - p.log.Warn().Dur("latency", latency).RawJSON("req", logger.Proto(queryCriteria)).Int("resp_count", len(spans)).Msg("trace slow query") + spanCount := 0 + for _, trace := range traces { + spanCount += len(trace.Spans) + } + p.log.Warn().Dur("latency", latency).RawJSON("req", logger.Proto(queryCriteria)).Int("resp_count", spanCount).Msg("trace slow query") } } return diff --git a/docs/api-reference.md b/docs/api-reference.md index e7d778cc..67a5e8f9 100644 --- a/docs/api-reference.md +++ b/docs/api-reference.md @@ -312,6 +312,7 @@ - [QueryRequest](#banyandb-trace-v1-QueryRequest) - [QueryResponse](#banyandb-trace-v1-QueryResponse) - [Span](#banyandb-trace-v1-Span) + - [Trace](#banyandb-trace-v1-Trace) - [banyandb/trace/v1/write.proto](#banyandb_trace_v1_write-proto) - [InternalWriteRequest](#banyandb-trace-v1-InternalWriteRequest) @@ -4584,7 +4585,7 @@ QueryResponse is the response of a query. | Field | Type | Label | Description | | ----- | ---- | ----- | ----------- | -| spans | [Span](#banyandb-trace-v1-Span) | repeated | spans is a list of spans that match the query. | +| traces | [Trace](#banyandb-trace-v1-Trace) | repeated | traces is a list of traces that match the query, with spans grouped by trace ID. | | trace_query_result | [banyandb.common.v1.Trace](#banyandb-common-v1-Trace) | | trace_query_result contains the trace of the query execution if tracing is enabled. | @@ -4607,6 +4608,21 @@ Span is a single operation within a trace. + +<a name="banyandb-trace-v1-Trace"></a> + +### Trace +Trace contains all spans that belong to a single trace ID. + + +| Field | Type | Label | Description | +| ----- | ---- | ----- | ----------- | +| spans | [Span](#banyandb-trace-v1-Span) | repeated | spans is the list of spans that belong to this trace. | + + + + + diff --git a/test/cases/trace/data/data.go b/test/cases/trace/data/data.go index ee5a6cec..292a158b 100644 --- a/test/cases/trace/data/data.go +++ b/test/cases/trace/data/data.go @@ -74,7 +74,7 @@ var VerifyFn = func(innerGm gm.Gomega, sharedContext helpers.SharedContext, args } innerGm.Expect(err).NotTo(gm.HaveOccurred(), query.String()) if args.WantEmpty { - innerGm.Expect(resp.Spans).To(gm.BeEmpty()) + innerGm.Expect(resp.Traces).To(gm.BeEmpty()) return } if args.Want == "" { @@ -86,19 +86,36 @@ var VerifyFn = func(innerGm gm.Gomega, sharedContext helpers.SharedContext, args unmarshalYAMLWithSpanEncoding(ww, want) if args.DisOrder { - slices.SortFunc(want.Spans, func(a, b *tracev1.Span) int { - // Sort by first tag value for consistency - if len(a.Tags) > 0 && len(b.Tags) > 0 { - return strings.Compare(a.Tags[0].Value.GetStr().GetValue(), b.Tags[0].Value.GetStr().GetValue()) + // Sort traces by first span's tag for consistency + slices.SortFunc(want.Traces, func(a, b *tracev1.Trace) int { + if len(a.Spans) > 0 && len(b.Spans) > 0 && len(a.Spans[0].Tags) > 0 && len(b.Spans[0].Tags) > 0 { + return strings.Compare(a.Spans[0].Tags[0].Value.GetStr().GetValue(), b.Spans[0].Tags[0].Value.GetStr().GetValue()) } return 0 }) - slices.SortFunc(resp.Spans, func(a, b *tracev1.Span) int { - if len(a.Tags) > 0 && len(b.Tags) > 0 { - return strings.Compare(a.Tags[0].Value.GetStr().GetValue(), b.Tags[0].Value.GetStr().GetValue()) + slices.SortFunc(resp.Traces, func(a, b *tracev1.Trace) int { + if len(a.Spans) > 0 && len(b.Spans) > 0 && len(a.Spans[0].Tags) > 0 && len(b.Spans[0].Tags) > 0 { + return strings.Compare(a.Spans[0].Tags[0].Value.GetStr().GetValue(), b.Spans[0].Tags[0].Value.GetStr().GetValue()) } return 0 }) + // Sort spans within each trace for consistent ordering + for _, trace := range want.Traces { + slices.SortFunc(trace.Spans, func(a, b *tracev1.Span) int { + if len(a.Tags) > 0 && len(b.Tags) > 0 { + return strings.Compare(a.Tags[0].Value.GetStr().GetValue(), b.Tags[0].Value.GetStr().GetValue()) + } + return 0 + }) + } + for _, trace := range resp.Traces { + slices.SortFunc(trace.Spans, func(a, b *tracev1.Span) int { + if len(a.Tags) > 0 && len(b.Tags) > 0 { + return strings.Compare(a.Tags[0].Value.GetStr().GetValue(), b.Tags[0].Value.GetStr().GetValue()) + } + return 0 + }) + } } var extra []cmp.Option extra = append(extra, protocmp.IgnoreUnknown(), @@ -219,13 +236,19 @@ func unmarshalYAMLWithSpanEncoding(yamlData []byte, response *tracev1.QueryRespo err = json.Unmarshal(j, &jsonData) gm.Expect(err).NotTo(gm.HaveOccurred()) - // Convert span strings to base64 - if spans, ok := jsonData["spans"].([]interface{}); ok { - for _, spanInterface := range spans { - if span, ok := spanInterface.(map[string]interface{}); ok { - if spanValue, ok := span["span"].(string); ok { - // Encode the plain string as base64 - span["span"] = base64.StdEncoding.EncodeToString([]byte(spanValue)) + // Convert span strings to base64 in traces structure + if traces, ok := jsonData["traces"].([]interface{}); ok { + for _, traceInterface := range traces { + if trace, ok := traceInterface.(map[string]interface{}); ok { + if spans, ok := trace["spans"].([]interface{}); ok { + for _, spanInterface := range spans { + if span, ok := spanInterface.(map[string]interface{}); ok { + if spanValue, ok := span["span"].(string); ok { + // Encode the plain string as base64 + span["span"] = base64.StdEncoding.EncodeToString([]byte(spanValue)) + } + } + } } } } @@ -254,18 +277,24 @@ func marshalToJSONWithStringBytes(resp *tracev1.QueryResponse) ([]byte, error) { return nil, err } - // Convert base64 encoded span fields back to strings - if spans, ok := jsonData["spans"].([]interface{}); ok { - for _, spanInterface := range spans { - if span, ok := spanInterface.(map[string]interface{}); ok { - if spanB64, ok := span["span"].(string); ok { - // Decode base64 back to original string - spanBytes, err := base64.StdEncoding.DecodeString(spanB64) - if err != nil { - // If it's not valid base64, keep the original value - continue + // Convert base64 encoded span fields back to strings in traces structure + if traces, ok := jsonData["traces"].([]interface{}); ok { + for _, traceInterface := range traces { + if trace, ok := traceInterface.(map[string]interface{}); ok { + if spans, ok := trace["spans"].([]interface{}); ok { + for _, spanInterface := range spans { + if span, ok := spanInterface.(map[string]interface{}); ok { + if spanB64, ok := span["span"].(string); ok { + // Decode base64 back to original string + spanBytes, err := base64.StdEncoding.DecodeString(spanB64) + if err != nil { + // If it's not valid base64, keep the original value + continue + } + span["span"] = string(spanBytes) + } + } } - span["span"] = string(spanBytes) } } } diff --git a/test/cases/trace/data/want/eq_endpoint_order_duration_asc.yml b/test/cases/trace/data/want/eq_endpoint_order_duration_asc.yml index 3af289c8..5ff3b2b1 100644 --- a/test/cases/trace/data/want/eq_endpoint_order_duration_asc.yml +++ b/test/cases/trace/data/want/eq_endpoint_order_duration_asc.yml @@ -15,11 +15,13 @@ # specific language governing permissions and limitations # under the License. -spans: - - span: trace_005_span_1 - - span: trace_005_span_2 - - span: trace_005_span_3 - - span: trace_005_span_4 - - span: trace_001_span_1 - - span: trace_001_span_2 - - span: trace_001_span_3 +traces: + - spans: + - span: trace_005_span_1 + - span: trace_005_span_2 + - span: trace_005_span_3 + - span: trace_005_span_4 + - spans: + - span: trace_001_span_1 + - span: trace_001_span_2 + - span: trace_001_span_3 diff --git a/test/cases/trace/data/want/eq_service_instance_order_time_asc.yml b/test/cases/trace/data/want/eq_service_instance_order_time_asc.yml index a5540160..40c116cf 100644 --- a/test/cases/trace/data/want/eq_service_instance_order_time_asc.yml +++ b/test/cases/trace/data/want/eq_service_instance_order_time_asc.yml @@ -15,16 +15,20 @@ # specific language governing permissions and limitations # under the License. -spans: - - span: trace_001_span_1 - - span: trace_001_span_2 - - span: trace_001_span_3 - - span: trace_002_span_1 - - span: trace_002_span_2 - - span: trace_003_span_1 - - span: trace_003_span_2 - - span: trace_003_span_3 - - span: trace_005_span_1 - - span: trace_005_span_2 - - span: trace_005_span_3 - - span: trace_005_span_4 +traces: + - spans: + - span: trace_001_span_1 + - span: trace_001_span_2 + - span: trace_001_span_3 + - spans: + - span: trace_002_span_1 + - span: trace_002_span_2 + - spans: + - span: trace_003_span_1 + - span: trace_003_span_2 + - span: trace_003_span_3 + - spans: + - span: trace_005_span_1 + - span: trace_005_span_2 + - span: trace_005_span_3 + - span: trace_005_span_4 diff --git a/test/cases/trace/data/want/eq_service_order_timestamp_desc.yml b/test/cases/trace/data/want/eq_service_order_timestamp_desc.yml index 791dd197..66108cf9 100644 --- a/test/cases/trace/data/want/eq_service_order_timestamp_desc.yml +++ b/test/cases/trace/data/want/eq_service_order_timestamp_desc.yml @@ -15,17 +15,22 @@ # specific language governing permissions and limitations # under the License. -spans: - - span: trace_005_span_1 - - span: trace_005_span_2 - - span: trace_005_span_3 - - span: trace_005_span_4 - - span: trace_004_span_1 - - span: trace_003_span_1 - - span: trace_003_span_2 - - span: trace_003_span_3 - - span: trace_002_span_1 - - span: trace_002_span_2 - - span: trace_001_span_1 - - span: trace_001_span_2 - - span: trace_001_span_3 +traces: + - spans: + - span: trace_005_span_1 + - span: trace_005_span_2 + - span: trace_005_span_3 + - span: trace_005_span_4 + - spans: + - span: trace_004_span_1 + - spans: + - span: trace_003_span_1 + - span: trace_003_span_2 + - span: trace_003_span_3 + - spans: + - span: trace_002_span_1 + - span: trace_002_span_2 + - spans: + - span: trace_001_span_1 + - span: trace_001_span_2 + - span: trace_001_span_3 diff --git a/test/cases/trace/data/want/eq_trace_id.yml b/test/cases/trace/data/want/eq_trace_id.yml index 5fb3eeef..8e538949 100644 --- a/test/cases/trace/data/want/eq_trace_id.yml +++ b/test/cases/trace/data/want/eq_trace_id.yml @@ -15,22 +15,23 @@ # specific language governing permissions and limitations # under the License. -spans: - - span: trace_001_span_1 - tags: - - key: trace_id - value: - str: - value: "trace_001" - - span: trace_001_span_2 - tags: - - key: trace_id - value: - str: - value: "trace_001" - - span: trace_001_span_3 - tags: - - key: trace_id - value: - str: - value: "trace_001" +traces: + - spans: + - span: trace_001_span_1 + tags: + - key: trace_id + value: + str: + value: "trace_001" + - span: trace_001_span_2 + tags: + - key: trace_id + value: + str: + value: "trace_001" + - span: trace_001_span_3 + tags: + - key: trace_id + value: + str: + value: "trace_001" diff --git a/test/cases/trace/data/want/order_duration_desc.yml b/test/cases/trace/data/want/order_duration_desc.yml index e1e03c58..00b79f64 100644 --- a/test/cases/trace/data/want/order_duration_desc.yml +++ b/test/cases/trace/data/want/order_duration_desc.yml @@ -15,17 +15,22 @@ # specific language governing permissions and limitations # under the License. -spans: - - span: trace_003_span_1 - - span: trace_003_span_2 - - span: trace_003_span_3 - - span: trace_001_span_1 - - span: trace_001_span_2 - - span: trace_001_span_3 - - span: trace_005_span_1 - - span: trace_005_span_2 - - span: trace_005_span_3 - - span: trace_005_span_4 - - span: trace_002_span_1 - - span: trace_002_span_2 - - span: trace_004_span_1 +traces: + - spans: + - span: trace_003_span_1 + - span: trace_003_span_2 + - span: trace_003_span_3 + - spans: + - span: trace_001_span_1 + - span: trace_001_span_2 + - span: trace_001_span_3 + - spans: + - span: trace_005_span_1 + - span: trace_005_span_2 + - span: trace_005_span_3 + - span: trace_005_span_4 + - spans: + - span: trace_002_span_1 + - span: trace_002_span_2 + - spans: + - span: trace_004_span_1 diff --git a/test/cases/trace/data/want/order_timestamp_desc.yml b/test/cases/trace/data/want/order_timestamp_desc.yml index 791dd197..66108cf9 100644 --- a/test/cases/trace/data/want/order_timestamp_desc.yml +++ b/test/cases/trace/data/want/order_timestamp_desc.yml @@ -15,17 +15,22 @@ # specific language governing permissions and limitations # under the License. -spans: - - span: trace_005_span_1 - - span: trace_005_span_2 - - span: trace_005_span_3 - - span: trace_005_span_4 - - span: trace_004_span_1 - - span: trace_003_span_1 - - span: trace_003_span_2 - - span: trace_003_span_3 - - span: trace_002_span_1 - - span: trace_002_span_2 - - span: trace_001_span_1 - - span: trace_001_span_2 - - span: trace_001_span_3 +traces: + - spans: + - span: trace_005_span_1 + - span: trace_005_span_2 + - span: trace_005_span_3 + - span: trace_005_span_4 + - spans: + - span: trace_004_span_1 + - spans: + - span: trace_003_span_1 + - span: trace_003_span_2 + - span: trace_003_span_3 + - spans: + - span: trace_002_span_1 + - span: trace_002_span_2 + - spans: + - span: trace_001_span_1 + - span: trace_001_span_2 + - span: trace_001_span_3 diff --git a/test/cases/trace/data/want/order_timestamp_desc_limit.yml b/test/cases/trace/data/want/order_timestamp_desc_limit.yml index 6e8d4e68..c5fe78ce 100644 --- a/test/cases/trace/data/want/order_timestamp_desc_limit.yml +++ b/test/cases/trace/data/want/order_timestamp_desc_limit.yml @@ -15,10 +15,12 @@ # specific language governing permissions and limitations # under the License. -spans: - - span: trace_005_span_1 - - span: trace_005_span_2 - - span: trace_005_span_3 - - span: trace_005_span_4 - - span: trace_004_span_1 +traces: + - spans: + - span: trace_005_span_1 + - span: trace_005_span_2 + - span: trace_005_span_3 + - span: trace_005_span_4 + - spans: + - span: trace_004_span_1