Skip to content

Commit 4690ded

Browse files
bharathi-tennetibharathi-tenneti
committed
Addressing PR comments
1 parent a0790e0 commit 4690ded

File tree

3 files changed

+46
-58
lines changed

3 files changed

+46
-58
lines changed

‎pkg/log/zap/flags.go

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -35,9 +35,8 @@ var levelStrings = map[string]zapcore.Level{
3535
}
3636

3737
var stackLevelStrings = map[string]zapcore.Level{
38-
"disabled": zap.FatalLevel,
39-
"info": zap.InfoLevel,
40-
"error": zap.ErrorLevel,
38+
"info": zap.InfoLevel,
39+
"error": zap.ErrorLevel,
4140
}
4241

4342
type encoderFlag struct {

‎pkg/log/zap/zap.go

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -184,7 +184,7 @@ func (o *Options) addDefaults() {
184184
// Disable sampling when we are in debug mode. Otherwise, this will
185185
// cause index out of bounds errors in the sampling code.
186186
if o.Level.Enabled(zapcore.DebugLevel) {
187-
o.ZapOpts = append(o.ZapOpts, zap.Development())
187+
o.ZapOpts = append(o.ZapOpts)
188188
} else {
189189
o.ZapOpts = append(o.ZapOpts,
190190
zap.WrapCore(func(core zapcore.Core) zapcore.Core {
@@ -220,7 +220,7 @@ func NewRaw(opts ...Opts) *zap.Logger {
220220
// zap-encoder: Zap log encoding ('json' or 'console')
221221
// zap-log-level: Zap Level to configure the verbosity of logging. Can be one of 'debug', 'info', 'error',
222222
// or any integer value > 0 which corresponds to custom debug levels of increasing verbosity")
223-
// zap-stacktrace-level: Zap Level at and above which stacktraces are captured (one of 'warn' or 'error')
223+
// zap-stacktrace-level: Zap Level at and above which stacktraces are captured (one of 'info' or 'error')
224224
func (o *Options) BindFlags(fs *flag.FlagSet) {
225225

226226
// Set Development mode value
@@ -250,8 +250,7 @@ func (o *Options) BindFlags(fs *flag.FlagSet) {
250250
o.StacktraceLevel = fromFlag
251251
}
252252
fs.Var(&stackVal, "zap-stacktrace-level",
253-
"Zap Level at and above which stacktraces are captured (one of 'info', 'error', or 'disabled')."+
254-
"--zap-stacktrace-level=disabled will set level to be 'fatal'.")
253+
"Zap Level at and above which stacktraces are captured (one of 'info', 'error').")
255254
}
256255

257256
// UseFlagOptions configures the logger to use the Options set by parsing zap option flags from the CLI.

‎pkg/log/zap/zap_test.go

Lines changed: 41 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -302,39 +302,39 @@ var _ = Describe("Zap logger setup", func() {
302302

303303
var _ = Describe("Zap log level flag options setup", func() {
304304
var (
305-
fromFlags Options
306-
fs flag.FlagSet
307-
info = "info text"
308-
debug1 = "debug 1 text"
309-
debug2 = "debug 2 text"
310-
debug3 = "debug 3 text"
305+
fromFlags Options
306+
fs flag.FlagSet
307+
logInfoLevel0 = "info text"
308+
logDebugLevel1 = "debug 1 text"
309+
logDebugLevel2 = "debug 2 text"
310+
logDebugLevel3 = "debug 3 text"
311311
)
312312

313313
BeforeEach(func() {
314314
fromFlags = Options{}
315-
fs = *flag.NewFlagSet(os.Args[0], flag.ExitOnError) //flags are now reset
315+
fs = *flag.NewFlagSet(os.Args[0], flag.ExitOnError)
316316
})
317317

318318
Context("with zap-log-level options provided", func() {
319-
It("Should set info/debug zap-log-level flags.", func() {
319+
It("Should output logs for info and debug zap-log-level.", func() {
320320
args := []string{"--zap-log-level=debug"}
321321
fromFlags.BindFlags(&fs)
322322
err := fs.Parse(args)
323323
Expect(err).ToNot(HaveOccurred())
324324
logOut := new(bytes.Buffer)
325325

326326
logger := New(UseFlagOptions(&fromFlags), WriteTo(logOut))
327-
logger.V(0).Info(info)
328-
logger.V(1).Info(debug1)
327+
logger.V(0).Info(logInfoLevel0)
328+
logger.V(1).Info(logDebugLevel1)
329329

330330
outRaw := logOut.Bytes()
331331

332-
Expect(string(outRaw)).Should(ContainSubstring(info))
333-
Expect(string(outRaw)).Should(ContainSubstring(debug1))
332+
Expect(string(outRaw)).Should(ContainSubstring(logInfoLevel0))
333+
Expect(string(outRaw)).Should(ContainSubstring(logDebugLevel1))
334334

335335
})
336336

337-
It("Should set error zap-log-level flags.", func() {
337+
It("Should output only error logs, otherwise empty logs", func() {
338338
args := []string{"--zap-log-level=error"}
339339
fromFlags.BindFlags(&fs)
340340
err := fs.Parse(args)
@@ -343,8 +343,8 @@ var _ = Describe("Zap log level flag options setup", func() {
343343
logOut := new(bytes.Buffer)
344344

345345
logger := New(UseFlagOptions(&fromFlags), WriteTo(logOut))
346-
logger.V(0).Info(info)
347-
logger.V(1).Info(debug1)
346+
logger.V(0).Info(logInfoLevel0)
347+
logger.V(1).Info(logDebugLevel1)
348348

349349
outRaw := logOut.Bytes()
350350

@@ -354,84 +354,84 @@ var _ = Describe("Zap log level flag options setup", func() {
354354
})
355355

356356
Context("with zap-log-level with increased verbosity.", func() {
357-
It("Should set integer debug level 1 for zap-log-level flag.", func() {
357+
It("Should output debug and info log, with default production mode.", func() {
358358
args := []string{"--zap-log-level=1"}
359359
fromFlags.BindFlags(&fs)
360360
err := fs.Parse(args)
361361
Expect(err).ToNot(HaveOccurred())
362362
logOut := new(bytes.Buffer)
363363

364364
logger := New(UseFlagOptions(&fromFlags), WriteTo(logOut))
365-
logger.V(0).Info(info)
366-
logger.V(1).Info(debug1)
365+
logger.V(0).Info(logInfoLevel0)
366+
logger.V(1).Info(logDebugLevel1)
367367

368368
outRaw := logOut.Bytes()
369369

370-
Expect(string(outRaw)).Should(ContainSubstring(info))
371-
Expect(string(outRaw)).Should(ContainSubstring(debug1))
370+
Expect(string(outRaw)).Should(ContainSubstring(logInfoLevel0))
371+
Expect(string(outRaw)).Should(ContainSubstring(logDebugLevel1))
372372
})
373373

374-
It("Should set integer debug level 1 for zap-log-level flag.", func() {
374+
It("Should output info and debug logs, with development mode.", func() {
375375
args := []string{"--zap-log-level=1", "--zap-devel=true"}
376376
fromFlags.BindFlags(&fs)
377377
err := fs.Parse(args)
378378
Expect(err).ToNot(HaveOccurred())
379379
logOut := new(bytes.Buffer)
380380

381381
logger := New(UseFlagOptions(&fromFlags), WriteTo(logOut))
382-
logger.V(0).Info(info)
383-
logger.V(1).Info(debug1)
382+
logger.V(0).Info(logInfoLevel0)
383+
logger.V(1).Info(logDebugLevel1)
384384

385385
outRaw := logOut.Bytes()
386386

387-
Expect(string(outRaw)).Should(ContainSubstring(info))
388-
Expect(string(outRaw)).Should(ContainSubstring(debug1))
387+
Expect(string(outRaw)).Should(ContainSubstring(logInfoLevel0))
388+
Expect(string(outRaw)).Should(ContainSubstring(logDebugLevel1))
389389
})
390390

391-
It("Should set integer debug level 2 for zap-log-level flag.", func() {
391+
It("Should output info, and debug logs with increased verbosity, and with development mode set to true.", func() {
392392
args := []string{"--zap-log-level=2", "--zap-devel=true"}
393393
fromFlags.BindFlags(&fs)
394394
err := fs.Parse(args)
395395
Expect(err).ToNot(HaveOccurred())
396396
logOut := new(bytes.Buffer)
397397

398398
logger := New(UseFlagOptions(&fromFlags), WriteTo(logOut))
399-
logger.V(0).Info(info)
400-
logger.V(1).Info(debug1)
401-
logger.V(2).Info(debug2)
399+
logger.V(0).Info(logInfoLevel0)
400+
logger.V(1).Info(logDebugLevel1)
401+
logger.V(2).Info(logDebugLevel2)
402402

403403
outRaw := logOut.Bytes()
404404

405-
Expect(string(outRaw)).Should(ContainSubstring(info))
406-
Expect(string(outRaw)).Should(ContainSubstring(debug1))
407-
Expect(string(outRaw)).Should(ContainSubstring(debug2))
405+
Expect(string(outRaw)).Should(ContainSubstring(logInfoLevel0))
406+
Expect(string(outRaw)).Should(ContainSubstring(logDebugLevel1))
407+
Expect(string(outRaw)).Should(ContainSubstring(logDebugLevel2))
408408

409409
})
410-
It("Should set integer debug level 3 for zap-log-level flag.", func() {
410+
It("Should output info, and debug logs with increased verbosity, and with production mode set to true.", func() {
411411
args := []string{"--zap-log-level=3", "--zap-devel=false"}
412412
fromFlags.BindFlags(&fs)
413413
err := fs.Parse(args)
414414
Expect(err).ToNot(HaveOccurred())
415415
logOut := new(bytes.Buffer)
416416

417417
logger := New(UseFlagOptions(&fromFlags), WriteTo(logOut))
418-
logger.V(0).Info(info)
419-
logger.V(1).Info(debug1)
420-
logger.V(3).Info(debug3)
418+
logger.V(0).Info(logInfoLevel0)
419+
logger.V(1).Info(logDebugLevel1)
420+
logger.V(3).Info(logDebugLevel3)
421421

422422
outRaw := logOut.Bytes()
423423

424-
Expect(string(outRaw)).Should(ContainSubstring(info))
425-
Expect(string(outRaw)).Should(ContainSubstring(debug1))
426-
Expect(string(outRaw)).Should(ContainSubstring(debug3))
424+
Expect(string(outRaw)).Should(ContainSubstring(logInfoLevel0))
425+
Expect(string(outRaw)).Should(ContainSubstring(logDebugLevel1))
426+
Expect(string(outRaw)).Should(ContainSubstring(logDebugLevel3))
427427

428428
})
429429

430430
})
431431

432432
Context("with zap-stacktrace-level options provided", func() {
433433

434-
It("Should set info stacktrace for zap-stacktrace-level flags.", func() {
434+
It("Should output stacktrace at info level, with development mode set to true.", func() {
435435
args := []string{"--zap-stacktrace-level=info", "--zap-devel=true"}
436436
fromFlags.BindFlags(&fs)
437437
err := fs.Parse(args)
@@ -442,7 +442,7 @@ var _ = Describe("Zap log level flag options setup", func() {
442442
Expect(out.StacktraceLevel.Enabled(zapcore.InfoLevel)).To(BeTrue())
443443
})
444444

445-
It("Should set error stacktrace for zap-stacktrace-level flags.", func() {
445+
It("Should output stacktrace at error level, with development mode set to true.", func() {
446446
args := []string{"--zap-stacktrace-level=error", "--zap-devel=true"}
447447
fromFlags.BindFlags(&fs)
448448
err := fs.Parse(args)
@@ -453,16 +453,6 @@ var _ = Describe("Zap log level flag options setup", func() {
453453
Expect(out.StacktraceLevel.Enabled(zapcore.ErrorLevel)).To(BeTrue())
454454
})
455455

456-
It("Should disable stacktrace for zap-stacktrace-level flags.", func() {
457-
args := []string{"--zap-stacktrace-level=disabled", "--zap-devel=true"}
458-
fromFlags.BindFlags(&fs)
459-
err := fs.Parse(args)
460-
Expect(err).ToNot(HaveOccurred())
461-
out := Options{}
462-
UseFlagOptions(&fromFlags)(&out)
463-
464-
Expect(out.StacktraceLevel.Enabled(zapcore.FatalLevel)).To(BeTrue())
465-
})
466456
})
467457

468458
})

0 commit comments

Comments
 (0)