This implements "part 2" of what was described in the initial Trace2 logging pull request (#28), adding region tracing to repo.go, bundles.go, and bundle-server.go.
The branch is broken up as follows:
The first commit is a consistency "fixup", making sure implementations of interfaces return the interface type in their New*().
The second commit is probably the most complicated of the PR and probably warrants an extra-careful review. After starting to turn more function collections into methods of structs, I realized that there was a lot of messy dependency injection code throughout the codebase. I looked into integrating some more capable service location/dependency injection libraries (like uber-go/dig or google/wire), but none quite fit the use case I wanted (singleton dependency resolution with one instance per type).
I ended up writing a custom dependency resolver, as well as a containers-helper.go file to contain the dependency registering code. Unfortunately, this approach (while cleaning things up nicely in the rest of the codebase) opens us up to more potential runtime errors. To (at least partially) mitigate that, I added a few tests to verify that the container(s) created in containers-helper.go would resolve properly, and that all types requested from the container are registered. The tests are complicated, but I did my best to comment what was going on. Happy to clarify if anyone has any questions though!
Commits 3-5 all deal with refactoring repo.go and bundles.go to make it easier to add logging to them
Commit 6 cleans up a leftover comment from the last PR
Commit 7 implements region tracing. The implementation is meant to take advantage of Go's defer behavior, with Region() starting region tracing and returning a deferrable function that ends the same region.
Commits 8-10 add some regions to bundle-server.go, repo.go, and bundles.go. I'm sure more can/will be added in the future, but these seemed like a good set to start with.
Future work
In addition to the remaining parts mentioned in #28, I ended up doing a lot of refactoring of repo.go and bundles.go to use common.FileSystem and common.CommandExecutor. I pulled out of this PR (since it ended up fairly out-of-scope), but I do plan to eventually finish those and add the appropriate unit tests.
Part of #22.
Summary
This implements "part 2" of what was described in the initial Trace2 logging pull request (#28), adding region tracing to
repo.go,bundles.go, andbundle-server.go.The branch is broken up as follows:
New*().uber-go/digorgoogle/wire), but none quite fit the use case I wanted (singleton dependency resolution with one instance per type). I ended up writing a custom dependency resolver, as well as acontainers-helper.gofile to contain the dependency registering code. Unfortunately, this approach (while cleaning things up nicely in the rest of the codebase) opens us up to more potential runtime errors. To (at least partially) mitigate that, I added a few tests to verify that the container(s) created incontainers-helper.gowould resolve properly, and that all types requested from the container are registered. The tests are complicated, but I did my best to comment what was going on. Happy to clarify if anyone has any questions though!repo.goandbundles.goto make it easier to add logging to themdeferbehavior, withRegion()starting region tracing and returning a deferrable function that ends the same region.bundle-server.go,repo.go, andbundles.go. I'm sure more can/will be added in the future, but these seemed like a good set to start with.Future work
In addition to the remaining parts mentioned in #28, I ended up doing a lot of refactoring of
repo.goandbundles.goto usecommon.FileSystemandcommon.CommandExecutor. I pulled out of this PR (since it ended up fairly out-of-scope), but I do plan to eventually finish those and add the appropriate unit tests.