First of all, thank you for this excellent library. It is idiomatic and helped us a lot in deploying our services with ease.

One thing that will be nice to have (and I would like to contribute to) is to have a way to modify objects that are already provided in the graph. As we deployed our services such patterns were required more on more.

Example:

We wanted to add Open Census metrics and trace to our application. This can be done by wrapping the http handler in Open Census's http handler. Currently we modify the handler provider/invoker function to check if open census is enabled and wrap the http handler to return an Open Census enable http handler. If we had a way to modify provided objects especially interfaces i.e. provide a wrapped implementation things like tracing caching auth can be separated into their own layer.

This can be implemented without any breaking changes. It will require a new API, something like, fx.Modify(...) which will take inputs from providers and return providers that were already in the graph. Modifiers will only be applied if types returned by them have been requested by invokers.

What do you think of this?

fx.OnStart reduces boilerplate when adding a function to be run when the application is started.

It reduces boilerplate like:

fx.Invoke(
    func(lc fx.Lifecycle, a *Type1, b *Type2) {
        lc.Append(fx.Hook{
            OnStart: func(ctx context.Context) error {
                doSomething(a, b)
            },
        })
    },
)

into

fx.OnStart(
    func(a *Type1, b *Type2) {
        doSomething(a, b)
    },
)

This is a WIP implementation which implements the logic "outside" of fx, to start a discussion of whether this is what we want.

There are some details to consider:

• What happens if the graph has a context and the passed in function has a context Currently: If the first argument is a context, it's always the Start context. Any other argument as context is provided from the graph.
• What is the source of the OnStart for errors? Today it'll be a generated function, but fx should probably skip these. We might want a t.Helper() to provide fx helpers that don't pollute the stack.

This could also be implemented "internally" in fx, which would complicate the core framework but possibly provide better stacks/errors.

See T2133659 for more details.

Because we create service before the yarpc.Dispatcher it is hard for clients to get it in the constructor, they have to get it during request serving to e.g. create client.

With this change services will be able to resolve Dispatcher at construction time.

Several things still to tidy up, but seems to be working to review.

