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.

Currently, passing fx.WithLogger to an fx.Module throws an error like this:

[Fx] ERROR              Failed to start: fx.WithLogger Option should be passed to top-level App, not to fx.Module

This isn't ideal because a library author that exports their app as an fx.Module cannot specify a custom logger whose scope applies to just their Module.

This tracks extending the current implementation of WithLogger option so that it can be passed directly to an fx.Module, such that when it is, it only affects the scope of that Module.

Internal Ref: GO-1488

I'm currently working on reusable modules for common infrastructure like database, MQ, ID provider, etc and these modules may also depends on other sub-modules, such as a config server.

Ideally, I'd like to declare my modules as follows:

package main

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

type ConfigSrvr struct{}

func NewConfigSrvr() *ConfigSrvr {
	return new(ConfigSrvr)
}

var ModConfig = fx.Module("config", fx.Provide(NewConfigSrvr))

type DbConn struct{}

func NewDbConn(_ *ConfigSrvr) *DbConn {
	return new(DbConn)
}

// ModDb requires *ConfigSrvr
var ModDb = fx.Module("db", fx.Provide(NewDbConn), ModConfig)

type WebApp struct{}

func NewWebApp(_ *ConfigSrvr, _ *DbConn) *WebApp {
	return new(WebApp)
}

// ModWebApp requires *ConfigSrvr and *DbConn.
// But ModWebApp developer is not aware *ConfigSrvr is already "imported" by ModDb 
var ModWebApp = fx.Module("webapp", fx.Provide(NewWebApp), ModDb, ModConfig)

func main() {
	fx.New(
		ModWebApp,
		fx.Invoke(func(_ *WebApp, s fx.Shutdowner) {
			fmt.Println("Web app running")
			s.Shutdown()
		}),
	).Run()
}

Currently, if I declare my modules as previously mentioned, I'll get an "already provided" error, which is not ideal because the user of the database and oidc modules would not be aware of it's dependencies.

The framework, could keep track of the modules that have already populated the container and skip it if already processed.

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

I'm building a GTK app (using gotk4), using fx to create some of the Views and their backing models, and letting fx figure out hierarchy of dependencies for everything (view gets data from model, model gets data from database, database needs outstanding migrations run first, etc).

Unfortunately the Views (and other things) can only be safely (ie without panicing) constructed after the GTK subsystem has called the ConnectActivate callback of my gtk.Application. This callback is called when I call app.Run(...).

Describe the solution you'd like

Good question. I know I could spin up a separate fx.App inside my ConnectActivate callback, but I would lose all the rest of my wiring, and would have to figure out logging again, etc.. so I don't really want that.

Maybe I want to be able to be passed an invoker of sorts:

