Skip to content

[clickhouse] Update FindTraceIDs to populate start and end timestamps #7770

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
yurishkuro merged 5 commits into from
Dec 25, 2025
Merged

[clickhouse] Update FindTraceIDs to populate start and end timestamps #7770

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
a83df4b
Update SearchTraceIDs Query To Join on trace_id_timestamps
mahadzaryab1 Dec 25, 2025
23105c1
Update Reader To Populate Start and End For TraceIDs
mahadzaryab1 Dec 25, 2025
4daaccd
Update Unit Tests
mahadzaryab1 Dec 25, 2025
578a631
Add Zero Times
mahadzaryab1 Dec 25, 2025
d4c11c1
Format
mahadzaryab1 Dec 25, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion internal/storage/v2/clickhouse/sql/queries.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,17 @@ WHERE
// The query begins with a no-op predicate (`WHERE 1=1`) so that additional
// filters can be appended unconditionally using `AND` without needing to check
// whether this is the first WHERE clause.
const SearchTraceIDs = `SELECT DISTINCT trace_id FROM spans WHERE 1=1`
//
// The query joins with trace_id_timestamps to retrieve the start and end times
// for each trace ID.
const SearchTraceIDs = `
SELECT DISTINCT
s.trace_id,
t.start,
t.end
FROM spans s
LEFT JOIN trace_id_timestamps t ON s.trace_id = t.trace_id
Copy link
Copy Markdown
Collaborator Author

@mahadzaryab1 mahadzaryab1 Dec 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@yurishkuro Thoughts on a LEFT JOIN vs. an INNER JOIN? A LEFT JOIN could return some null timestamps in between updates whereas an INNER JOIN could result in some trace_ids not being returned in between updates.

WHERE 1=1`

