Skip to content

Commit 4c08b8a

Browse files
mostafamostafa
authored
Handle hclog format string properly (#689)
* Handle hclog Format string properly * Remove unused function * Simplify function * Assert type * Clean up tests * Fix race condition with global logger * Use no-op logger to avoid cluttering the output
1 parent 9416df5 commit 4c08b8a

File tree

11 files changed

+82
-104
lines changed

11 files changed

+82
-104
lines changed

‎act/act_helpers_test.go‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
package act
22

33
import (
4+
"io"
5+
"log"
46
"testing"
57
"time"
68

@@ -72,6 +74,7 @@ func createTestRedis(t *testing.T) string {
7274
),
7375
},
7476
Started: true,
77+
Logger: log.New(io.Discard, "", 0),
7578
},
7679
)
7780
assert.NoError(t, err)

‎act/builtins_test.go‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ func Test_Terminate_Action(t *testing.T) {
4747
params: []sdkAct.Parameter{
4848
{
4949
Key: LoggerKey,
50-
Value: zerolog.New(nil),
50+
Value: zerolog.Nop(),
5151
},
5252
},
5353
result: true,
@@ -56,7 +56,7 @@ func Test_Terminate_Action(t *testing.T) {
5656
params: []sdkAct.Parameter{
5757
{
5858
Key: LoggerKey,
59-
Value: zerolog.New(nil),
59+
Value: zerolog.Nop(),
6060
},
6161
{
6262
Key: ResultKey,
@@ -69,7 +69,7 @@ func Test_Terminate_Action(t *testing.T) {
6969
params: []sdkAct.Parameter{
7070
{
7171
Key: LoggerKey,
72-
Value: zerolog.New(nil),
72+
Value: zerolog.Nop(),
7373
},
7474
{
7575
Key: ResultKey,
@@ -114,7 +114,7 @@ func Test_Log_Action(t *testing.T) {
114114
params: []sdkAct.Parameter{
115115
{
116116
Key: LoggerKey,
117-
Value: zerolog.New(nil),
117+
Value: zerolog.Nop(),
118118
},
119119
},
120120
result: true,

‎act/registry_test.go‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ import (
2020

2121
// Test_NewRegistry tests the NewRegistry function.
2222
func Test_NewRegistry(t *testing.T) {
23-
logger := zerolog.Logger{}
23+
logger := zerolog.Nop()
2424

2525
actRegistry := NewActRegistry(
2626
Registry{
@@ -129,7 +129,7 @@ func Test_Add(t *testing.T) {
129129
DefaultPolicyName: config.DefaultPolicy,
130130
PolicyTimeout: config.DefaultPolicyTimeout,
131131
DefaultActionTimeout: config.DefaultActionTimeout,
132-
Logger: zerolog.Logger{},
132+
Logger: zerolog.Nop(),
133133
})
134134
assert.NotNil(t, actRegistry)
135135

@@ -196,7 +196,7 @@ func Test_Apply(t *testing.T) {
196196
DefaultPolicyName: config.DefaultPolicy,
197197
PolicyTimeout: config.DefaultPolicyTimeout,
198198
DefaultActionTimeout: config.DefaultActionTimeout,
199-
Logger: zerolog.Logger{},
199+
Logger: zerolog.Nop(),
200200
})
201201
assert.NotNil(t, actRegistry)
202202

@@ -593,7 +593,7 @@ func Test_Apply_Hook(t *testing.T) {
593593

594594
// Test_Run tests the Run function of the act registry with a non-terminal action.
595595
func Test_Run(t *testing.T) {
596-
logger := zerolog.Logger{}
596+
logger := zerolog.Nop()
597597
actRegistry := NewActRegistry(
598598
Registry{
599599
Signals: BuiltinSignals(),
@@ -623,7 +623,7 @@ func Test_Run(t *testing.T) {
623623

624624
// Test_Run_Terminate tests the Run function of the act registry with a terminal action.
625625
func Test_Run_Terminate(t *testing.T) {
626-
logger := zerolog.Logger{}
626+
logger := zerolog.Nop()
627627
actRegistry := NewActRegistry(
628628
Registry{
629629
Signals: BuiltinSignals(),

‎api/api_helpers_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import (
1313

1414
// getAPIConfig returns a new API configuration with all the necessary components.
1515
func getAPIConfig(httpAddr, grpcAddr string) *API {
16-
logger := zerolog.New(nil)
16+
logger := zerolog.Nop()
1717
defaultPool := pool.NewPool(context.Background(), config.DefaultPoolSize)
1818
pluginReg := plugin.NewRegistry(
1919
context.Background(),

‎api/api_test.go‎

Lines changed: 24 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
package api
22

33
import (
4-
"io"
54
"regexp"
65
"testing"
76
"time"
@@ -137,13 +136,13 @@ func TestGetPlugins(t *testing.T) {
137136
DefaultPolicyName: config.DefaultPolicy,
138137
PolicyTimeout: config.DefaultPolicyTimeout,
139138
DefaultActionTimeout: config.DefaultActionTimeout,
140-
Logger: zerolog.Logger{},
139+
Logger: zerolog.Nop(),
141140
})
142141
pluginRegistry := plugin.NewRegistry(
143142
t.Context(),
144143
plugin.Registry{
145144
ActRegistry: actRegistry,
146-
Logger: zerolog.Logger{},
145+
Logger: zerolog.Nop(),
147146
DevMode: true,
148147
},
149148
)
@@ -191,13 +190,13 @@ func TestGetPluginsWithEmptyPluginRegistry(t *testing.T) {
191190
DefaultPolicyName: config.DefaultPolicy,
192191
PolicyTimeout: config.DefaultPolicyTimeout,
193192
DefaultActionTimeout: config.DefaultActionTimeout,
194-
Logger: zerolog.Logger{},
193+
Logger: zerolog.Nop(),
195194
})
196195
pluginRegistry := plugin.NewRegistry(
197196
t.Context(),
198197
plugin.Registry{
199198
ActRegistry: actRegistry,
200-
Logger: zerolog.Logger{},
199+
Logger: zerolog.Nop(),
201200
DevMode: true,
202201
},
203202
)
@@ -250,7 +249,7 @@ func TestGetProxies(t *testing.T) {
250249
Network: config.DefaultNetwork,
251250
Address: postgresAddress,
252251
}
253-
client := network.NewClient(t.Context(), clientConfig, zerolog.Logger{}, nil)
252+
client := network.NewClient(t.Context(), clientConfig, zerolog.Nop(), nil)
254253
require.NotNil(t, client)
255254
newPool := pool.NewPool(t.Context(), 1)
256255
assert.Nil(t, newPool.Put(client.ID, client))
@@ -264,7 +263,7 @@ func TestGetProxies(t *testing.T) {
264263
Network: config.DefaultNetwork,
265264
Address: postgresAddress,
266265
},
267-
Logger: zerolog.Logger{},
266+
Logger: zerolog.Nop(),
268267
PluginTimeout: config.DefaultPluginTimeout,
269268
},
270269
)
@@ -302,7 +301,7 @@ func TestGetServers(t *testing.T) {
302301
Network: config.DefaultNetwork,
303302
Address: postgresAddress,
304303
}
305-
client := network.NewClient(t.Context(), clientConfig, zerolog.Logger{}, nil)
304+
client := network.NewClient(t.Context(), clientConfig, zerolog.Nop(), nil)
306305
newPool := pool.NewPool(t.Context(), 1)
307306
require.NotNil(t, newPool)
308307
assert.Nil(t, newPool.Put(client.ID, client))
@@ -316,7 +315,7 @@ func TestGetServers(t *testing.T) {
316315
Network: config.DefaultNetwork,
317316
Address: postgresAddress,
318317
},
319-
Logger: zerolog.Logger{},
318+
Logger: zerolog.Nop(),
320319
PluginTimeout: config.DefaultPluginTimeout,
321320
},
322321
)
@@ -329,14 +328,14 @@ func TestGetServers(t *testing.T) {
329328
DefaultPolicyName: config.DefaultPolicy,
330329
PolicyTimeout: config.DefaultPolicyTimeout,
331330
DefaultActionTimeout: config.DefaultActionTimeout,
332-
Logger: zerolog.Logger{},
331+
Logger: zerolog.Nop(),
333332
})
334333

335334
pluginRegistry := plugin.NewRegistry(
336335
t.Context(),
337336
plugin.Registry{
338337
ActRegistry: actRegistry,
339-
Logger: zerolog.Logger{},
338+
Logger: zerolog.Nop(),
340339
DevMode: true,
341340
},
342341
)
@@ -351,7 +350,7 @@ func TestGetServers(t *testing.T) {
351350
EnableTicker: false,
352351
},
353352
Proxies: []network.IProxy{proxy},
354-
Logger: zerolog.Logger{},
353+
Logger: zerolog.Nop(),
355354
PluginRegistry: pluginRegistry,
356355
PluginTimeout: config.DefaultPluginTimeout,
357356
HandshakeTimeout: config.DefaultHandshakeTimeout,
@@ -445,7 +444,7 @@ func TestRemovePeerAPI(t *testing.T) {
445444

446445
// Initialize nodes
447446
for i, cfg := range nodeConfigs {
448-
node, err := raft.NewRaftNode(zerolog.New(io.Discard).With().Timestamp().Logger(), cfg)
447+
node, err := raft.NewRaftNode(zerolog.Nop().With().Timestamp().Logger(), cfg)
449448
require.NoError(t, err)
450449
nodes[i] = node
451450
}
@@ -478,7 +477,7 @@ func TestRemovePeerAPI(t *testing.T) {
478477
api: &API{
479478
ctx: t.Context(),
480479
Options: &Options{
481-
Logger: zerolog.New(io.Discard),
480+
Logger: zerolog.Nop(),
482481
RaftNode: nodes[0],
483482
},
484483
},
@@ -490,7 +489,7 @@ func TestRemovePeerAPI(t *testing.T) {
490489
api: &API{
491490
ctx: t.Context(),
492491
Options: &Options{
493-
Logger: zerolog.New(io.Discard),
492+
Logger: zerolog.Nop(),
494493
RaftNode: nil,
495494
},
496495
},
@@ -503,7 +502,7 @@ func TestRemovePeerAPI(t *testing.T) {
503502
api: &API{
504503
ctx: t.Context(),
505504
Options: &Options{
506-
Logger: zerolog.New(io.Discard),
505+
Logger: zerolog.Nop(),
507506
RaftNode: nodes[0],
508507
},
509508
},
@@ -545,7 +544,7 @@ func TestGetPeers(t *testing.T) {
545544
}
546545

547546
// Initialize node
548-
node, err := raft.NewRaftNode(zerolog.New(io.Discard).With().Timestamp().Logger(), nodeConfig)
547+
node, err := raft.NewRaftNode(zerolog.Nop().With().Timestamp().Logger(), nodeConfig)
549548
require.NoError(t, err)
550549
defer func() {
551550
if node != nil {
@@ -569,7 +568,7 @@ func TestGetPeers(t *testing.T) {
569568
api: &API{
570569
ctx: t.Context(),
571570
Options: &Options{
572-
Logger: zerolog.New(io.Discard),
571+
Logger: zerolog.Nop(),
573572
RaftNode: node,
574573
},
575574
},
@@ -580,7 +579,7 @@ func TestGetPeers(t *testing.T) {
580579
api: &API{
581580
ctx: t.Context(),
582581
Options: &Options{
583-
Logger: zerolog.New(io.Discard),
582+
Logger: zerolog.Nop(),
584583
RaftNode: nil,
585584
},
586585
},
@@ -634,7 +633,7 @@ func TestAddPeer(t *testing.T) {
634633
}
635634

636635
// Initialize node
637-
node, err := raft.NewRaftNode(zerolog.New(io.Discard).With().Timestamp().Logger(), nodeConfig)
636+
node, err := raft.NewRaftNode(zerolog.Nop().With().Timestamp().Logger(), nodeConfig)
638637
require.NoError(t, err)
639638
defer func() {
640639
if node != nil {
@@ -659,7 +658,7 @@ func TestAddPeer(t *testing.T) {
659658
api: &API{
660659
ctx: t.Context(),
661660
Options: &Options{
662-
Logger: zerolog.New(io.Discard),
661+
Logger: zerolog.Nop(),
663662
RaftNode: node,
664663
},
665664
},
@@ -675,7 +674,7 @@ func TestAddPeer(t *testing.T) {
675674
api: &API{
676675
ctx: t.Context(),
677676
Options: &Options{
678-
Logger: zerolog.New(io.Discard),
677+
Logger: zerolog.Nop(),
679678
RaftNode: nil,
680679
},
681680
},
@@ -692,7 +691,7 @@ func TestAddPeer(t *testing.T) {
692691
api: &API{
693692
ctx: t.Context(),
694693
Options: &Options{
695-
Logger: zerolog.New(io.Discard),
694+
Logger: zerolog.Nop(),
696695
RaftNode: node,
697696
},
698697
},
@@ -708,7 +707,7 @@ func TestAddPeer(t *testing.T) {
708707
api: &API{
709708
ctx: t.Context(),
710709
Options: &Options{
711-
Logger: zerolog.New(io.Discard),
710+
Logger: zerolog.Nop(),
712711
RaftNode: node,
713712
},
714713
},
@@ -724,7 +723,7 @@ func TestAddPeer(t *testing.T) {
724723
api: &API{
725724
ctx: t.Context(),
726725
Options: &Options{
727-
Logger: zerolog.New(io.Discard),
726+
Logger: zerolog.Nop(),
728727
RaftNode: node,
729728
},
730729
},

‎api/healthcheck_test.go‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ func Test_Healthchecker(t *testing.T) {
2323
Network: config.DefaultNetwork,
2424
Address: postgresAddress,
2525
}
26-
client := network.NewClient(t.Context(), clientConfig, zerolog.Logger{}, nil)
26+
client := network.NewClient(t.Context(), clientConfig, zerolog.Nop(), nil)
2727
newPool := pool.NewPool(t.Context(), 1)
2828
require.NotNil(t, newPool)
2929
assert.Nil(t, newPool.Put(client.ID, client))
@@ -37,7 +37,7 @@ func Test_Healthchecker(t *testing.T) {
3737
Network: config.DefaultNetwork,
3838
Address: postgresAddress,
3939
},
40-
Logger: zerolog.Logger{},
40+
Logger: zerolog.Nop(),
4141
PluginTimeout: config.DefaultPluginTimeout,
4242
},
4343
)
@@ -50,14 +50,14 @@ func Test_Healthchecker(t *testing.T) {
5050
DefaultPolicyName: config.DefaultPolicy,
5151
PolicyTimeout: config.DefaultPolicyTimeout,
5252
DefaultActionTimeout: config.DefaultActionTimeout,
53-
Logger: zerolog.Logger{},
53+
Logger: zerolog.Nop(),
5454
})
5555

5656
pluginRegistry := plugin.NewRegistry(
5757
t.Context(),
5858
plugin.Registry{
5959
ActRegistry: actRegistry,
60-
Logger: zerolog.Logger{},
60+
Logger: zerolog.Nop(),
6161
DevMode: true,
6262
},
6363
)
@@ -82,7 +82,7 @@ func Test_Healthchecker(t *testing.T) {
8282
EnableTicker: false,
8383
},
8484
Proxies: []network.IProxy{proxy},
85-
Logger: zerolog.Logger{},
85+
Logger: zerolog.Nop(),
8686
PluginRegistry: pluginRegistry,
8787
PluginTimeout: config.DefaultPluginTimeout,
8888
HandshakeTimeout: config.DefaultHandshakeTimeout,

‎logging/hclog_adapter.go‎

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -190,8 +190,22 @@ func ToMap(keyValues []any) map[string]any {
190190
keyValues = append(keyValues, nil)
191191
}
192192

193-
for i := 0; i < len(keyValues); i += 2 {
194-
merge(mapped, keyValues[i], keyValues[i+1])
193+
for idx := 0; idx < len(keyValues); idx += 2 {
194+
key := keyValues[idx]
195+
value := keyValues[idx+1]
196+
197+
if formatted, ok := value.(hclog.Format); ok {
198+
formatStr, ok := formatted[0].(string)
199+
if !ok {
200+
// Default to %+v if the format string is not a string
201+
// This should never happen, but we'll handle it just in case.
202+
formatStr = "%+v"
203+
}
204+
205+
value = fmt.Sprintf(formatStr, formatted[1:]...)
206+
}
207+
208+
merge(mapped, key, value)
195209
}
196210

197211
return mapped

0 commit comments

Comments
 (0)