Include Module Traces in Events #1122
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
JacobOaks
changed the title
Codecov Report
@@ Coverage Diff @@
## master #1122 +/- ##
=======================================
Coverage 98.80% 98.80%
=======================================
Files 36 36
Lines 3088 3106 +18
=======================================
+ Hits 3051 3069 +18
Misses 30 30
Partials 7 7
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
tchung1118
reviewed
Oct 3, 2023
JacobOaks
force-pushed
the
joaks/module_stack
branch
2 times, most recently
from
29707a2
to
52ccde7
Compare
Currently, the events that the fxevent package emits for provides/decorates/supplies/replaces
include stack traces of where the Fx call was made.
These stack traces often look something like this:
```
somepkg.init (/path/to/some/distant/package/module.go:14)
runtime.doInit1 (/opt/homebrew/Cellar/go/1.21.1/libexec/src/runtime/proc.go:6740)
runtime.doInit (/opt/homebrew/Cellar/go/1.21.1/libexec/src/runtime/proc.go:6707)
runtime.main (/opt/homebrew/Cellar/go/1.21.1/libexec/src/runtime/proc.go:249)
```
While these do show exactly where the provide/decorate/supply/replace statement lives,
for larger codebases where a single app may include a wide variety of modules,
each with their own chain of sub-modules, this stack trace does a bad job of answering:
"How did this constructor get included in my app in the first place?"
This PR proposes module traces, an attempt to alleviate this issue
by providing a trace through the module chain by which a
provide/decorate/supply/replace found its way to the top-level app declaration.
For this example,
```
9 func getThing() thing {
10 return thing{}
11 }
12
13 var ThingModule = fx.Module(
14 "ThingFx",
15 fx.Provide(getThing),
16 )
17
18 var ServiceModule = fx.Module(
19 "ServiceFx",
20 ThingModule,
21 )
22
23 func main() {
24 app := fx.New(
25 ServiceModule,
26 fx.Invoke(useThing),
27 )
28 }
```
The provide event for `getThing` will include the following module trace:
```
main.init (/Users/joaks/go/src/scratch/mod_stacks_scratch/main.go:15)
main.init (/Users/joaks/go/src/scratch/mod_stacks_scratch/main.go:13) (ThingFx)
main.init (/Users/joaks/go/src/scratch/mod_stacks_scratch/main.go:18) (ServiceFx)
main.run (/Users/joaks/go/src/scratch/mod_stacks_scratch/main.go:24)
```
Which shows exactly how `getThing` got included in `app` in order:
* getThing is provided at `main.go:15`
* ThingFx is declared at `main.go:13`
* ServiceFx is declared at `main.go:18`
* The app is declared at `main.go:24`
This makes it more clear to the developer how functions are getting included,
especially for large codebase where modules may be distributed all over.
JacobOaks
force-pushed
the
joaks/module_stack
branch
from
52ccde7
to
d1ee0d1
Compare
tchung1118
approved these changes
Oct 4, 2023
sywhang
reviewed
Oct 17, 2023
app.go
Outdated
| @@ -432,7 +432,8 @@ func New(opts ...Option) *App { | |||
| // user gave us. For the last case, however, we need to fall | |||
| // back to what was provided to fx.Logger if fx.WithLogger | |||
| // fails. | |||
| log: logger, | |||
| log: logger, | |||
| trace: []string{fxreflect.CallerStack(1, 0)[0].String()}, | |||
There was a problem hiding this comment.
Consider calling CallerStack with depth 1 instead of 0 since you're only taking the top frame. This makes us collect up to 8 frames only to take the top one.
There was a problem hiding this comment.
Good point - I updated to pass depth 2, since we skip 1.
JacobOaks
force-pushed
the
joaks/module_stack
branch
from
e228192
to
d62e237
Compare
sywhang
approved these changes
Oct 17, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Currently, the events that the fxevent package emits for provides/decorates/supplies/replaces include stack traces of where the Fx call was made.
These stack traces often look something like this:
While these do show exactly where the provide/decorate/supply/replace statement lives, for larger codebases where a single app may include a wide variety of modules, each with their own chain of sub-modules, this stack trace does a bad job of answering: "How did this constructor get included in my app in the first place?"
This PR proposes module traces, an attempt to alleviate this issue by providing a trace through the module chain by which a provide/decorate/supply/replace found its way to the top-level app declaration.
For this example,
The provide event for
getThingwill include the following module trace:Which shows exactly how
getThinggot included inappin order:main.go:15main.go:13main.go:18main.go:24This makes it more clear to the developer how functions are getting included, especially for large codebase where modules may be distributed all over.