Skip to content

Commit

Permalink
refactor(container)!: remove dependency on C graphviz (#11934)
Browse files Browse the repository at this point in the history
## Description

Closes: #11925 

This replace the dependency on https://pkg.go.dev/github.com/goccy/go-graphviz which wraps the whole C Graphviz library and is causing ARM build problems in #11924 and generally probably shouldn't be used because it's a heavyweight dependency just used for debugging.

It adds:
* a custom `graphviz` package that does just what we need for `container`
* updates to graphviz rendering to make it nicer and a README with some examples
* golden tests for graphviz and log debugging
* a `StderrLogger` `DebugOption` which is now the default for `Debug`/`AutoDebug`



---

### Author Checklist

*All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.*

I have...

- [ ] included the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title
- [ ] added `!` to the type prefix if API or client breaking change
- [ ] targeted the correct branch (see [PR Targeting](https://github.com/cosmos/cosmos-sdk/blob/main/CONTRIBUTING.md#pr-targeting))
- [ ] provided a link to the relevant issue or specification
- [ ] followed the guidelines for [building modules](https://github.com/cosmos/cosmos-sdk/blob/main/docs/building-modules)
- [ ] included the necessary unit and integration [tests](https://github.com/cosmos/cosmos-sdk/blob/main/CONTRIBUTING.md#testing)
- [ ] added a changelog entry to `CHANGELOG.md`
- [ ] included comments for [documenting Go code](https://blog.golang.org/godoc)
- [ ] updated the relevant documentation or specification
- [ ] reviewed "Files changed" and left comments if necessary
- [ ] confirmed all CI checks have passed

### Reviewers Checklist

*All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.*

I have...

- [ ] confirmed the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title
- [ ] confirmed `!` in the type prefix if API or client breaking change
- [ ] confirmed all author checklist items have been addressed 
- [ ] reviewed state machine logic
- [ ] reviewed API design and naming
- [ ] reviewed documentation is accurate
- [ ] reviewed tests and test coverage
- [ ] manually tested (if applicable)
  • Loading branch information
aaronc authored May 12, 2022
1 parent 38df584 commit 7a31a28
Show file tree
Hide file tree
Showing 24 changed files with 943 additions and 195 deletions.
4 changes: 4 additions & 0 deletions container/Makefile
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
update-testdata-examples:
go test . -test.update-golden
dot -Tsvg testdata/example.dot > testdata/example.svg
dot -Tsvg testdata/example_error.dot > testdata/example_error.svg
34 changes: 34 additions & 0 deletions container/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
# Cosmos SDK Dependency Injection `container` Module

## Overview

TODO

## Usage

TODO

## Debugging

Issues with resolving dependencies in the container can be done with logs
and [Graphviz](https://graphviz.org) renderings of the container tree. By default, whenever there is an error, logs will
be printed to stderr and a rendering of the dependency graph in Graphviz DOT format will be saved to
`debug_container.dot`.

Here is an example Graphviz rendering of a successful build of a dependency graph:
![Graphviz Example](./testdata/example.svg)

Rectangles represent functions, ovals represent types, rounded rectangles represent modules and the single hexagon
represents the function which called `Build`. Black-colored shapes mark functions and types that were called/resolved
without an error. Gray-colored nodes mark functions and types that could have been called/resolved in the container but
were left unused.

Here is an example Graphviz rendering of a dependency graph build which failed:
![Graphviz Error Example](./testdata/example_error.svg)

Graphviz DOT files can be converted into SVG's for viewing in a web browser using the `dot` command-line tool, ex:
```
> dot -Tsvg debug_container.dot > debug_container.svg
```

Many other tools including some IDEs support working with DOT files.
1 change: 1 addition & 0 deletions container/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ func build(loc Location, debugOpt DebugOption, option Option, outputs ...interfa

err = doBuild(cfg, loc, debugOpt, option, outputs...)
if err != nil {
cfg.logf("Error: %v", err)
if cfg.onError != nil {
err2 := cfg.onError.applyConfig(cfg)
if err2 != nil {
Expand Down
70 changes: 22 additions & 48 deletions container/container.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,9 @@ import (
"fmt"
"reflect"

"github.com/goccy/go-graphviz/cgraph"
"github.com/pkg/errors"

"github.com/cosmos/cosmos-sdk/container/internal/graphviz"
)

type container struct {
Expand Down Expand Up @@ -38,10 +39,8 @@ func newContainer(cfg *debugConfig) *container {

func (c *container) call(provider *ProviderDescriptor, moduleKey *moduleKey) ([]reflect.Value, error) {
loc := provider.Location
graphNode, err := c.locationGraphNode(loc, moduleKey)
if err != nil {
return nil, err
}
graphNode := c.locationGraphNode(loc, moduleKey)

markGraphNodeAsFailed(graphNode)

if c.callerMap[loc] {
Expand Down Expand Up @@ -87,17 +86,13 @@ func (c *container) getResolver(typ reflect.Type) (resolver, error) {
elemType = elemType.Elem()
}

var typeGraphNode *cgraph.Node
var err error
var typeGraphNode *graphviz.Node

if isAutoGroupType(elemType) {
c.logf("Registering resolver for auto-group type %v", elemType)
sliceType := reflect.SliceOf(elemType)

typeGraphNode, err = c.typeGraphNode(sliceType)
if err != nil {
return nil, err
}
typeGraphNode = c.typeGraphNode(sliceType)
typeGraphNode.SetComment("auto-group")

r := &groupResolver{
Expand All @@ -112,10 +107,7 @@ func (c *container) getResolver(typ reflect.Type) (resolver, error) {
c.logf("Registering resolver for one-per-module type %v", elemType)
mapType := reflect.MapOf(stringType, elemType)

typeGraphNode, err = c.typeGraphNode(mapType)
if err != nil {
return nil, err
}
typeGraphNode = c.typeGraphNode(mapType)
typeGraphNode.SetComment("one-per-module")

r := &onePerModuleResolver{
Expand All @@ -136,11 +128,7 @@ func (c *container) getResolver(typ reflect.Type) (resolver, error) {
var stringType = reflect.TypeOf("")

func (c *container) addNode(provider *ProviderDescriptor, key *moduleKey) (interface{}, error) {
providerGraphNode, err := c.locationGraphNode(provider.Location, key)
if err != nil {
return nil, err
}

providerGraphNode := c.locationGraphNode(provider.Location, key)
hasModuleKeyParam := false
hasOwnModuleKeyParam := false
for _, in := range provider.Inputs {
Expand All @@ -164,11 +152,11 @@ func (c *container) addNode(provider *ProviderDescriptor, key *moduleKey) (inter
return nil, err
}

var typeGraphNode *cgraph.Node
var typeGraphNode *graphviz.Node
if vr != nil {
typeGraphNode = vr.typeGraphNode()
} else {
typeGraphNode, err = c.typeGraphNode(typ)
typeGraphNode = c.typeGraphNode(typ)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -215,11 +203,7 @@ func (c *container) addNode(provider *ProviderDescriptor, key *moduleKey) (inter
} else {
c.logf("Registering resolver for simple type %v", typ)

typeGraphNode, err := c.typeGraphNode(typ)
if err != nil {
return nil, err
}

typeGraphNode := c.typeGraphNode(typ)
vr = &simpleResolver{
node: sp,
typ: typ,
Expand Down Expand Up @@ -260,11 +244,7 @@ func (c *container) addNode(provider *ProviderDescriptor, key *moduleKey) (inter
typ, provider.Location, existing.describeLocation())
}

typeGraphNode, err := c.typeGraphNode(typ)
if err != nil {
return reflect.Value{}, err
}

typeGraphNode := c.typeGraphNode(typ)
c.resolvers[typ] = &moduleDepResolver{
typ: typ,
idxInValues: i,
Expand All @@ -282,17 +262,9 @@ func (c *container) addNode(provider *ProviderDescriptor, key *moduleKey) (inter

func (c *container) supply(value reflect.Value, location Location) error {
typ := value.Type()
locGrapNode, err := c.locationGraphNode(location, nil)
if err != nil {
return err
}
locGrapNode := c.locationGraphNode(location, nil)
markGraphNodeAsUsed(locGrapNode)

typeGraphNode, err := c.typeGraphNode(typ)
if err != nil {
return err
}

typeGraphNode := c.typeGraphNode(typ)
c.addGraphEdge(locGrapNode, typeGraphNode)

if existing, ok := c.resolvers[typ]; ok {
Expand All @@ -312,10 +284,7 @@ func (c *container) supply(value reflect.Value, location Location) error {
func (c *container) resolve(in ProviderInput, moduleKey *moduleKey, caller Location) (reflect.Value, error) {
c.resolveStack = append(c.resolveStack, resolveFrame{loc: caller, typ: in.Type})

typeGraphNode, err := c.typeGraphNode(in.Type)
if err != nil {
return reflect.Value{}, err
}
typeGraphNode := c.typeGraphNode(in.Type)

if in.Type == moduleKeyType {
if moduleKey == nil {
Expand Down Expand Up @@ -392,6 +361,8 @@ func (c *container) build(loc Location, outputs ...interface{}) error {
},
Location: loc,
}
callerGraphNode := c.locationGraphNode(loc, nil)
callerGraphNode.SetShape("hexagon")

desc, err := expandStructArgsProvider(desc)
if err != nil {
Expand Down Expand Up @@ -443,10 +414,13 @@ func (c container) formatResolveStack() string {
return buf.String()
}

func markGraphNodeAsUsed(node *cgraph.Node) {
func markGraphNodeAsUsed(node *graphviz.Node) {
node.SetColor("black")
node.SetPenWidth("1.5")
node.SetFontColor("black")
}

func markGraphNodeAsFailed(node *cgraph.Node) {
func markGraphNodeAsFailed(node *graphviz.Node) {
node.SetColor("red")
node.SetFontColor("red")
}
40 changes: 31 additions & 9 deletions container/container_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"testing"

"github.com/stretchr/testify/require"
"gotest.tools/v3/golden"

"github.com/cosmos/cosmos-sdk/container"
)
Expand Down Expand Up @@ -81,6 +82,13 @@ func (ModuleB) Provide(dependencies BDependencies) (BProvides, Handler, error) {
}, Handler{}, nil
}

var scenarioConfig = container.Options(
container.Provide(ProvideMsgClientA),
container.ProvideInModule("runtime", ProvideKVStoreKey),
container.ProvideInModule("a", wrapMethod0(ModuleA{})),
container.ProvideInModule("b", wrapMethod0(ModuleB{})),
)

func TestScenario(t *testing.T) {
var (
handlers map[string]Handler
Expand All @@ -90,12 +98,7 @@ func TestScenario(t *testing.T) {
)
require.NoError(t,
container.Build(
container.Options(
container.Provide(ProvideMsgClientA),
container.ProvideInModule("runtime", ProvideKVStoreKey),
container.ProvideInModule("a", wrapMethod0(ModuleA{})),
container.ProvideInModule("b", wrapMethod0(ModuleB{})),
),
scenarioConfig,
&handlers,
&commands,
&a,
Expand Down Expand Up @@ -539,7 +542,7 @@ func TestStructArgs(t *testing.T) {
))
}

func TestLogging(t *testing.T) {
func TestDebugOptions(t *testing.T) {
var logOut string
var dotGraph string

Expand All @@ -563,7 +566,7 @@ func TestLogging(t *testing.T) {
dotGraph = g
}),
container.LogVisualizer(),
container.FileVisualizer(graphfile.Name(), "svg"),
container.FileVisualizer(graphfile.Name()),
container.StdoutLogger(),
),
container.Options(),
Expand All @@ -578,7 +581,26 @@ func TestLogging(t *testing.T) {

graphfileContents, err := os.ReadFile(graphfile.Name())
require.NoError(t, err)
require.Contains(t, string(graphfileContents), "<svg")
require.Contains(t, string(graphfileContents), "digraph")
}

func TestGraphAndLogOutput(t *testing.T) {
var graphOut string
var b KeeperB
debugOpts := container.DebugOptions(
container.Visualizer(func(dotGraph string) {
graphOut = dotGraph
}))
require.NoError(t, container.BuildDebug(debugOpts, scenarioConfig, &b))
golden.Assert(t, graphOut, "example.dot")

badConfig := container.Options(
container.ProvideInModule("runtime", ProvideKVStoreKey),
container.ProvideInModule("a", wrapMethod0(ModuleA{})),
container.ProvideInModule("b", wrapMethod0(ModuleB{})),
)
require.Error(t, container.BuildDebug(debugOpts, badConfig, &b))
golden.Assert(t, graphOut, "example_error.dot")
}

func TestConditionalDebugging(t *testing.T) {
Expand Down
Loading

0 comments on commit 7a31a28

Please sign in to comment.