feat(querysvc): Compute trace summaries natively or fall back to full-trace aggregation

cmd/jaeger/internal/extension/jaegerquery/querysvc/service.go

@@ -162,13 +162,27 @@ func (qs QueryService) FindTraces(
 // of lightweight summary information. If the underlying storage implements
 // tracestore.SummaryReader, it delegates to that; otherwise it falls back to
 // FindTraces and computes summaries from the full trace data.
+//
+// A SummaryReader implementation that does not support the operation should return
+// errors.ErrUnsupported (wrapped with %w) as the direct error, which causes
+// FindTraceSummaries to fall back transparently to computeSummaries.
+//
 // The iterator is single-use: once consumed, it cannot be used again.
 func (qs QueryService) FindTraceSummaries(
 	ctx context.Context,
 	query TraceQueryParams,
 ) iter.Seq2[[]tracestore.TraceSummary, error] {
 	if sr := findSummaryReader(qs.traceReader); sr != nil {
-		return sr.FindTraceSummaries(ctx, query.TraceQueryParams)
+		seqIter, err := sr.FindTraceSummaries(ctx, query.TraceQueryParams)
+		if err == nil {
+			return seqIter
+		}
+		if !errors.Is(err, errors.ErrUnsupported) {
+			return func(yield func([]tracestore.TraceSummary, error) bool) {
+				yield(nil, err)
+			}
+		}
+		// ErrUnsupported — fall through to computeSummaries
 	}
 	return computeSummaries(qs.traceReader.FindTraces(ctx, query.TraceQueryParams), qs.adjuster)
 }

cmd/jaeger/internal/extension/jaegerquery/querysvc/summary_test.go

@@ -5,6 +5,8 @@ package querysvc
 
 import (
 	"context"
+	"errors"
+	"fmt"
 	"iter"
 	"testing"
 	"time"
@@ -191,16 +193,15 @@ type mockSummaryReader struct {
 	err       error
 }
 
-func (m *mockSummaryReader) FindTraceSummaries(_ context.Context, _ tracestore.TraceQueryParams) iter.Seq2[[]tracestore.TraceSummary, error] {
+func (m *mockSummaryReader) FindTraceSummaries(_ context.Context, _ tracestore.TraceQueryParams) (iter.Seq2[[]tracestore.TraceSummary, error], error) {
+	if m.err != nil {
+		return nil, m.err
+	}
 	return func(yield func([]tracestore.TraceSummary, error) bool) {
-		if m.err != nil {
-			yield(nil, m.err)
-			return
-		}
 		if len(m.summaries) > 0 {
 			yield(m.summaries, nil)
 		}
-	}
+	}, nil
 }
 
 func TestFindTraceSummaries_NativePath(t *testing.T) {
@@ -243,3 +244,28 @@ func TestFindTraceSummaries_NativePath_ThroughWrapper(t *testing.T) {
 	assert.Equal(t, want, got)
 	wrapped.AssertNotCalled(t, "FindTraces")
 }
+
+// TestFindTraceSummaries_ErrUnsupported verifies that when a SummaryReader returns
+// errors.ErrUnsupported as the direct error, QueryService transparently falls back
+// to FindTraces + computeSummaries rather than propagating the error to the caller.
+func TestFindTraceSummaries_ErrUnsupported(t *testing.T) {
+	unsupportedReader := &mockSummaryReader{
+		err: fmt.Errorf("remote storage does not support FindTraceSummaries: %w", errors.ErrUnsupported),
+	}
+	trace := makeTestTrace()
+	unsupportedReader.On("FindTraces", mock.Anything, mock.Anything).
+		Return(iter.Seq2[[]ptrace.Traces, error](func(yield func([]ptrace.Traces, error) bool) {
+			yield([]ptrace.Traces{trace}, nil)
+		})).Once()
+
+	depsMock := initializeTestService().depsReader
+	qs := NewQueryService(unsupportedReader, depsMock, QueryServiceOptions{})
+
+	got, err := jiter.FlattenWithErrors(qs.FindTraceSummaries(context.Background(), TraceQueryParams{
+		TraceQueryParams: tracestore.TraceQueryParams{Attributes: pcommon.NewMap()},
+	}))
+	require.NoError(t, err)
+	require.Len(t, got, 1, "expected one summary from fallback aggregation")
+	// Verify the fallback produced a real summary from the trace data.
+	assert.Equal(t, trace.SpanCount(), got[0].SpanCount)
+}

cmd/jaeger/internal/integration/trace_reader.go

@@ -5,13 +5,16 @@ package integration
 
 import (
 	"context"
+	"encoding/hex"
 	"errors"
 	"fmt"
 	"io"
 	"iter"
 	"math"
 	"strings"
+	"time"
 
+	"go.opentelemetry.io/collector/pdata/pcommon"
 	"go.opentelemetry.io/collector/pdata/ptrace"
 	"go.uber.org/zap"
 	"google.golang.org/grpc"
@@ -27,8 +30,9 @@ import (
 )
 
 var (
-	_ tracestore.Reader = (*traceReader)(nil)
-	_ io.Closer         = (*traceReader)(nil)
+	_ tracestore.Reader        = (*traceReader)(nil)
+	_ tracestore.SummaryReader = (*traceReader)(nil)
+	_ io.Closer                = (*traceReader)(nil)
 )
 
 // traceReader retrieves trace data from the jaeger-v2 query service through the api_v2.QueryServiceClient.
@@ -141,6 +145,72 @@ func (*traceReader) FindTraceIDs(
 	panic("not implemented")
 }
 
+func (r *traceReader) FindTraceSummaries(
+	ctx context.Context,
+	query tracestore.TraceQueryParams,
+) (iter.Seq2[[]tracestore.TraceSummary, error], error) {
+	if query.SearchDepth > math.MaxInt32 {
+		return nil, fmt.Errorf("SearchDepth must not be greater than %d", math.MaxInt32)
+	}
+	stream, err := r.client.FindTraceSummaries(ctx, &api_v3.FindTraceSummariesRequest{
+		Query: &api_v3.TraceQueryParameters{
+			ServiceName:   query.ServiceName,
+			OperationName: query.OperationName,
+			Attributes:    jptrace.PcommonMapToPlainMap(query.Attributes),
+			StartTimeMin:  query.StartTimeMin,
+			StartTimeMax:  query.StartTimeMax,
+			DurationMin:   query.DurationMin,
+			DurationMax:   query.DurationMax,
+			SearchDepth:   int32(query.SearchDepth), //nolint:gosec // G115 - bounds checked above
+		},
+	})
+	if err != nil {
+		return nil, err
+	}
+	return func(yield func([]tracestore.TraceSummary, error) bool) {
+		for {
+			resp, err := stream.Recv()
+			if errors.Is(err, io.EOF) {
+				return
+			}
+			if err != nil {
+				yield(nil, err)
+				return
+			}
+			batch := make([]tracestore.TraceSummary, len(resp.GetSummaries()))
+			for i, ps := range resp.GetSummaries() {
+				traceID, parseErr := traceIDFromHex(ps.GetTraceId())
+				if parseErr != nil {
+					yield(nil, parseErr)
+					return
+				}
+				svcs := make([]tracestore.ServiceSummary, len(ps.GetServices()))
+				for j, ss := range ps.GetServices() {
+					svcs[j] = tracestore.ServiceSummary{
+						Name:           ss.GetName(),
+						SpanCount:      int(ss.GetSpanCount()),
+						ErrorSpanCount: int(ss.GetErrorSpanCount()),
+					}
+				}
+				batch[i] = tracestore.TraceSummary{
+					TraceID:           traceID,
+					RootServiceName:   ps.GetRootServiceName(),
+					RootOperationName: ps.GetRootOperationName(),
+					MinStartTime:      unixNanoToTime(ps.GetMinStartTimeUnixNano()),
+					MaxEndTime:        unixNanoToTime(ps.GetMaxEndTimeUnixNano()),
+					SpanCount:         int(ps.GetSpanCount()),
+					ErrorSpanCount:    int(ps.GetErrorSpanCount()),
+					OrphanSpanCount:   int(ps.GetOrphanSpanCount()),
+					Services:          svcs,
+				}
+			}
+			if !yield(batch, nil) {
+				return
+			}
+		}
+	}, nil
+}
+
 type traceStream interface {
 	Recv() (*jptrace.TracesData, error)
 }
@@ -187,3 +257,24 @@ func unwrapNotFoundErr(err error) error {
 	}
 	return err
 }
+
+// unixNanoToTime converts a uint64 Unix nanosecond timestamp to time.Time.
+// Returns zero time for a zero value (field omitted in proto).
+func unixNanoToTime(nano uint64) time.Time {
+	if nano == 0 {
+		return time.Time{}
+	}
+	return time.Unix(0, int64(nano)).UTC() //nolint:gosec // G115
+}
+
+// traceIDFromHex parses a 32-character hex string into a pcommon.TraceID.
+func traceIDFromHex(s string) (pcommon.TraceID, error) {
+	b, err := hex.DecodeString(s)
+	if err != nil {
+		return pcommon.TraceID{}, fmt.Errorf("invalid trace ID %q: %w", s, err)
+	}
+	if len(b) != 16 {
+		return pcommon.TraceID{}, fmt.Errorf("trace ID must be 16 bytes, got %d", len(b))
+	}
+	return pcommon.TraceID(b), nil
+}

docs/adr/010-trace-summary-api.md

@@ -220,7 +220,7 @@ existing storage implementations), a new **optional** interface is introduced:
 // or more summaries, and implementations may yield results incrementally as the
 // underlying query executes rather than buffering all results first.
 type SummaryReader interface {
-    FindTraceSummaries(ctx context.Context, query TraceQueryParams) iter.Seq2[[]TraceSummary, error]
+    FindTraceSummaries(ctx context.Context, query TraceQueryParams) (iter.Seq2[[]TraceSummary, error], error)
 }
 
 // ServiceSummary holds per-service statistics for a single trace.
@@ -292,9 +292,36 @@ for callers to derive.
 
 The gRPC-based remote storage adapter (`plugin/storage/grpc/`) wraps the remote
 `TraceReader` gRPC client. Its `FindTraceSummaries` implementation calls the remote RPC
-and, if the server returns `codes.Unimplemented`, falls back to calling `FindTraces`
-and computing summaries client-side. This makes the feature work transparently with
-existing remote storage plugins that have not yet implemented the new RPC.
+and, if the server returns `codes.Unimplemented`, returns an error wrapping
+`errors.ErrUnsupported` directly (before returning an iterator). This makes the feature
+work transparently with existing remote storage plugins that have not yet implemented the
+new RPC.
+
+`SummaryReader.FindTraceSummaries` returns a direct error (not via the iterator) when
+the capability is unavailable, using `errors.ErrUnsupported` (Go 1.21 standard sentinel)
+as the sentinel value. `QueryService.FindTraceSummaries` checks this direct error and
+falls back to `FindTraces` + `computeSummaries` when it wraps `ErrUnsupported`. This
+design lets the caller detect "not supported" immediately — before starting any iteration
+— avoiding unnecessary work.
+
+Using `errors.ErrUnsupported` rather than a Jaeger-specific sentinel keeps the interface
+clean: any `SummaryReader` implementation can signal "not available" without importing
+internal packages.
+
+```go
+// In QueryService.FindTraceSummaries (simplified):
+if sr := findSummaryReader(qs.traceReader); sr != nil {
+    seqIter, err := sr.FindTraceSummaries(ctx, query)
+    if err == nil {
+        return seqIter
+    }
+    if !errors.Is(err, errors.ErrUnsupported) {
+        return func(yield func([]tracestore.TraceSummary, error) bool) { yield(nil, err) }
+    }
+    // ErrUnsupported — fall through to computeSummaries
+}
+return computeSummaries(qs.traceReader.FindTraces(ctx, query), qs.adjuster)
+```
 
 ### 7. gRPC and HTTP Handlers
 
@@ -429,6 +456,34 @@ matters only if a non-conforming or mocked backend is used in tests.
 
 ---
 
+### 9. Integration Tests
+
+The existing `TestJaegerQueryService` integration test (`cmd/jaeger/internal/integration/query_test.go`)
+runs two Jaeger instances connected over gRPC remote storage. It exercises the full stack but
+did not previously cover the `FindTraceSummaries` endpoint.
+
+**End-to-end test coverage added:**
+
+- `traceReader` in `cmd/jaeger/internal/integration/trace_reader.go` implements
+  `tracestore.SummaryReader` by calling the `api_v3.QueryService.FindTraceSummaries` gRPC RPC
+  and converting `api_v3.TraceSummary` proto messages to `tracestore.TraceSummary`.
+- `StorageIntegration` in `internal/storage/integration/integration.go` always runs a
+  `FindTraceSummaries` sub-test via `RunSpanStoreTests`. The test casts `TraceReader` to
+  `tracestore.SummaryReader` and fails loudly if the cast does not succeed. Storage backends
+  that do not yet implement `SummaryReader` opt out by adding `"FindTraceSummaries"` to their
+  `Capabilities.SkipList`. The sub-test:
+  1. Writes the `example_trace` fixture via the trace writer.
+  2. Queries summaries with a time window covering the trace.
+  3. Asserts the returned summary matches the expected trace ID, span count, and non-zero timestamps.
+- `traceReader` already implements both `tracestore.Reader` and `tracestore.SummaryReader`,
+  so the e2e integration test gains `FindTraceSummaries` coverage automatically — no extra
+  field wiring or separate binary needed.
+
+This exercises the complete path:
+`HTTP/gRPC handler → QueryService (fallback aggregation) → gRPC remote storage reader → memory backend`
+
+---
+
 ## Alternatives Considered
 
 ### A. Add query parameter `summary=true` to `FindTraces`
@@ -498,10 +553,11 @@ the HTTP contract before touching other repositories.
 **Delivered:**
 1. `tracestore.ServiceSummary`, `tracestore.TraceSummary`, and the optional `tracestore.SummaryReader` interface (`internal/storage/v2/api/tracestore/summary.go`).
 2. `computeSummaries` fallback aggregation in `querysvc/summary.go`, using `jptrace.AggregateTraces` to reassemble multi-chunk traces before summarizing.
-3. `querysvc.QueryService.FindTraceSummaries` with both the `SummaryReader` native path and the fallback path. The `SummaryReader` discovery uses a chain-walker (`findSummaryReader`) that traverses `Unwrap()` on decorator types (e.g. `ReadMetricsDecorator`).
+3. `querysvc.QueryService.FindTraceSummaries` with both the `SummaryReader` native path and the fallback path. The `SummaryReader` discovery uses a chain-walker (`findSummaryReader`) that traverses `Unwrap()` on decorator types (e.g. `ReadMetricsDecorator`). If the `SummaryReader` yields `errors.ErrUnsupported`, `QueryService` falls back transparently to `computeSummaries` (see §6).
 4. `GET /api/v3/trace-summaries` in the HTTP gateway, reusing `parseFindTracesQuery`. Response is plain JSON; timestamps encoded as decimal strings per proto3 JSON convention.
 5. `query.search_depth` is the canonical query parameter (matching the proto field); `query.num_traces` is accepted as a deprecated alias (jaegertracing/jaeger#8617). Defaults to 100 when omitted.
-6. Unit tests for `computeSummaries` (empty, error, multi-service, multi-chunk, orphan spans), `FindTraceSummaries` (fallback path, native `SummaryReader`, `SummaryReader` through decorator chain), HTTP handler (success, storage error, deprecated alias).
+6. Unit tests for `computeSummaries` (empty, error, multi-service, multi-chunk, orphan spans), `FindTraceSummaries` (fallback path, native `SummaryReader`, `SummaryReader` through decorator chain, `ErrUnsupported` fallback), HTTP handler (success, storage error, deprecated alias).
+7. Integration test: `FindTraceSummaries` added to `RunSpanStoreTests`, exercised end-to-end via `TestJaegerQueryService` (see §9).
 
 ---
 

internal/storage/integration/capabilities/capabilities.go

@@ -4,8 +4,9 @@
 package capabilities
 
 const (
-	scopeAttributesTest = "Scope_Attributes"
-	linkAttributesTest  = "Link_Attributes"
+	scopeAttributesTest    = "Scope_Attributes"
+	linkAttributesTest     = "Link_Attributes"
+	FindTraceSummariesTest = "FindTraceSummaries"
 )
 
 // Capabilities defines the capabilities of a storage backend for integration tests.
@@ -33,6 +34,20 @@ func (c Capabilities) SkipList() []string {
 	return c.skipList
 }
 
+// Memory returns the capabilities for the in-process memory storage backend.
+func Memory() Capabilities {
+	return Capabilities{
+		skipList: []string{FindTraceSummariesTest},
+	}
+}
+
+// GRPC returns the capabilities for the gRPC remote storage backend.
+func GRPC() Capabilities {
+	return Capabilities{
+		skipList: []string{FindTraceSummariesTest},
+	}
+}
+
 // Cassandra returns the capabilities for the Cassandra storage backend.
 func Cassandra() Capabilities {
 	return Capabilities{
@@ -46,14 +61,15 @@ func Cassandra() Capabilities {
 			"Multiple_Traces",
 			scopeAttributesTest,
 			linkAttributesTest,
+			FindTraceSummariesTest,
 		},
 	}
 }
 
 // ClickHouse returns the capabilities for the ClickHouse storage backend.
 func ClickHouse() Capabilities {
 	return Capabilities{
-		skipList: []string{"GetThroughput", "GetLatestProbability"},
+		skipList: []string{"GetThroughput", "GetLatestProbability", FindTraceSummariesTest},
 	}
 }
 
@@ -62,7 +78,7 @@ func Badger() Capabilities {
 	return Capabilities{
 		// TODO: remove this once Badger supports returning spanKind from GetOperations
 		getOperationsMissingSpanKind: true,
-		skipList:                     []string{scopeAttributesTest, linkAttributesTest},
+		skipList:                     []string{scopeAttributesTest, linkAttributesTest, FindTraceSummariesTest},
 	}
 }
 
@@ -72,22 +88,22 @@ func Elasticsearch() Capabilities {
 		// TODO: remove this flag after ES supports returning spanKind
 		//  Issue https://github.com/jaegertracing/jaeger/issues/1923
 		getOperationsMissingSpanKind: true,
-		skipList:                     []string{scopeAttributesTest, linkAttributesTest},
+		skipList:                     []string{scopeAttributesTest, linkAttributesTest, FindTraceSummariesTest},
 	}
 }
 
 // OpenSearch defines the capabilities for the OpenSearch storage backend.
 func OpenSearch() Capabilities {
 	return Capabilities{
 		getOperationsMissingSpanKind: true,
-		skipList:                     []string{scopeAttributesTest, linkAttributesTest},
+		skipList:                     []string{scopeAttributesTest, linkAttributesTest, FindTraceSummariesTest},
 	}
 }
 
 // Kafka defines the capabilities for the Kafka storage backend.
 func Kafka() Capabilities {
 	return Capabilities{
 		getDependenciesMissingSource: true,
-		skipList:                     []string{scopeAttributesTest, linkAttributesTest},
+		skipList:                     []string{scopeAttributesTest, linkAttributesTest, FindTraceSummariesTest},
 	}
 }