Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Include Module Traces in Events #1122

Merged
merged 3 commits into from
Oct 17, 2023
Merged

Conversation

JacobOaks
Copy link
Contributor

@JacobOaks JacobOaks commented Oct 2, 2023

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 JacobOaks changed the title Include Module Traces in Provided/Decorated/Supplied/Replaced Events Include Module Traces in Events Oct 2, 2023
@codecov
Copy link

codecov bot commented Oct 2, 2023

Codecov Report

Merging #1122 (7692fed) into master (bbb3783) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           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           
Files Coverage Δ
app.go 97.23% <100.00%> (+<0.01%) ⬆️
fxevent/event.go 100.00% <ø> (ø)
fxevent/zap.go 100.00% <100.00%> (ø)
module.go 100.00% <100.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@JacobOaks JacobOaks marked this pull request as ready for review October 2, 2023 18:34
Copy link
Contributor

@tchung1118 tchung1118 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. I just have one comment.

module.go Outdated Show resolved Hide resolved
@JacobOaks JacobOaks force-pushed the joaks/module_stack branch 2 times, most recently from 29707a2 to 52ccde7 Compare October 3, 2023 22:34
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.
module.go Outdated Show resolved Hide resolved
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()},
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point - I updated to pass depth 2, since we skip 1.

@JacobOaks JacobOaks merged commit 1a5cc04 into uber-go:master Oct 17, 2023
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants