add output formatter #673
add output formatter #673
Conversation
maksymvavilov
force-pushed
the
gh-657
branch
from
7520699 to
2879449
Compare
maksymvavilov
force-pushed
the
gh-657
branch
2 times, most recently
from
32b1be7 to
89e636e
Compare
maksymvavilov
force-pushed
the
gh-657
branch
5 times, most recently
from
f561a28 to
87d155d
Compare
Signed-off-by: Maskym Vavilov <mvavilov@redhat.com>
maksymvavilov
force-pushed
the
gh-657
branch
from
87d155d to
f1afffe
Compare
| default: | ||
| loggerLevel = zapcore.InfoLevel | ||
| } | ||
| return zap.New(zap.UseDevMode(false), zap.WriteTo(os.Stderr), zap.Level(loggerLevel)) |
There was a problem hiding this comment.
I'm setting devMode here to false. It will be less human-readable this way. The DevMode uses a different config, and it invalidates any log levels passed in (it always will act as if it is in a debug mode)
| externaldns "sigs.k8s.io/external-dns/endpoint" | ||
| ) | ||
|
|
||
| func RenderEndpoints(endpoints []*externaldns.Endpoint) { |
There was a problem hiding this comment.
The RenderEndpoints became a more generic PrintTable in the formatter.
| type OutputFormatter interface { | ||
| Print(message string) | ||
| Error(err error, message string) | ||
| PrintObject(object any) |
There was a problem hiding this comment.
There were concerns voiced over using any type, and I'm not a fan of it myself. However, I found that it works well enough with primitives.
That said, I don't see it being used - the Print(string) and PrintTable(...) should cover us for most of the cases, and the PrintObject(any), despite having any as type, should only be used for a single struct.
Also, I opted for the yaml format for a single struct. I've tried a number of options and found yaml to be the most human-friendly.
| func init() { | ||
| rootCMD.SetArgs(os.Args[1:]) | ||
| rootCMD.PersistentFlags().IntVarP(&verbose, "verbose", "v", 0, "verbosity level: 0 (errors only), 1 (+ info), 2 (+ debug)") | ||
| rootCMD.PersistentFlags().IntVarP(&verbose, "verbose", "v", output.DefaultLevel, "the higher verbosity level the more restrictive output; range from -1 (debug, includes everything) to 4 (panic, most restrictive)") |
There was a problem hiding this comment.
I've changed those levels to match the approach logr has. There is still a bit of a "translation" happening, but our operator and cli will now follow the same idea - the higher level, the more restrictive it is. With the -1 being debug/verbose mode
| PrintTable(array PrintableTable) | ||
| } | ||
|
|
||
| type PrintableTable struct { |
There was a problem hiding this comment.
What do you expect this to look like, when output as YAML or JSON or whatever?
philbrookes
left a comment
philbrookes
left a comment
There was a problem hiding this comment.
I've tried this locally and the format looks fine now. I still have some concerns over how this will look when extended to more data-focused formats like JSON or YAML, but it's probably easier to just address those at the time, rather than trying to over-engineer for them here.
Happy for this to merge as it is now.
2 participants
adding text formatter and aligning logger levels with the zap implementation