Skip to content

Commit 556f433

Browse files
qiujian16claude
andcommitted
Add log tracing support for contextual logging
This commit adds comprehensive log tracing support across the cloudevents subsystem and base controller. Key changes include: - Add logging package with tracing annotation utilities - Transfer log tracing annotations between CloudEvents and ManifestWork objects - Enhance base controller with improved logging context - Add comprehensive test coverage for log tracing functionality - Update base controller default queue key to be more descriptive The log tracing feature enables better observability by propagating contextual logging metadata through the CloudEvents flow. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> Signed-off-by: Jian Qiu <[email protected]>
1 parent a185f88 commit 556f433

File tree

11 files changed

+1572
-12
lines changed

11 files changed

+1572
-12
lines changed

pkg/basecontroller/factory/base_controller.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ func (c *baseController) Run(ctx context.Context, workers int) {
8080
queueContext, queueContextCancel := context.WithCancel(ctx)
8181

8282
for i := 1; i <= workers; i++ {
83-
logger.Info("Starting worker of controller ...", "numberOfWorkers", i)
83+
logger.Info("Starting worker of controller ...", "worker-ID", i)
8484
workerWg.Add(1)
8585
go func() {
8686
defer func() {
@@ -157,10 +157,10 @@ func (c *baseController) processNextWorkItem(queueCtx context.Context) {
157157
queueKey := key
158158

159159
if err := c.sync(queueCtx, syncCtx, queueKey); err != nil {
160-
if logger.V(4).Enabled() || key != "key" {
161-
utilruntime.HandleErrorWithContext(queueCtx, err, "controller failed to sync", "key", key, "error", err)
160+
if logger.V(4).Enabled() || key != DefaultQueueKey {
161+
utilruntime.HandleErrorWithContext(queueCtx, err, "controller failed to sync", "key", key)
162162
} else {
163-
utilruntime.HandleErrorWithContext(queueCtx, err, "reconciliation failed", "error", err)
163+
utilruntime.HandleErrorWithContext(queueCtx, err, "reconciliation failed")
164164
}
165165
c.syncContext.Queue().AddRateLimited(key)
166166
return
Lines changed: 363 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,363 @@
1+
package factory
2+
3+
import (
4+
"context"
5+
"errors"
6+
"sync"
7+
"testing"
8+
"time"
9+
10+
"k8s.io/apimachinery/pkg/runtime"
11+
"k8s.io/client-go/tools/cache"
12+
"k8s.io/klog/v2"
13+
)
14+
15+
// mockInformer is a mock implementation of Informer for testing
16+
type mockInformer struct {
17+
handlers []cache.ResourceEventHandler
18+
synced bool
19+
}
20+
21+
func (m *mockInformer) AddEventHandler(handler cache.ResourceEventHandler) (cache.ResourceEventHandlerRegistration, error) {
22+
m.handlers = append(m.handlers, handler)
23+
return nil, nil
24+
}
25+
26+
func (m *mockInformer) HasSynced() bool {
27+
return m.synced
28+
}
29+
30+
// TestBaseControllerRun tests the basic Run functionality of baseController
31+
func TestBaseControllerRun(t *testing.T) {
32+
tests := []struct {
33+
name string
34+
workers int
35+
cacheSynced bool
36+
expectSync bool
37+
resyncInterval time.Duration
38+
addToQueue bool
39+
}{
40+
{
41+
name: "controller runs with synced caches",
42+
workers: 1,
43+
cacheSynced: true,
44+
expectSync: true,
45+
addToQueue: true,
46+
},
47+
{
48+
name: "controller runs with multiple workers",
49+
workers: 3,
50+
cacheSynced: true,
51+
expectSync: true,
52+
addToQueue: true,
53+
},
54+
{
55+
name: "controller with periodic resync",
56+
workers: 1,
57+
cacheSynced: true,
58+
expectSync: true,
59+
resyncInterval: 100 * time.Millisecond,
60+
},
61+
}
62+
63+
for _, tt := range tests {
64+
t.Run(tt.name, func(t *testing.T) {
65+
ctx := klog.NewContext(context.Background(), klog.Background())
66+
ctx, cancel := context.WithTimeout(ctx, 2*time.Second)
67+
defer cancel()
68+
69+
syncCalled := &sync.WaitGroup{}
70+
if tt.expectSync {
71+
syncCalled.Add(1)
72+
}
73+
74+
syncFunc := func(ctx context.Context, syncCtx SyncContext, key string) error {
75+
syncCalled.Done()
76+
return nil
77+
}
78+
79+
mockInf := &mockInformer{synced: tt.cacheSynced}
80+
controller := &baseController{
81+
name: "test-controller",
82+
sync: syncFunc,
83+
syncContext: NewSyncContext("test"),
84+
resyncEvery: tt.resyncInterval,
85+
cachesToSync: []cache.InformerSynced{mockInf.HasSynced},
86+
cacheSyncTimeout: 1 * time.Second,
87+
}
88+
89+
// Add item to queue if needed
90+
if tt.addToQueue {
91+
controller.syncContext.Queue().Add("test-key")
92+
}
93+
94+
// Run controller in background
95+
go controller.Run(ctx, tt.workers)
96+
97+
// Wait for sync to be called or timeout
98+
done := make(chan struct{})
99+
go func() {
100+
syncCalled.Wait()
101+
close(done)
102+
}()
103+
104+
select {
105+
case <-done:
106+
// Success - sync was called
107+
case <-time.After(1500 * time.Millisecond):
108+
if tt.expectSync {
109+
t.Error("timeout waiting for sync to be called")
110+
}
111+
}
112+
})
113+
}
114+
}
115+
116+
// TestBaseControllerProcessNextWorkItem tests the processNextWorkItem function
117+
func TestBaseControllerProcessNextWorkItem(t *testing.T) {
118+
tests := []struct {
119+
name string
120+
queueKey string
121+
syncError error
122+
expectRequeue bool
123+
}{
124+
{
125+
name: "successful sync with default queue key",
126+
queueKey: DefaultQueueKey,
127+
syncError: nil,
128+
expectRequeue: false,
129+
},
130+
{
131+
name: "successful sync with custom queue key",
132+
queueKey: "custom-key",
133+
syncError: nil,
134+
expectRequeue: false,
135+
},
136+
{
137+
name: "failed sync with default queue key",
138+
queueKey: DefaultQueueKey,
139+
syncError: errors.New("sync failed"),
140+
expectRequeue: true,
141+
},
142+
{
143+
name: "failed sync with custom queue key",
144+
queueKey: "namespace/name",
145+
syncError: errors.New("sync failed"),
146+
expectRequeue: true,
147+
},
148+
}
149+
150+
for _, tt := range tests {
151+
t.Run(tt.name, func(t *testing.T) {
152+
ctx := klog.NewContext(context.Background(), klog.Background())
153+
154+
syncCalled := false
155+
syncFunc := func(ctx context.Context, syncCtx SyncContext, key string) error {
156+
syncCalled = true
157+
if key != tt.queueKey {
158+
t.Errorf("expected key %s, got %s", tt.queueKey, key)
159+
}
160+
return tt.syncError
161+
}
162+
163+
controller := &baseController{
164+
name: "test-controller",
165+
sync: syncFunc,
166+
syncContext: NewSyncContext("test"),
167+
}
168+
169+
// Add item to queue
170+
controller.syncContext.Queue().Add(tt.queueKey)
171+
172+
// Process the item
173+
controller.processNextWorkItem(ctx)
174+
175+
if !syncCalled {
176+
t.Error("sync function was not called")
177+
}
178+
179+
// For rate limited items, we need to wait a bit
180+
if tt.expectRequeue {
181+
time.Sleep(10 * time.Millisecond)
182+
}
183+
184+
// Check if item was requeued on error
185+
queueLen := controller.syncContext.Queue().Len()
186+
if tt.expectRequeue && queueLen == 0 {
187+
t.Error("expected item to be requeued but queue is empty")
188+
}
189+
if !tt.expectRequeue && queueLen > 0 {
190+
t.Errorf("expected queue to be empty but has %d items", queueLen)
191+
}
192+
193+
// Cleanup
194+
controller.syncContext.Queue().ShutDown()
195+
})
196+
}
197+
}
198+
199+
// TestBaseControllerDefaultQueueKey tests the error logging behavior with DefaultQueueKey
200+
func TestBaseControllerDefaultQueueKey(t *testing.T) {
201+
ctx := klog.NewContext(context.Background(), klog.Background())
202+
203+
syncError := errors.New("test error")
204+
syncFunc := func(ctx context.Context, syncCtx SyncContext, key string) error {
205+
return syncError
206+
}
207+
208+
controller := &baseController{
209+
name: "test-controller",
210+
sync: syncFunc,
211+
syncContext: NewSyncContext("test"),
212+
}
213+
214+
// Test with DefaultQueueKey
215+
controller.syncContext.Queue().Add(DefaultQueueKey)
216+
controller.processNextWorkItem(ctx)
217+
218+
// Wait for rate limited item
219+
time.Sleep(10 * time.Millisecond)
220+
221+
// Verify item was requeued
222+
if controller.syncContext.Queue().Len() == 0 {
223+
t.Error("expected item to be requeued")
224+
}
225+
226+
// Test with custom key
227+
controller.syncContext.Queue().Add("custom/key")
228+
controller.processNextWorkItem(ctx)
229+
230+
// Wait for rate limited item
231+
time.Sleep(10 * time.Millisecond)
232+
233+
// Verify item was requeued
234+
if controller.syncContext.Queue().Len() == 0 {
235+
t.Error("expected item to be requeued")
236+
}
237+
238+
// Cleanup
239+
controller.syncContext.Queue().ShutDown()
240+
}
241+
242+
// TestBaseControllerName tests the Name() method
243+
func TestBaseControllerName(t *testing.T) {
244+
controller := &baseController{
245+
name: "test-controller-name",
246+
}
247+
248+
if controller.Name() != "test-controller-name" {
249+
t.Errorf("expected name 'test-controller-name', got '%s'", controller.Name())
250+
}
251+
}
252+
253+
// TestBaseControllerSyncContext tests the SyncContext() method
254+
func TestBaseControllerSyncContext(t *testing.T) {
255+
syncCtx := NewSyncContext("test")
256+
controller := &baseController{
257+
syncContext: syncCtx,
258+
}
259+
260+
if controller.SyncContext() != syncCtx {
261+
t.Error("SyncContext() returned different context than expected")
262+
}
263+
}
264+
265+
// TestBaseControllerSync tests the Sync() method
266+
func TestBaseControllerSync(t *testing.T) {
267+
ctx := klog.NewContext(context.Background(), klog.Background())
268+
269+
syncCalled := false
270+
expectedKey := "test-key"
271+
syncFunc := func(ctx context.Context, syncCtx SyncContext, key string) error {
272+
syncCalled = true
273+
if key != expectedKey {
274+
t.Errorf("expected key %s, got %s", expectedKey, key)
275+
}
276+
return nil
277+
}
278+
279+
syncCtx := NewSyncContext("test")
280+
controller := &baseController{
281+
sync: syncFunc,
282+
syncContext: syncCtx,
283+
}
284+
285+
err := controller.Sync(ctx, syncCtx, expectedKey)
286+
if err != nil {
287+
t.Errorf("unexpected error: %v", err)
288+
}
289+
if !syncCalled {
290+
t.Error("sync function was not called")
291+
}
292+
}
293+
294+
// TestBaseControllerRunPeriodicalResync tests periodic resync functionality
295+
func TestBaseControllerRunPeriodicalResync(t *testing.T) {
296+
ctx := klog.NewContext(context.Background(), klog.Background())
297+
ctx, cancel := context.WithTimeout(ctx, 500*time.Millisecond)
298+
defer cancel()
299+
300+
syncCtx := NewSyncContext("test")
301+
controller := &baseController{
302+
syncContext: syncCtx,
303+
resyncEvery: 50 * time.Millisecond,
304+
}
305+
306+
// Run periodic resync
307+
go controller.runPeriodicalResync(ctx, controller.resyncEvery)
308+
309+
// Wait for items to be added to queue
310+
time.Sleep(400 * time.Millisecond)
311+
312+
// Check that queue has items (should have at least 1-2 items added due to timing variations)
313+
queueLen := controller.syncContext.Queue().Len()
314+
if queueLen < 1 {
315+
t.Errorf("expected at least 1 item in queue from periodic resync, got %d", queueLen)
316+
}
317+
318+
// Verify the items are DefaultQueueKey
319+
for i := 0; i < queueLen; i++ {
320+
item, _ := controller.syncContext.Queue().Get()
321+
if item != DefaultQueueKey {
322+
t.Errorf("expected DefaultQueueKey, got %v", item)
323+
}
324+
controller.syncContext.Queue().Done(item)
325+
}
326+
327+
// Cleanup
328+
controller.syncContext.Queue().ShutDown()
329+
}
330+
331+
// TestDefaultQueueKeysFunc tests the DefaultQueueKeysFunc function
332+
func TestDefaultQueueKeysFunc(t *testing.T) {
333+
tests := []struct {
334+
name string
335+
obj runtime.Object
336+
expect []string
337+
}{
338+
{
339+
name: "nil object",
340+
obj: nil,
341+
expect: []string{DefaultQueueKey},
342+
},
343+
{
344+
name: "non-nil object",
345+
obj: &runtime.Unknown{},
346+
expect: []string{DefaultQueueKey},
347+
},
348+
}
349+
350+
for _, tt := range tests {
351+
t.Run(tt.name, func(t *testing.T) {
352+
result := DefaultQueueKeysFunc(tt.obj)
353+
if len(result) != len(tt.expect) {
354+
t.Errorf("expected %d keys, got %d", len(tt.expect), len(result))
355+
}
356+
for i, key := range result {
357+
if key != tt.expect[i] {
358+
t.Errorf("expected key %s at index %d, got %s", tt.expect[i], i, key)
359+
}
360+
}
361+
})
362+
}
363+
}

pkg/basecontroller/factory/factory.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import (
1010
)
1111

1212
// DefaultQueueKey is the queue key used for string trigger based controllers.
13-
const DefaultQueueKey = "key"
13+
const DefaultQueueKey = "basecontroller-default-key"
1414

1515
// DefaultQueueKeysFunc returns a slice with a single element - the DefaultQueueKey
1616
func DefaultQueueKeysFunc(_ runtime.Object) []string {

0 commit comments

Comments
 (0)