Skip to content

Commit 62ec449

Browse files
committed
pkg/util/traceevent: stabilize flaky TestFlightRecorder
1 parent ee3439d commit 62ec449

2 files changed

Lines changed: 57 additions & 3 deletions

File tree

pkg/util/traceevent/flightrecorder_test.go

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -333,9 +333,25 @@ func testFlightRecorderConfigBadCase(t *testing.T) {
333333
}
334334
}
335335

336+
func testSamplingCounterDeterministic(t *testing.T) {
337+
recorder := &HTTPFlightRecorder{}
338+
conf := &DumpTriggerConfig{
339+
Type: "sampling",
340+
Sampling: 5,
341+
}
342+
sampledCount := 0
343+
for i := 0; i < 10; i++ {
344+
if recorder.CheckSampling(conf) {
345+
sampledCount++
346+
}
347+
}
348+
require.Equal(t, 2, sampledCount)
349+
}
350+
336351
func TestFlightRecorderConfig(t *testing.T) {
337352
testFlightRecorderConfigGoodCase(t)
338353
testFlightRecorderConfigBadCase(t)
354+
testSamplingCounterDeterministic(t)
339355
}
340356

341357
func TestParseTraceCategory(t *testing.T) {

pkg/util/traceevent/test/integration_test.go

Lines changed: 41 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import (
2424
"regexp"
2525
"strings"
2626
"testing"
27+
"time"
2728

2829
"github.com/pingcap/tidb/pkg/config"
2930
"github.com/pingcap/tidb/pkg/config/kerneltype"
@@ -48,6 +49,38 @@ func drainEvents(eventCh <-chan []traceevent.Event) {
4849
}
4950
}
5051

52+
func eventsBelongToTrace(events []tracing.Event, traceID []byte) bool {
53+
if len(events) == 0 || len(traceID) == 0 {
54+
return false
55+
}
56+
matched := false
57+
for _, event := range events {
58+
if len(event.TraceID) == 0 {
59+
continue
60+
}
61+
if !bytes.Equal(event.TraceID, traceID) {
62+
return false
63+
}
64+
matched = true
65+
}
66+
return matched
67+
}
68+
69+
func waitTraceEvents(t *testing.T, eventCh <-chan []tracing.Event, traceID []byte) []tracing.Event {
70+
t.Helper()
71+
deadline := time.After(3 * time.Second)
72+
for {
73+
select {
74+
case events := <-eventCh:
75+
if eventsBelongToTrace(events, traceID) {
76+
return events
77+
}
78+
case <-deadline:
79+
require.FailNowf(t, "failed to find trace events", "trace_id=%s", hex.EncodeToString(traceID))
80+
}
81+
}
82+
}
83+
5184
func TestPrevTraceIDPersistence(t *testing.T) {
5285
if kerneltype.IsClassic() {
5386
t.Skip("trace events only work for next-gen kernel")
@@ -254,10 +287,12 @@ func TestFlightRecorder(t *testing.T) {
254287
}
255288
flightRecorder, err := traceevent.StartHTTPFlightRecorder(eventCh, &config)
256289
require.NoError(t, err)
290+
drainEvents(eventCh)
257291
tk.MustQueryWithContext(ctx, "select * from t").Check(testkit.Rows())
292+
traceID := bytes.Clone(tk.Session().GetSessionVars().PrevTraceID)
293+
require.NotEmpty(t, traceID)
258294
sink.DiscardOrFlush(ctx)
259-
require.NotEmpty(t, eventCh)
260-
events := <-eventCh
295+
events := waitTraceEvents(t, eventCh, traceID)
261296
for _, event := range events {
262297
require.Equal(t, event.Category, traceevent.KvRequest)
263298
}
@@ -279,7 +314,10 @@ func TestFlightRecorder(t *testing.T) {
279314
tk.MustQueryWithContext(ctx, "select * from t").Check(testkit.Rows())
280315
sink.DiscardOrFlush(ctx)
281316
}
282-
require.Len(t, eventCh, 2)
317+
// The recorder state is process-global in this package, so unrelated traces can
318+
// race into this channel. Keep this integration check as a smoke test and verify
319+
// exact sampling math in unit tests.
320+
require.GreaterOrEqual(t, len(eventCh), 1)
283321
flightRecorder.Close()
284322
drainEvents(eventCh)
285323
}

0 commit comments

Comments
 (0)