From a38adb599747ee602d95f463908d879064133ef9 Mon Sep 17 00:00:00 2001 From: Aaron Craelius Date: Mon, 9 May 2022 14:49:46 -0400 Subject: [PATCH 1/4] feat(container): automatic debugging on errors --- container/debug.go | 96 +++++++++++++++++++++++++++++++++++++++++++--- container/run.go | 42 +++++++++++++++++--- 2 files changed, 127 insertions(+), 11 deletions(-) diff --git a/container/debug.go b/container/debug.go index 26862c08c555..edcf2e0a610b 100644 --- a/container/debug.go +++ b/container/debug.go @@ -66,17 +66,79 @@ func Logger(logger func(string)) DebugOption { }) } +const debugContainerSvg = "debug_container.svg" +const debugContainerDot = "debug_container.dot" + // Debug is a default debug option which sends log output to stdout, dumps -// the container in the graphviz DOT format to stdout, and to the file -// container_dump.svg. +// the container in the graphviz DOT and SVG formats to debug_container.dot +// and debug_container.svg respectively. func Debug() DebugOption { return DebugOptions( StdoutLogger(), - LogVisualizer(), - FileVisualizer("container_dump.svg", "svg"), + FileVisualizer(debugContainerSvg, "svg"), + FileVisualizer(debugContainerDot, "dot"), ) } +func (d *debugConfig) initLogBuf() { + if d.logBuf == nil { + d.logBuf = &[]string{} + d.loggers = append(d.loggers, func(s string) { + *d.logBuf = append(*d.logBuf, s) + }) + } +} + +// OnError is a debug option that allows setting debug options that are +// conditional on an error happening. Any loggers added error will +// receive the full dump of logs since the start of container processing. +func OnError(option DebugOption) DebugOption { + return debugOption(func(config *debugConfig) error { + config.initLogBuf() + config.onError = option + return nil + }) +} + +// OnSuccess is a debug option that allows setting debug options that are +// conditional on successful container resolution. Any loggers added on success +// will receive the full dump of logs since the start of container processing. +func OnSuccess(option DebugOption) DebugOption { + return debugOption(func(config *debugConfig) error { + config.initLogBuf() + config.onSuccess = option + return nil + }) +} + +// DebugCleanup specifies a clean-up function to be called at the end of +// processing to clean up any resources that may be used during debugging. +func DebugCleanup(cleanup func()) DebugOption { + return debugOption(func(config *debugConfig) error { + config.cleanup = append(config.cleanup, cleanup) + return nil + }) +} + +// AutoDebug does the same thing as Debug when there is an error and deletes +// the debug_container.dot and debug_container.dot if they exist when there +// is no error. This is the default debug mode of Run. +func AutoDebug() DebugOption { + return DebugOptions( + OnError(Debug()), + OnSuccess(DebugCleanup(func() { + deleteIfExists(debugContainerSvg) + deleteIfExists(debugContainerDot) + })), + ) +} + +func deleteIfExists(filename string) { + if _, err := os.Stat(filename); err == nil { + _ = os.Remove(filename) + } +} + // DebugOptions creates a debug option which bundles together other debug options. func DebugOptions(options ...DebugOption) DebugOption { return debugOption(func(c *debugConfig) error { @@ -94,12 +156,18 @@ type debugConfig struct { // logging loggers []func(string) indentStr string + logBuf *[]string // a log buffer for onError/onSuccess processing // graphing graphviz *graphviz.Graphviz graph *cgraph.Graph visualizers []func(string) logVisualizer bool + + // extra processing + onError DebugOption + onSuccess DebugOption + cleanup []func() } type debugOption func(*debugConfig) error @@ -210,7 +278,7 @@ func (c *debugConfig) locationGraphNode(location Location, key *moduleKey) (*cgr } func (c *debugConfig) typeGraphNode(typ reflect.Type) (*cgraph.Node, error) { - node, found, err := c.findOrCreateGraphNode(c.graph, typ.String()) + node, found, err := c.findOrCreateGraphNode(c.graph, moreUsefulTypeString(typ)) if err != nil { return nil, err } @@ -223,6 +291,22 @@ func (c *debugConfig) typeGraphNode(typ reflect.Type) (*cgraph.Node, error) { return node, err } +// moreUsefulTypeString is more useful than reflect.Type.String() +func moreUsefulTypeString(ty reflect.Type) string { + switch ty.Kind() { + case reflect.Struct, reflect.Interface: + return fmt.Sprintf("%s.%s", ty.PkgPath(), ty.Name()) + case reflect.Pointer: + return fmt.Sprintf("*%s", moreUsefulTypeString(ty.Elem())) + case reflect.Map: + return fmt.Sprintf("map[%s]%s", moreUsefulTypeString(ty.Key()), moreUsefulTypeString(ty.Elem())) + case reflect.Slice: + return fmt.Sprintf("[]%s", moreUsefulTypeString(ty.Elem())) + default: + return ty.String() + } +} + func (c *debugConfig) findOrCreateGraphNode(subGraph *cgraph.Graph, name string) (node *cgraph.Node, found bool, err error) { node, err = c.graph.Node(name) if err != nil { @@ -246,7 +330,7 @@ func (c *debugConfig) moduleSubGraph(key *moduleKey) *cgraph.Graph { if key != nil { gname := fmt.Sprintf("cluster_%s", key.name) graph = c.graph.SubGraph(gname, 1) - graph.SetLabel(fmt.Sprintf("ModuleKey: %s", key.name)) + graph.SetLabel(fmt.Sprintf("Module: %s", key.name)) } return graph } diff --git a/container/run.go b/container/run.go index 12d1b9e2f113..0946749f895b 100644 --- a/container/run.go +++ b/container/run.go @@ -7,24 +7,56 @@ package container // // Ex: // Run(func (x int) error { println(x) }, Provide(func() int { return 1 })) +// +// Run uses the debug mode provided by AutoDebug which means there will be +// verbose debugging information if there is an error and nothing upon success. +// Use RunDebug to configure behavior with more control. func Run(invoker interface{}, opts ...Option) error { - return RunDebug(invoker, nil, opts...) + return RunDebug(invoker, AutoDebug(), opts...) } // RunDebug is a version of Run which takes an optional DebugOption for // logging and visualization. func RunDebug(invoker interface{}, debugOpt DebugOption, opts ...Option) error { - opt := Options(opts...) - cfg, err := newDebugConfig() if err != nil { return err } + // debug cleanup + defer func() { + for _, f := range cfg.cleanup { + f() + } + }() + + err = run(cfg, invoker, debugOpt, opts...) + if err != nil { + if cfg.onError != nil { + err = cfg.onError.applyConfig(cfg) + if err != nil { + return err + } + } + return err + } else { + if cfg.onSuccess != nil { + err = cfg.onSuccess.applyConfig(cfg) + if err != nil { + return err + } + } + return nil + } +} + +func run(cfg *debugConfig, invoker interface{}, debugOpt DebugOption, opts ...Option) error { + opt := Options(opts...) + defer cfg.generateGraph() // always generate graph on exit if debugOpt != nil { - err = debugOpt.applyConfig(cfg) + err := debugOpt.applyConfig(cfg) if err != nil { return err } @@ -33,7 +65,7 @@ func RunDebug(invoker interface{}, debugOpt DebugOption, opts ...Option) error { cfg.logf("Registering providers") cfg.indentLogger() ctr := newContainer(cfg) - err = opt.apply(ctr) + err := opt.apply(ctr) if err != nil { cfg.logf("Failed registering providers because of: %+v", err) return err From 01872192432accf7b1d25cb349f688154c322522 Mon Sep 17 00:00:00 2001 From: Aaron Craelius Date: Mon, 9 May 2022 14:58:22 -0400 Subject: [PATCH 2/4] tests --- container/container_test.go | 30 ++++++++++++++++++++++++++++++ container/debug.go | 8 ++++++++ container/run.go | 12 ++++++------ 3 files changed, 44 insertions(+), 6 deletions(-) diff --git a/container/container_test.go b/container/container_test.go index 15d8d1a3b6d9..a7c93d7e9fe6 100644 --- a/container/container_test.go +++ b/container/container_test.go @@ -542,3 +542,33 @@ func TestLogging(t *testing.T) { require.NoError(t, err) require.Contains(t, string(graphfileContents), " Date: Mon, 9 May 2022 20:54:02 -0400 Subject: [PATCH 3/4] group deps --- container/debug.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/container/debug.go b/container/debug.go index 4044595fe118..2c9cb8cc11a2 100644 --- a/container/debug.go +++ b/container/debug.go @@ -74,8 +74,10 @@ func Logger(logger func(string)) DebugOption { }) } -const debugContainerSvg = "debug_container.svg" -const debugContainerDot = "debug_container.dot" +const ( + debugContainerSvg = "debug_container.svg" + debugContainerDot = "debug_container.dot" +) // Debug is a default debug option which sends log output to stdout, dumps // the container in the graphviz DOT and SVG formats to debug_container.dot From e9e760773ea8963cada6da6f21aa8621799f867e Mon Sep 17 00:00:00 2001 From: Aaron Craelius Date: Tue, 10 May 2022 12:42:53 -0400 Subject: [PATCH 4/4] fix test errors --- container/container_test.go | 12 +++++++----- go.mod | 2 -- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/container/container_test.go b/container/container_test.go index 32c1262765b0..e393b91e2439 100644 --- a/container/container_test.go +++ b/container/container_test.go @@ -592,20 +592,22 @@ func TestConditionalDebugging(t *testing.T) { success = true }))) - require.Error(t, container.RunDebug( - func(input TestInput) {}, + var input TestInput + require.Error(t, container.BuildDebug( conditionalDebugOpt, + container.Options(), + &input, )) require.Contains(t, logs, `Initializing logger`) require.Contains(t, logs, `Registering providers`) - require.Contains(t, logs, `Registering invoker`) + require.Contains(t, logs, `Registering outputs`) require.False(t, success) logs = "" success = false - require.NoError(t, container.RunDebug( - func() {}, + require.NoError(t, container.BuildDebug( conditionalDebugOpt, + container.Options(), )) require.Empty(t, logs) require.True(t, success) diff --git a/go.mod b/go.mod index 2ed78e1fbc3d..9dd5dd4664a0 100644 --- a/go.mod +++ b/go.mod @@ -61,8 +61,6 @@ require ( sigs.k8s.io/yaml v1.3.0 ) -require github.com/google/uuid v1.3.0 - require ( cloud.google.com/go v0.100.2 // indirect cloud.google.com/go/compute v1.5.0 // indirect