Skip to content

Commit d9550d8

Browse files
AlinsRanAlinsRan
authored
chore: unify the logging component (#2584)
1 parent 63c7d11 commit d9550d8

28 files changed

+207
-231
lines changed

‎cmd/root/root.go‎

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,6 @@ import (
3838
"github.com/apache/apisix-ingress-controller/internal/controller/config"
3939
"github.com/apache/apisix-ingress-controller/internal/manager"
4040
"github.com/apache/apisix-ingress-controller/internal/version"
41-
"github.com/api7/gopkg/pkg/log"
4241
)
4342

4443
type GatewayConfigsFlag struct {
@@ -115,16 +114,6 @@ func newAPISIXIngressController() *cobra.Command {
115114
return err
116115
}
117116

118-
l, err := log.NewLogger(
119-
log.WithOutputFile("stderr"),
120-
log.WithLogLevel(cfg.LogLevel),
121-
log.WithSkipFrames(3),
122-
)
123-
if err != nil {
124-
return err
125-
}
126-
log.DefaultLogger = l
127-
128117
// controllers log
129118
core := zapcore.NewCore(
130119
zapcore.NewConsoleEncoder(zap.NewDevelopmentEncoderConfig()),

‎internal/adc/cache/store.go‎

Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,8 @@ import (
2121
"fmt"
2222
"sync"
2323

24-
"github.com/api7/gopkg/pkg/log"
24+
"github.com/go-logr/logr"
2525
"github.com/google/uuid"
26-
"go.uber.org/zap"
2726

2827
adctypes "github.com/apache/apisix-ingress-controller/api/adc"
2928
"github.com/apache/apisix-ingress-controller/internal/controller/label"
@@ -34,12 +33,14 @@ type Store struct {
3433
pluginMetadataMap map[string]adctypes.PluginMetadata
3534

3635
sync.Mutex
36+
log logr.Logger
3737
}
3838

39-
func NewStore() *Store {
39+
func NewStore(log logr.Logger) *Store {
4040
return &Store{
4141
cacheMap: make(map[string]Cache),
4242
pluginMetadataMap: make(map[string]adctypes.PluginMetadata),
43+
log: log,
4344
}
4445
}
4546

@@ -55,7 +56,7 @@ func (s *Store) Insert(name string, resourceTypes []string, resources *adctypes.
5556
s.cacheMap[name] = db
5657
targetCache = s.cacheMap[name]
5758
}
58-
log.Debugw("Inserting resources into cache for", zap.String("name", name))
59+
s.log.V(1).Info("Inserting resources into cache", "name", name, "resourceTypes", resourceTypes, "Labels", Labels)
5960
selector := &KindLabelSelector{
6061
Kind: Labels[label.LabelKind],
6162
Name: Labels[label.LabelName],
@@ -162,41 +163,41 @@ func (s *Store) Delete(name string, resourceTypes []string, Labels map[string]st
162163
case adctypes.TypeService:
163164
services, err := targetCache.ListServices(selector)
164165
if err != nil {
165-
log.Errorw("failed to list services", zap.Error(err))
166+
s.log.Error(err, "failed to list services")
166167
}
167168
for _, service := range services {
168169
if err := targetCache.DeleteService(service); err != nil {
169-
log.Errorw("failed to delete service", zap.Error(err), zap.String("service", service.ID))
170+
s.log.Error(err, "failed to delete service", "service", service.ID)
170171
}
171172
}
172173
case adctypes.TypeSSL:
173174
ssls, err := targetCache.ListSSL(selector)
174175
if err != nil {
175-
log.Errorw("failed to list ssl", zap.Error(err))
176+
s.log.Error(err, "failed to list ssl")
176177
}
177178
for _, ssl := range ssls {
178179
if err := targetCache.DeleteSSL(ssl); err != nil {
179-
log.Errorw("failed to delete ssl", zap.Error(err), zap.String("ssl", ssl.ID))
180+
s.log.Error(err, "failed to delete ssl", "ssl", ssl.ID)
180181
}
181182
}
182183
case adctypes.TypeConsumer:
183184
consumers, err := targetCache.ListConsumers(selector)
184185
if err != nil {
185-
log.Errorw("failed to list consumers", zap.Error(err))
186+
s.log.Error(err, "failed to list consumers")
186187
}
187188
for _, consumer := range consumers {
188189
if err := targetCache.DeleteConsumer(consumer); err != nil {
189-
log.Errorw("failed to delete consumer", zap.Error(err), zap.String("consumer", consumer.Username))
190+
s.log.Error(err, "failed to delete consumer", "consumer", consumer.Username)
190191
}
191192
}
192193
case adctypes.TypeGlobalRule:
193194
globalRules, err := targetCache.ListGlobalRules(selector)
194195
if err != nil {
195-
log.Errorw("failed to list global rules", zap.Error(err))
196+
s.log.Error(err, "failed to list global rules")
196197
}
197198
for _, globalRule := range globalRules {
198199
if err := targetCache.DeleteGlobalRule(globalRule); err != nil {
199-
log.Errorw("failed to delete global rule", zap.Error(err), zap.String("global rule", globalRule.ID))
200+
s.log.Error(err, "failed to delete global rule", "global rule", globalRule.ID)
200201
}
201202
}
202203
case adctypes.TypePluginMetadata:
@@ -229,7 +230,7 @@ func (s *Store) GetResources(name string) (*adctypes.Resources, error) {
229230
}
230231
globalrule = adctypes.GlobalRule(merged)
231232
}
232-
log.Debugw("get resources global rule items", zap.Any("globalRuleItems", globalRuleItems))
233+
s.log.V(1).Info("GetResources fetched global rule items", "items", globalRuleItems, "gobalrule", globalrule)
233234
if meta, ok := s.pluginMetadataMap[name]; ok {
234235
metadata = meta.DeepCopy()
235236
}

‎internal/adc/client/client.go‎

Lines changed: 46 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -26,9 +26,8 @@ import (
2626
"sync"
2727
"time"
2828

29-
"github.com/api7/gopkg/pkg/log"
29+
"github.com/go-logr/logr"
3030
"github.com/pkg/errors"
31-
"go.uber.org/zap"
3231

3332
adctypes "github.com/apache/apisix-ingress-controller/api/adc"
3433
"github.com/apache/apisix-ingress-controller/internal/adc/cache"
@@ -47,22 +46,28 @@ type Client struct {
4746

4847
ConfigManager *common.ConfigManager[types.NamespacedNameKind, adctypes.Config]
4948
ADCDebugProvider *common.ADCDebugProvider
49+
50+
log logr.Logger
5051
}
5152

52-
func New(mode string, timeout time.Duration) (*Client, error) {
53+
func New(log logr.Logger, mode string, timeout time.Duration) (*Client, error) {
5354
serverURL := os.Getenv("ADC_SERVER_URL")
5455
if serverURL == "" {
5556
serverURL = defaultHTTPADCExecutorAddr
5657
}
57-
store := cache.NewStore()
58+
store := cache.NewStore(log)
5859
configManager := common.NewConfigManager[types.NamespacedNameKind, adctypes.Config]()
59-
log.Infow("using HTTP ADC Executor", zap.String("server_url", serverURL))
60+
61+
logger := log.WithName("client")
62+
logger.Info("ADC client initialized", "mode", mode)
63+
6064
return &Client{
6165
Store: store,
62-
executor: NewHTTPADCExecutor(serverURL, timeout),
66+
executor: NewHTTPADCExecutor(log, serverURL, timeout),
6367
BackendMode: mode,
6468
ConfigManager: configManager,
6569
ADCDebugProvider: common.NewADCDebugProvider(store, configManager),
70+
log: logger,
6671
}, nil
6772
}
6873

@@ -80,55 +85,55 @@ type StoreDelta struct {
8085
Applied map[types.NamespacedNameKind]adctypes.Config
8186
}
8287

83-
func (d *Client) applyStoreChanges(args Task, isDelete bool) (StoreDelta, error) {
84-
d.mu.Lock()
85-
defer d.mu.Unlock()
88+
func (c *Client) applyStoreChanges(args Task, isDelete bool) (StoreDelta, error) {
89+
c.mu.Lock()
90+
defer c.mu.Unlock()
8691

8792
var delta StoreDelta
8893

8994
if isDelete {
90-
delta.Deleted = d.ConfigManager.Get(args.Key)
91-
d.ConfigManager.Delete(args.Key)
95+
delta.Deleted = c.ConfigManager.Get(args.Key)
96+
c.ConfigManager.Delete(args.Key)
9297
} else {
93-
deleted := d.ConfigManager.Update(args.Key, args.Configs)
98+
deleted := c.ConfigManager.Update(args.Key, args.Configs)
9499
delta.Deleted = deleted
95100
delta.Applied = args.Configs
96101
}
97102

98103
for _, cfg := range delta.Deleted {
99-
if err := d.Store.Delete(cfg.Name, args.ResourceTypes, args.Labels); err != nil {
100-
log.Errorw("store delete failed", zap.Error(err), zap.Any("cfg", cfg), zap.Any("args", args))
104+
if err := c.Store.Delete(cfg.Name, args.ResourceTypes, args.Labels); err != nil {
105+
c.log.Error(err, "store delete failed", "cfg", cfg, "args", args)
101106
return StoreDelta{}, errors.Wrap(err, fmt.Sprintf("store delete failed for config %s", cfg.Name))
102107
}
103108
}
104109

105110
for _, cfg := range delta.Applied {
106-
if err := d.Insert(cfg.Name, args.ResourceTypes, args.Resources, args.Labels); err != nil {
107-
log.Errorw("store insert failed", zap.Error(err), zap.Any("cfg", cfg), zap.Any("args", args))
111+
if err := c.Insert(cfg.Name, args.ResourceTypes, args.Resources, args.Labels); err != nil {
112+
c.log.Error(err, "store insert failed", "cfg", cfg, "args", args)
108113
return StoreDelta{}, errors.Wrap(err, fmt.Sprintf("store insert failed for config %s", cfg.Name))
109114
}
110115
}
111116

112117
return delta, nil
113118
}
114119

115-
func (d *Client) applySync(ctx context.Context, args Task, delta StoreDelta) error {
116-
d.syncMu.RLock()
117-
defer d.syncMu.RUnlock()
120+
func (c *Client) applySync(ctx context.Context, args Task, delta StoreDelta) error {
121+
c.syncMu.RLock()
122+
defer c.syncMu.RUnlock()
118123

119124
if len(delta.Deleted) > 0 {
120-
if err := d.sync(ctx, Task{
125+
if err := c.sync(ctx, Task{
121126
Name: args.Name,
122127
Labels: args.Labels,
123128
ResourceTypes: args.ResourceTypes,
124129
Configs: delta.Deleted,
125130
}); err != nil {
126-
log.Warnw("failed to sync deleted configs", zap.Error(err))
131+
c.log.Error(err, "failed to sync deleted configs", "args", args, "delta", delta)
127132
}
128133
}
129134

130135
if len(delta.Applied) > 0 {
131-
return d.sync(ctx, Task{
136+
return c.sync(ctx, Task{
132137
Name: args.Name,
133138
Labels: args.Labels,
134139
ResourceTypes: args.ResourceTypes,
@@ -139,53 +144,53 @@ func (d *Client) applySync(ctx context.Context, args Task, delta StoreDelta) err
139144
return nil
140145
}
141146

142-
func (d *Client) Update(ctx context.Context, args Task) error {
143-
delta, err := d.applyStoreChanges(args, false)
147+
func (c *Client) Update(ctx context.Context, args Task) error {
148+
delta, err := c.applyStoreChanges(args, false)
144149
if err != nil {
145150
return err
146151
}
147-
return d.applySync(ctx, args, delta)
152+
return c.applySync(ctx, args, delta)
148153
}
149154

150-
func (d *Client) UpdateConfig(ctx context.Context, args Task) error {
151-
_, err := d.applyStoreChanges(args, false)
155+
func (c *Client) UpdateConfig(ctx context.Context, args Task) error {
156+
_, err := c.applyStoreChanges(args, false)
152157
return err
153158
}
154159

155-
func (d *Client) Delete(ctx context.Context, args Task) error {
156-
delta, err := d.applyStoreChanges(args, true)
160+
func (c *Client) Delete(ctx context.Context, args Task) error {
161+
delta, err := c.applyStoreChanges(args, true)
157162
if err != nil {
158163
return err
159164
}
160-
return d.applySync(ctx, args, delta)
165+
return c.applySync(ctx, args, delta)
161166
}
162167

163-
func (d *Client) DeleteConfig(ctx context.Context, args Task) error {
164-
_, err := d.applyStoreChanges(args, true)
168+
func (c *Client) DeleteConfig(ctx context.Context, args Task) error {
169+
_, err := c.applyStoreChanges(args, true)
165170
return err
166171
}
167172

168173
func (c *Client) Sync(ctx context.Context) (map[string]types.ADCExecutionErrors, error) {
169174
c.syncMu.Lock()
170175
defer c.syncMu.Unlock()
171-
log.Debug("syncing all resources")
176+
c.log.Info("syncing all resources")
172177

173178
configs := c.ConfigManager.List()
174179

175180
if len(configs) == 0 {
176-
log.Warn("no GatewayProxy configs provided")
181+
c.log.Info("no GatewayProxy configs provided")
177182
return nil, nil
178183
}
179184

180-
log.Debugw("syncing resources with multiple configs", zap.Any("configs", configs))
185+
c.log.V(1).Info("syncing resources with multiple configs", "configs", configs)
181186

182187
failedMap := map[string]types.ADCExecutionErrors{}
183188
var failedConfigs []string
184189
for _, config := range configs {
185190
name := config.Name
186191
resources, err := c.GetResources(name)
187192
if err != nil {
188-
log.Errorw("failed to get resources from store", zap.String("name", name), zap.Error(err))
193+
c.log.Error(err, "failed to get resources from store", "name", name)
189194
failedConfigs = append(failedConfigs, name)
190195
continue
191196
}
@@ -200,7 +205,7 @@ func (c *Client) Sync(ctx context.Context) (map[string]types.ADCExecutionErrors,
200205
},
201206
Resources: resources,
202207
}); err != nil {
203-
log.Errorw("failed to sync resources", zap.String("name", name), zap.Error(err))
208+
c.log.Error(err, "failed to sync resources", "name", name)
204209
failedConfigs = append(failedConfigs, name)
205210
var execErrs types.ADCExecutionErrors
206211
if errors.As(err, &execErrs) {
@@ -219,10 +224,10 @@ func (c *Client) Sync(ctx context.Context) (map[string]types.ADCExecutionErrors,
219224
}
220225

221226
func (c *Client) sync(ctx context.Context, task Task) error {
222-
log.Debugw("syncing resources", zap.Any("task", task))
227+
c.log.V(1).Info("syncing resources", "task", task)
223228

224229
if len(task.Configs) == 0 {
225-
log.Warnw("no adc configs provided", zap.Any("task", task))
230+
c.log.Info("no adc configs provided")
226231
return nil
227232
}
228233

@@ -238,6 +243,7 @@ func (c *Client) sync(ctx context.Context, task Task) error {
238243
}
239244
pkgmetrics.RecordFileIODuration("prepare_sync_file", adctypes.StatusSuccess, time.Since(fileIOStart).Seconds())
240245
defer cleanup()
246+
c.log.V(1).Info("prepared sync file", "path", syncFilePath)
241247

242248
args := BuildADCExecuteArgs(syncFilePath, task.Labels, task.ResourceTypes)
243249

@@ -255,7 +261,7 @@ func (c *Client) sync(ctx context.Context, task Task) error {
255261
status := adctypes.StatusSuccess
256262
if err != nil {
257263
status = "failure"
258-
log.Errorw("failed to execute adc command", zap.Error(err), zap.Any("config", config))
264+
c.log.Error(err, "failed to execute adc command", "config", config)
259265

260266
var execErr types.ADCExecutionError
261267
if errors.As(err, &execErr) {
@@ -295,7 +301,5 @@ func prepareSyncFile(resources any) (string, func(), error) {
295301
return "", nil, err
296302
}
297303

298-
log.Debugw("generated adc file", zap.String("filename", tmpFile.Name()), zap.String("json", string(data)))
299-
300304
return tmpFile.Name(), cleanup, nil
301305
}

0 commit comments

Comments
 (0)