const SelectServices = `
SELECT DISTINCT
Expand Down
29 changes: 21 additions & 8 deletions internal/storage/v2/clickhouse/tracestore/reader.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"encoding/hex"
"fmt"
"iter"
"time"

"github.com/ClickHouse/clickhouse-go/v2/lib/driver"
"go.opentelemetry.io/collector/pdata/pcommon"
Expand Down Expand Up @@ -142,19 +143,31 @@ func (*Reader) FindTraces(
}

func readRowIntoTraceID(rows driver.Rows) ([]tracestore.FoundTraceID, error) {
var str string
var traceIDHex string
var start, end time.Time

if err := rows.Scan(&str); err != nil {
if err := rows.Scan(&traceIDHex, &start, &end); err != nil {
return nil, fmt.Errorf("failed to scan row: %w", err)
}

b, err := hex.DecodeString(str)
b, err := hex.DecodeString(traceIDHex)
if err != nil {
return nil, fmt.Errorf("failed to decode trace ID: %w", err)
}

traceID := tracestore.FoundTraceID{
TraceID: pcommon.TraceID(b),
}

if !start.IsZero() {
traceID.Start = start
}
if !end.IsZero() {
traceID.End = end
}

return []tracestore.FoundTraceID{
{TraceID: pcommon.TraceID(b)},
traceID,
}, nil
}

Expand All @@ -167,19 +180,19 @@ func (r *Reader) FindTraceIDs(
args := []any{}

if query.ServiceName != "" {
q += " AND service_name = ?"
q += " AND s.service_name = ?"
args = append(args, query.ServiceName)
}
if query.OperationName != "" {
q += " AND name = ?"
q += " AND s.name = ?"
args = append(args, query.OperationName)
}
if query.DurationMin > 0 {
q += " AND duration >= ?"
q += " AND s.duration >= ?"
args = append(args, query.DurationMin.Nanoseconds())
}
if query.DurationMax > 0 {
q += " AND duration <= ?"
q += " AND s.duration <= ?"
args = append(args, query.DurationMax.Nanoseconds())
}
q += " LIMIT ?"
Expand Down
126 changes: 87 additions & 39 deletions internal/storage/v2/clickhouse/tracestore/reader_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,18 @@ var (
MaxSearchDepth: 1000,
}
testSearchTraceIDsQuery = sql.SearchTraceIDs + " LIMIT ?"
testTraceIDsData = [][]any{
{
traceIDHex1,
now.Add(-1 * time.Hour),
now,
},
{
traceIDHex2,
time.Time{},
time.Time{},
},
}
)

func scanSpanRowFn() func(dest any, src *dbmodel.SpanRow) error {
Expand Down Expand Up @@ -118,22 +130,35 @@ func scanSpanRowFn() func(dest any, src *dbmodel.SpanRow) error {
}
}

func scanTraceIDFn() func(dest any, src string) error {
return func(dest any, src string) error {
func scanTraceIDFn() func(dest any, src []any) error {
return func(dest any, src []any) error {
ptrs, ok := dest.([]any)
if !ok {
return fmt.Errorf("expected []any for dest, got %T", dest)
}
if len(ptrs) != 1 {
return fmt.Errorf("expected 1 destination argument, got %d", len(ptrs))
if len(ptrs) != 3 {
fmt.Println(src)
return fmt.Errorf("expected 3 destination arguments, got %d", len(ptrs))
}

ptr, ok := ptrs[0].(*string)
if !ok {
return fmt.Errorf("expected *string for dest[0], got %T", ptrs[0])
}

*ptr = src
startPtr, ok := ptrs[1].(*time.Time)
if !ok {
return fmt.Errorf("expected *time.Time for dest[1], got %T", ptrs[1])
}

endPtr, ok := ptrs[2].(*time.Time)
if !ok {
return fmt.Errorf("expected *time.Time for dest[2], got %T", ptrs[2])
}

*ptr = src[0].(string)
*startPtr = src[1].(time.Time)
*endPtr = src[2].(time.Time)
return nil
}
}
Expand Down Expand Up @@ -518,15 +543,16 @@ func TestFindTraces(t *testing.T) {
func TestFindTraceIDs(t *testing.T) {
driver := &testDriver{
t: t,
expectedQuery: `SELECT DISTINCT trace_id FROM spans WHERE 1=1 ` +
`AND service_name = ? AND name = ? ` +
`AND duration >= ? AND duration <= ? ` +
`LIMIT ?`,
rows: &testRows[string]{
data: []string{
"00000000000000000000000000000001",
"00000000000000000000000000000002",
},
expectedQuery: `
SELECT DISTINCT
s.trace_id,
t.start,
t.end
FROM spans s
LEFT JOIN trace_id_timestamps t ON s.trace_id = t.trace_id
WHERE 1=1 AND s.service_name = ? AND s.name = ? AND s.duration >= ? AND s.duration <= ? LIMIT ?`,
Copy link
Copy Markdown
Member

@yurishkuro yurishkuro Dec 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How is it that you don't have time bounds on the search? Time range (lookback) is the mandatory query condition

Copy link
Copy Markdown
Collaborator Author

@mahadzaryab1 mahadzaryab1 Dec 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Going to do this in the next PR

rows: &testRows[[]any]{
data: testTraceIDsData,
scanFn: scanTraceIDFn(),
},
}
Expand All @@ -543,6 +569,8 @@ func TestFindTraceIDs(t *testing.T) {
require.Equal(t, []tracestore.FoundTraceID{
{
TraceID: pcommon.TraceID([16]byte{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1}),
Start: now.Add(-1 * time.Hour),
End: now,
},
{
TraceID: pcommon.TraceID([16]byte{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 2}),
Expand All @@ -554,10 +582,18 @@ func TestFindTraceIDs_SearchDepthExceedsMax(t *testing.T) {
driver := &testDriver{
t: t,
expectedQuery: testSearchTraceIDsQuery,
rows: &testRows[string]{
data: []string{
"00000000000000000000000000000001",
"00000000000000000000000000000002",
rows: &testRows[[]any]{
data: [][]any{
{
"00000000000000000000000000000001",
time.Now().Add(-1 * time.Hour),
time.Now().Add(-1 * time.Minute),
},
{
"00000000000000000000000000000002",
time.Now().Add(-2 * time.Hour),
time.Now().Add(-2 * time.Minute),
},
},
scanFn: scanTraceIDFn(),
},
Expand All @@ -574,11 +610,8 @@ func TestFindTraceIDs_YieldFalseOnSuccessStopsIteration(t *testing.T) {
conn := &testDriver{
t: t,
expectedQuery: testSearchTraceIDsQuery,
rows: &testRows[string]{
data: []string{
"00000000000000000000000000000001",
"00000000000000000000000000000002",
},
rows: &testRows[[]any]{
data: testTraceIDsData,
scanFn: scanTraceIDFn(),
},
}
Expand All @@ -597,14 +630,16 @@ func TestFindTraceIDs_YieldFalseOnSuccessStopsIteration(t *testing.T) {
require.Equal(t, []tracestore.FoundTraceID{
{
TraceID: pcommon.TraceID([16]byte{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1}),
Start: now.Add(-1 * time.Hour),
End: now,
},
}, gotTraceIDs)
}

func TestFindTraceIDs_ScanErrorContinues(t *testing.T) {
scanCalled := 0

scanFn := func(dest any, src string) error {
scanFn := func(dest any, src []any) error {
scanCalled++
if scanCalled == 1 {
return assert.AnError // simulate scan error on the first row
Expand All @@ -615,11 +650,8 @@ func TestFindTraceIDs_ScanErrorContinues(t *testing.T) {
conn := &testDriver{
t: t,
expectedQuery: testSearchTraceIDsQuery,
rows: &testRows[string]{
data: []string{
"00000000000000000000000000000001",
"00000000000000000000000000000002",
},
rows: &testRows[[]any]{
data: testTraceIDsData,
scanFn: scanFn,
},
}
Expand All @@ -646,12 +678,20 @@ func TestFindTraceIDs_DecodeErrorContinues(t *testing.T) {
conn := &testDriver{
t: t,
expectedQuery: testSearchTraceIDsQuery,
rows: &testRows[string]{
data: []string{
"00000000000000000000000000000001",
"0x",
"invalid",
"00000000000000000000000000000002",
rows: &testRows[[]any]{
data: [][]any{
testTraceIDsData[0],
{
"0x",
time.Now().Add(-2 * time.Hour),
time.Now().Add(-2 * time.Minute),
},
{
"invalid",
time.Now().Add(-3 * time.Hour),
time.Now().Add(-3 * time.Minute),
},
testTraceIDsData[1],
},
scanFn: scanTraceIDFn(),
},
Expand All @@ -663,6 +703,8 @@ func TestFindTraceIDs_DecodeErrorContinues(t *testing.T) {
expectedValidTraceIDs := []tracestore.FoundTraceID{
{
TraceID: pcommon.TraceID([16]byte{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1}),
Start: now.Add(-1 * time.Hour),
End: now,
},
{
TraceID: pcommon.TraceID([16]byte{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 2}),
Expand Down Expand Up @@ -705,8 +747,8 @@ func TestFindTraceIDs_ErrorCases(t *testing.T) {
driver: &testDriver{
t: t,
expectedQuery: testSearchTraceIDsQuery,
rows: &testRows[string]{
data: []string{"0000000000000001", "0000000000000002"},
rows: &testRows[[]any]{
data: testTraceIDsData,
scanErr: assert.AnError,
},
},
Expand All @@ -717,8 +759,14 @@ func TestFindTraceIDs_ErrorCases(t *testing.T) {
driver: &testDriver{
t: t,
expectedQuery: testSearchTraceIDsQuery,
rows: &testRows[string]{
data: []string{"0x"},
rows: &testRows[[]any]{
data: [][]any{
{
"0x",
time.Now().Add(-1 * time.Hour),
time.Now().Add(-1 * time.Minute),
},
},
scanFn: scanTraceIDFn(),
},
},
Expand Down
5 changes: 5 additions & 0 deletions internal/storage/v2/clickhouse/tracestore/spans_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,11 @@ import (

var traceID = pcommon.TraceID([16]byte{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1})

var (
traceIDHex1 = "00000000000000000000000000000001"
traceIDHex2 = "00000000000000000000000000000002"
)

var now = time.Date(2025, 6, 14, 10, 0, 0, 0, time.UTC)

var singleSpan = []*dbmodel.SpanRow{
Expand Down
Loading