func NewApp(l *zap.Logger, invokerCalledOnActivate fx.Invoker) *gtk.Application {
  gtkApp := ...
  gtkApp.ConnectActivate(func() { 
    invokerCalledOnActivate.Invoke()
 }
return gtkApp
}

// NewUIDuringActivation only gets resolved by fx in the ConnectActivate callback above.
func NewUIDuringActivation(gtkApp *gtk.App, myTree *gtk.TreeView, myModel *gtk.TreeModel) InvokerCalledOnActivate { 
  invoker := &defaultInvoker {
   // member variables so that Invoke() eventually runs
   gtkApp.Attach(myTree)
  ...

 return invoker
}

..but that feels awkward.

Maybe I want to do all my initialization an OnStart ? That also seems suboptimal since my main goal is to create some of these dependencies during my ConnectActivate, not have them registered to be called later.

Maybe I wish executeInvoke was public and accessible through fx.App ?

Describe alternatives you've considered

See above.

Is this a breaking change?

No. This would be additive.

Hi,

Can anyone help, I think I am approaching this the wrong way :-)

I am sure this has been addressed but couldn't find anything in the issues.

I have a Build function that goes ahead and constructs all my constructures (New..) - most constructors receive a "config".

My issue is that the config is populated by Viper which is built via my Cobra implementation. (called from Cobra's OnInitialize function)

So in the constructors the config is actually empty so I can't use it, I have used an approach which I don't think is the best, I have put a "Initialize()" function in places where I need to ensure that things are constructed correctly - but of course, I have to call this manually.

Is there a way or some kind of order that can construct just the Cobra / Viper stuff first and then continue once I have this loaded? So the rest of my constructors receive the "config" populated.

It is a little difficult to explain in words - so I have placed some snippets below that I think help to illustrate it better.

Here is my build function.

func (d *DI) Build(funcs ...interface{}) *fx.App {
	app := fx.New(
		provider.ProvideConfiguration(),
		provider.ProvideLogger(),

		provider.ProvideHandler(),
		provider.ProvideController(),

		provider.ProvideRepository(),
		provider.ProvideGateway(),

		provider.ProvideCollector(),

		provider.ProvidePackage(),

		// d.provideContext(),

		d.provideVersion(),

		fx.Invoke(funcs...),

		fx.WithLogger(
			func() fxevent.Logger {
				return fxevent.NopLogger
			},
		),
	)

	return app
}

This is constructed like so and actually calls into Cobra (execute)

	app := d.Build(func(lifecycle fx.Lifecycle, command *command.Command) {
		lifecycle.Append(
			fx.Hook{
				OnStart: func(ctx context.Context) error {
					// Execute command
					command.Execute()

					return nil
				},
			})
	})

And the thing that provides the configuration (There are 2 provides here, 1 is the COnfiguration which takes care of building "config")

import (
          // more imports..
	"go.uber.org/fx"
)

func ProvideConfiguration() fx.Option {
	return fx.Options(
		fx.Provide(
			configuration.NewConfiguration,
		),
		fx.Provide(func(configuration *configuration.Configuration) *config.Config {
			return &configuration.Config
		}),
	)
}

I do have access to a method, that I do call manually anyway to bootstrap the config, this is in my "configuration" file. I will just paste some of it here to illustrate it.

func (c *Configuration) InitConfig(cfgFile string, dataDir string) error {
	if cfgFile != "" {
		// Return an error if the config file being passed could not be found
		if _, err := os.Stat(cfgFile); errors.Is(err, os.ErrNotExist) {
			return ErrConfigFileNotFound
		}

		// Return an error if the wrong config extension is being passed
		if ext := filepath.Ext(cfgFile); ext != ".yaml" && ext != ".yml" {
			return ErrConfigTypeUnsupported
		}

		viper.SetConfigFile(cfgFile)
	} else {
		var (
			configPath      = c.DefaultConfigDirectory()
			configName      = "config"
			configExtension = "yaml"
		)

		configFullPath := path.Join(configPath, fmt.Sprintf("%s.%s", configName, configExtension)

I basically call this an event I get from Cobra

	cobra.OnInitialize(func() {
		err := c.configuration.InitConfig(cfgFile, dataDir)
		if err != nil {
			log.Fatalf("could not get current directory, err: %v", err)
		}

		c.logger.Initialize()

		configMask, _ := masker.Struct(c.config)
		log.Debugf("configuration loaded, config: %+v", configMask)
	})

Hi, I found this project, and it looks interesting. I have no clue how to use it, though. It would be nice to have some content in the readme or some examples.

I'got a couple of use cases when I need to pass context.Context to the contractor function.

For example in one of the project I'm currently working on we have a function to establish a connection with a database, which uses a backoff retry strategy under the hood. The retries are controlled via context. If the context is cancelled, the function which is being retried will exit.

Similarly, we've got an authenticator component, which has to load a jwk set via http. It has a similar retry function to handle possible http failures.

Is there any way to pass/or use application start context in the constructor?

Below is some simplified function to demonstrate the issue:

func  NewAuthenicator(ctx context.Context, jwksURL string, log logr.Logger) (*Authenticator, error) {
	const maxRetries = 10

	var jwkSet  jwk.Set

	backoff := retry.WithMaxRetries(maxRetries, retry.NewExponential(time.Second))
	retryFn := func(ctx context.Context) error {
                var err error

		log.Info("fetching jwk set", "url", jwksURL)
		
		jwkSet, err = jwk.Fetch(ctx, jwksURL)
		if err != nil {
			return retry.RetryableError(err) 
		}

		return nil
	}

	if err := retry.Do(ctx, backoff, retryFn); err != nil {
		return nil, err
	}

	return Authenticator{jwkSet: jwkSet, log: log}, nil
}