README.md is a good place to start and examples/dig/* to see how one might use it in a "real" application.

TODO list:

• [x] Thread safety
• [x] Rebrand Inject to Register

Other TODOs are now filed as issues.

#282 #283 #284 #285

Given a stack var Stack = fx.Options(A.Module, B.Module, C.Module)

What options are available to the user to either mutate or provide a custom A.Module. Right now the only option is to copy-pasta the stack (and therefore forego and branch the tight default version).

I'd like to think through being able to override one of the options, or ability to decorate the object right after it's creation before it's piped through the rest of the graph.

Option A: Do nothing

Declare this as SOL situation and have the service owner re-assemble the stack from the components.

This has two downsides that jump out:

• Versioning control has been transferred from the stack to the service. This can be solved by still importing the stack in the glide.yaml and picking up the right version
• More importantly: additional default components that get added to the stack are no longer picked up, meaning every now and again the service author would have to manually peek to see what's going on in the stack and sync the changes.

Option B: Override approach

It could look something like:

import "default/stack"
import "fx"

func main() {
  // MyA.Module and A.Module provide the same types
  fx.New(stack.Stack, MyA.Module).Run() 
}

And would require Fx to understand which option is providing which types.

Option C: Decoration

This could be a new option that functions very similar to Invoke, however to solve the siblings problem it gets executed first before the rest of the constructors.

import "default/stack"
import "fx"

func main() {
  // <- A.Type is what's provided by A.Module
  fx.New(stack.Stack, fx.Decorate(func(a A.Type) { 
    a.mutate = true // ...
  }.Run()

I was working on writing a new Module for another project that also uses zap, and I noticed that FX currently uses zap on master, which is way behind the latest zap development on dev. I think this was on purpose (ie I think the whole purpose of the master branch on zap is so that FX could develop against a stable zap API), but as you approach 1.0 for FX, I think getting the latest updates would be a really good idea, so that users of FX are using zap as it will be going forward.

This also does a global glide update, but I think this is the only breaking change. It would be a good idea to audit the rest of the dependencies for updates before FX goes to 1.0 as well. If this is merged, it would be nice to just merge zap dev into master, and then have this depend on master again.

A few notes:

• This is a half hack job. I am just learning the workings of FX, and this should be a drop-in replacement, but I worked to get this just passing tests. This could use a fine-tooth comb by one of the FX maintainers, and I left some TODOs in the code as it is now.
• I think LogBuilder could use some general work afterwards, but I know you're all pushing for 1.0 so this is for later.
• Instead of using LogConfiguration, it might be nice to directly use zap's new configuration struct.
• A general wish: I wish FX wasn't so tightly tied to zap, I wish we could have other logging libraries as well (and a lot of users will want this). It would be non-trivial but not too difficult to separate the tight coupling to zap.
• This PR was a pain in the butt, and if you want to merge it, we should go over it NOW and get it in before this gets into rebase world. I'm happy to do a video chat to get this done quickly, or if someone else wants to take it over, go for it.

@sectioneight @akshayjshah @glibsm @anuptalwalkar

For yaml files, it feels more natural to have:

logging:
  text_formatter: true

Instead of:

logging:
  textFormatter: true

With how the config reads yaml files right now, the latter will work, while the former will not, unless you have explicit yaml tags (right?). Could we make this magical to do either/or? I haven't looked into it at all, so it may take a change to the yaml library.

Also something to note that you might want to use: https://github.com/peter-edge/pkg-go/blob/master/yaml/pkgyaml.go#L55

What this allows me to do is to have either yaml or json files, and do the same thing. This also makes parsing work if there are only json tags without yaml tags, which gets REALLY useful when you have protobuf/thrift entities like

message Config {
  bool stdout = 1;
}

And you can just have protobuf/thrift generate the structs, which (at least for protobuf) have json tags already, and then read them in using that function.

Adds sentry integration for ulog.

Can be configured out-of-the-box with defaults by just providing a logging.sentry.dsn in config, or for those wanting more control there is a way for that too.

It's very easy for users to make one of their params types have unexported fields which means it will silently have the default value.

It seems reasonable to me to to say that a struct being used as a dig.In struct should not be used for anything else. This means that we can warn users if any fields are unexported and improve this silent rough edge users can easily hit.

This PR sketches out one approach to making stacks composable. It introduces a Group type (bikeshed away), that let modules transparently provide multiple types or offer an all-in-one starter.

This PR upgrades UberFX to use the release candidate of zap 1.0. Along the way, it migrates FX to use the base zap logger (what you were calling the typed logger). It includes a few potentially controversial changes, which I'll address head-on here. I'm happy to chat about any of this in person on Monday.

(Clearly, this can't get merged until after your code freeze. That's cool, I'd just like to get review done before I'm constantly rebasing.)

No ulog.Log interface

Interfaces are good for a few things:

• depending on a subset of a type's methods
• allowing callers to pass third-party types without depending on those external packages
• abstracting away implementation, which lets you swap implementations later

However, the former ulog.Log interface doesn't accomplish any of these goals. It would have included the full zap.SugaredLogger API, and it doesn't prevent UberFX (and ulog) from taking a concrete dependency on zap. Most importantly, because we need users to be able to recover the strongly-typed *zap.Logger, it references tons of concrete zap types (*zap.Logger, which then references Field and *CheckedEntry). It's hard for me to imagine a reimplementation of that API outside zap.

That might still be okay, but interfaces are also harder to extend - adding even a single method is a breaking change. Given all that, it seems wiser to me to use *zap.Logger directly and eliminate a bunch of copypasta. Since we own both libraries, I don't see this as a significant problem. - users can and should declare narrow interfaces for just the portions of the logger that they're using.

No Sentry integration

This is purely a time issue. If you can't wait for the Filebeat plugin we've discussed, I'm happy to resurrect direct Sentry integration in a follow-on PR. The latest zap should allow us to do that without so many map copies and with zero impact on non-error log performance.

No loggers in contexts

I was a little confused by the existing code. It looks to me like we're largely working with a single global logger, which starts off with some default configuration and is reconfigured on service startup. We're supplying that logger to users by putting it into the request context and providing ulog.Logger(ctx context.Context) ulog.Logger as a getter. Where possible, we add tracing information to the logger before putting it into the context.

In all that, I'm not sure whether the logger needs to be in the context at all. We can keep ulog.Logger, and add just add the context's tracing information to the logger when it's retrieved. This is quite inexpensive, and it's much simpler.

No log builder

I don't think that this is required any more either; we can just use zap's config shapes instead. They already come with sensible default values, and we can modify the service scaffold to comply with our internal logging schemas.

Documentation

I didn't touch the ulog README much - I'll leave that for someone on your team.

Describe the bug I am trying to generate a wasm build with fx:

GOOS=js GOARCH=wasm go build -o ../../dist/backend.wasm main.go                                 2 ↵  7109  21:33:33
# go.uber.org/fx
/Users/dzou/go/pkg/mod/go.uber.org/[email protected]/shutdown.go:104:13: undefined: _sigTERM
/Users/dzou/go/pkg/mod/go.uber.org/[email protected]/signal.go:117:42: undefined: _sigINT
/Users/dzou/go/pkg/mod/go.uber.org/[email protected]/signal.go:117:51: undefined: _sigTERM

Maybe relate to https://github.com/golang/go/issues/28719

To Reproduce use fx in any project and build with GOOS=js GOARCH=wasm

Expected behavior It should support wasm build.

Additional context Add any other context about the problem here.

Describe the bug Regression: It's impossible to shutdown from an OS signal after startCtx is Done().

To Reproduce

1. Start an fx app
2. Wait startTimeout (default is 15s).
3. Send SIGTERM to the process
4. Nothing will handle the signal and the app will not shut down

Expected behavior The apps shut down correctly like it did in the previous release.

Additional context This line starts the goroutine that dispatches os signals. https://github.com/uber-go/fx/blob/213eb869667072f220b8224d16852a81897f6011/signal.go#L118 The goroutine then blocks on https://github.com/uber-go/fx/blob/213eb869667072f220b8224d16852a81897f6011/signal.go#L82-L95

After the startCtx is Done(), this goroutine exits and thus there's nothing to process OS signals to initiate shutdown.

Replacing the goroutine invocation by go recv.relayer(context.TODO()) fixed the issue for me. The context passed here should not be Done() until the application stops.

Describe the bug The fx.Dotgraph displayed when visualizing the dependency graph is incorrect when using the new fx.Private variable.

To Reproduce For the following code it prints as expected "foo\nbar" meaning ModuleB has a dependency on ModuleA and ModuleC has a dependency on AnotherModuleA.

package main

import (
	"fmt"
	"go.uber.org/fx"
)

type A struct{ text string }
type B struct{ a *A }
type C struct{ a *A }

func NewA() *A           { return &A{"foo"} }
func AnotherNewA() *A    { return &A{"bar"} }
func NewB(a *A) *B       { return &B{a: a} }
func NewC(a *A, b *B) *C { return &C{a: a} }

var ModuleA = fx.Provide(NewA)
var AnotherModuleA = fx.Provide(fx.Annotate(AnotherNewA, fx.ResultTags(`name:"anotherA"`)))
var ModuleB = fx.Provide(NewB)
var ModuleC = fx.Provide(NewC)

func main() {
	fx.New(
		ModuleA,
		ModuleB,
		AnotherModuleA,
		fx.Module(
			"submodule",
			fx.Provide(
				fx.Annotate(func(a *A) *A {
					return a
				}, fx.ParamTags(`name:"anotherA"`)), fx.Private),
			ModuleC,
		),
		fx.Invoke(func(b *B, c *C, g fx.DotGraph) {
			fmt.Println(b.a.text)
			fmt.Println(c.a.text)
		}),
	)
}

Yet the fx.DotGraph printed looks like this where the incorrect edge is highlighted in red.

Expected behavior My expectation is the dotgraph look like this again the corrected edge is highlighted in red.

Additional Context While this bug is a subtle point, the visualization helps a lot when using new features like fx.Private to determine usage is correctly understood and seeing an incorrect visualization creates confusion and makes the user think they are the ones doing something wrong.

Is your feature request related to a problem? Please describe.

This is a documented limitation, but it's somewhat unclear why such a limitation exists:

In the Annotated struct:

// ...
// A name option may not be provided if a group option is provided.
Name string

// ...
// A group option may not be provided if a name option is provided.
Group string

It seems reasonable that a constructor's output could be both annotated with a name as well as exist within a group.

Describe the solution you'd like A clear and concise description of what you want to happen.

The ability to use a single tag, specifying name and group, ie:

fx.Provide(
    fx.Annotate(
        NewMemoryStorageStrategy,
        fx.As(new(StorageStrategy)),
        fx.ResultTags(`name:"memory" group:"storage"`),
    ),
    fx.Annotate(
        NewFileStorageStrategy,
        fx.As(new(StorageStrategy)),
        fx.ResultTags(`name:"file" group:"storage"`),
    ),
)

Which would allow for injection of a group of StorageStrategies:

type StorageExpirationMonitor struct {
    strategies []StorageStrategy 
} 

func NewStorageExpirationMonitor(strats []StorageStrategy) *StorageExpirationMonitor {
    return &StorageExpirationMonitor{
        strategies: strats,
    }
}

func (smm *StorageExpirationMonitor) ExpireAll() {
	for _, s := range smm.strategies {
		fmt.Printf("Expiring: %T\n", s)
	}
}

fx.Provide(
    // ...
    fx.Annotate(
        NewStorageExpirationMonitor,
        fx.ParamTags(`group:"storage"`),
    ),
    // ...
)

while also being able to control (at the module) which StorageStrategy is used for other implementations, ie:

type ResultCache struct {
    storage StorageStrategy 
}

func NewResultCache(storage StorageStrategy) *ResultCache {
    return &ResultCache{
        storage: storage,
    }
}

fx.Provide(
    // ...
    fx.Annotate(
        NewResultCache,
        fx.ParamTags(`name:"file"`), // perhaps we have a separate test module that leverages name:"memory"
    ),
    // ...
)

Of course, it's possible there may be a major problem case with allowing this functionality that I am missing. Apologies if that is the case.

Describe alternatives you've considered A clear and concise description of any alternative solutions or features you've considered.

This type of functionality could be implemented on the user side by adding a StorageType() string func to StorageStrategy which could be used to collect all of the StorageStrategy instances in a manager type (using group:"storage"), and having the manager provide a ByStorageType(string) StorageStrategy method. Then inject the "manager" into any implementations that require a specific strategy type. ie: Create the "Provider" ourselves and write a simple scheme for retrieving instances by index/type.

So, this isn't a must-have issue, and these type scenarios aren't too frequent, but there are some conveniences this feature would provide that would improve quality of life.

Is this a breaking change? We do not accept breaking changes to the existing API. Please consider if your proposed solution is backwards compatible. If not, we can help you make it backwards compatible, but this must be considered when we consider new features.

Since this appears to be an intentional restriction, I assume that allowing this functionality would be backwards compatible.

Additional context Add any other context or screenshots about the feature request here.

Thank you for an amazing open source project!

Adds the fx.ShutdownError option, to be able to pass a custom error to Shutdowner and retrieve it from the application as the reason it shutdown.

Refers #672

Describe the bug The fx project use fmt.Errorf("%v", err) instead of fmt.Errorf("%w", err) like here. This prevent from using errors.Unwrap(), errors.Is() and errors.As() functions in custom logger. Also this hide true errors and make error message unclear.

To Reproduce

type Error struct {
    Message string
    Err error
}

func (e *Error) Error() string {
    return e.Message 
}

func (e *Error) Unwrap() error {
    return e.Err
}

fx.New(
    fx.Invoke(
        func() error {
            return &Error{
                 Message: "Error A",
                 Err: &Error{
                     Message: "Error B wrapped in error A",
                 },
            },
        },
    )
).Run()

• Error value provided by the fx in logger will contain only fx bla bla: ble ble: Error A without containing the message Error B wrapped in error A
• It always returns single error value without any wrapped errors. Missing error context
• Using errors.Unwrap(), errors.Is() or errors.As() is worthless

Expected behavior

Error provided by the fx should support wrapped errors without loosing error context.