Skip to content

Commit 8f5ad5c

Browse files
authored
Merge pull request #784 from 44YHC/fixes
#752, #753 Fixes
2 parents 3b1aa23 + d6ddc13 commit 8f5ad5c

6 files changed

Lines changed: 101 additions & 4 deletions

File tree

reader/model/responses.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ type TraceInfo struct {
3838
RootServiceName string `json:"rootServiceName"`
3939
RootTraceName string `json:"rootTraceName"`
4040
StartTimeUnixNano string `json:"startTimeUnixNano"`
41-
DurationMs float64 `json:"durationMs"`
41+
DurationMs int64 `json:"durationMs"`
4242
SpanSet SpanSet `json:"spanSet"`
4343
SpanSets []SpanSet `json:"spanSets"`
4444
}

reader/traceql/traceql_transpiler/clickhouse_transpiler/planner_test.go

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package clickhouse_transpiler
22

33
import (
44
"fmt"
5+
"strings"
56
"testing"
67
"time"
78

@@ -44,6 +45,47 @@ func TestPlanner(t *testing.T) {
4445
fmt.Println(res)
4546
}
4647

48+
// TestTracesDataDurationMsSQL verifies that the generated SQL uses intDiv (integer
49+
// division) for _duration_ms rather than toFloat64, which would produce fractional
50+
// values that Grafana's Tempo datasource cannot unmarshal into uint32 (issue #782).
51+
func TestTracesDataDurationMsSQL(t *testing.T) {
52+
script, err := traceql_parser.Parse(`{.service.name="test"}`)
53+
if err != nil {
54+
t.Fatal(err)
55+
}
56+
plan, err := Plan(script)
57+
if err != nil {
58+
t.Fatal(err)
59+
}
60+
req, err := plan.Process(&shared.PlannerContext{
61+
IsCluster: false,
62+
From: time.Now().Add(time.Hour * -1),
63+
To: time.Now(),
64+
Limit: 10,
65+
TracesAttrsTable: "tempo_traces_attrs_gin",
66+
TracesAttrsDistTable: "tempo_traces_attrs_gin_dist",
67+
TracesTable: "tempo_traces",
68+
TracesDistTable: "tempo_traces_dist",
69+
VersionInfo: map[string]int64{},
70+
})
71+
if err != nil {
72+
t.Fatal(err)
73+
}
74+
query, err := req.String(&sql.Ctx{
75+
Params: map[string]sql.SQLObject{},
76+
Result: map[string]sql.SQLObject{},
77+
})
78+
if err != nil {
79+
t.Fatal(err)
80+
}
81+
if strings.Contains(query, "toFloat64") {
82+
t.Errorf("generated SQL must not contain toFloat64 for _duration_ms; got:\n%s", query)
83+
}
84+
if !strings.Contains(query, "intDiv") {
85+
t.Errorf("generated SQL must use intDiv for _duration_ms; got:\n%s", query)
86+
}
87+
}
88+
4789
func TestComplexPlanner(t *testing.T) {
4890
script, err := traceql_parser.Parse(`{.randomContainer=~"admiring" && .randomFloat > 10} | count() > 2 || {.randomContainer=~"boring" && .randomFloat < 10}`)
4991
if err != nil {

reader/traceql/traceql_transpiler/clickhouse_transpiler/traces_data.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ func (t *TracesDataPlanner) Process(ctx *shared.PlannerContext) (sql.ISelect, er
4646
Select(
4747
sql.NewSimpleCol("traces.trace_id", "trace_id"),
4848
sql.NewSimpleCol("min(traces.timestamp_ns)", "_start_time_unix_nano"),
49-
sql.NewSimpleCol("toFloat64(max(traces.timestamp_ns + traces.duration_ns) - min(traces.timestamp_ns)) / 1000000", "_duration_ms"),
49+
sql.NewSimpleCol("intDiv(max(traces.timestamp_ns + traces.duration_ns) - min(traces.timestamp_ns), 1000000)", "_duration_ms"),
5050
sql.NewSimpleCol("argMin(traces.service_name, traces.timestamp_ns)", "_root_service_name"),
5151
sql.NewSimpleCol("argMin(traces.name, traces.timestamp_ns)", "_root_trace_name")).
5252
From(sql.NewSimpleCol(ctx.TracesTable, "traces")).

reader/traceql/traceql_transpiler/reqest_processor.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ func (t TraceQLRequestProcessor) Process(ctx *shared.PlannerContext) (chan []mod
4545
durationsNs []int64
4646
timestampsNs []int64
4747
startTimeUnixNano int64
48-
traceDurationMs float64
48+
traceDurationMs int64
4949
rootServiceName string
5050
rootTraceName string
5151
)

writer/utils/unmarshal/metrics_protobuf.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ func (l *promMetricsProtoDec) Decode() error {
4343
points++
4444
if points >= flushLimit {
4545
err := l.onEntries(oLblsBuf, tsns, msg, value,
46-
fastFillArray[uint8](len(ts.GetSamples()), model.SAMPLE_TYPE_METRIC))
46+
fastFillArray[uint8](len(tsns), model.SAMPLE_TYPE_METRIC))
4747
if err != nil {
4848
return err
4949
}
Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
package unmarshal
2+
3+
import (
4+
"fmt"
5+
"testing"
6+
7+
"github.com/metrico/qryn/v4/writer/utils/proto/prompb"
8+
)
9+
10+
// TestPromMetricsFlushLimitTypesLength verifies that when a time series has more
11+
// samples than flushLimit (1000), the types slice passed to onEntries always has
12+
// the same length as the timestampsNS slice (not the total sample count).
13+
//
14+
// This is a regression test for: https://github.com/metrico/gigapipe/issues/783
15+
func TestPromMetricsFlushLimitTypesLength(t *testing.T) {
16+
const totalSamples = 1001 // one more than flushLimit to trigger the mid-flush
17+
18+
samples := make([]*prompb.Sample, totalSamples)
19+
for i := range samples {
20+
samples[i] = &prompb.Sample{Timestamp: int64(i), Value: float64(i)}
21+
}
22+
23+
req := &prompb.WriteRequest{
24+
Timeseries: []*prompb.TimeSeries{
25+
{
26+
Labels: []*prompb.Label{{Name: "__name__", Value: "test_metric"}},
27+
Samples: samples,
28+
},
29+
},
30+
}
31+
32+
dec := &promMetricsProtoDec{
33+
ctx: &ParserCtx{bodyObject: req},
34+
}
35+
36+
var mismatches []string
37+
dec.SetOnEntries(func(labels [][]string, timestampsNS []int64, message []string, value []float64, types []uint8) error {
38+
if len(types) != len(timestampsNS) {
39+
mismatches = append(mismatches, fmt.Sprintf(
40+
"types length %d != timestampsNS length %d", len(types), len(timestampsNS),
41+
))
42+
}
43+
return nil
44+
})
45+
46+
if err := dec.Decode(); err != nil {
47+
t.Fatalf("Decode returned error: %v", err)
48+
}
49+
50+
if len(mismatches) > 0 {
51+
for _, m := range mismatches {
52+
t.Errorf("column length mismatch: %s", m)
53+
}
54+
}
55+
}

0 commit comments

Comments
 (0)