Intercept provided values

[asmeikal]

Describe the solution you'd like
We need to intercept the provided values after they are constructed, to check for implemented interfaces and possibly call some initialization methods.

We have an implementation ready that adds a WithProvidedCallback option to the container that serves this purpose. The callback will receive all values after they are constructed, along with name/group information and type information.

Describe alternatives you've considered
This could be implemented as a ProvideOption, but would need a list of values as input of the callback, since constructors can return multiple values, and would be duplicated each time Provide is called on the container.

The callback could be implemented also as a ScopeOption to override behaviour in nested scopes, but since it would be the first actual option of this kind, we skipped implementation for now.

Is this a breaking change?
No, even if the callback is implemented as a ScopeOption.

Additional context
We are open sourcing an extremly opinionated dependency injection framework we use to develop microservices in Go. This (small) framework handles automatically health check registration, calling initialization and destruction methods at service startup and destruction, registration of prometheus metrics. Instead of relying on lifecycle hooks like Fx does, our framework relies on interfaces implemented by the provided values, e.g. by looking for the Start method on provided values and invoking it at application startup. This reduces code duplication, as most of this shared logic can be implemented with single methods on each component.

[JacobOaks]

Hi @asmeikal, thanks for the issue & PR.

Dig already supports a callback system where you can register dig.Callback using dig.WithProviderCallback and dig.WithDecoratorCallback.

What are your thoughts on extending this system to support the needs you described? It seems like, specifically, you'd like:

• To be able to specify a default callback once for an entire Fx application
• Some additional information to get passed to callbacks.

I think both of these things can be accomplished in a backwards-compatible way without creating a second callback system. What do you think?

[asmeikal]

Hi @JacobOaks, as mentioned in the initial message I wouldn't rely on the existing options, since they are registered at the provider level and would result in a strange interface.

If you are suggesting instead to rely on the dig.Callback for this new option, I'm not sure how to add the required information in the dig.CallbackInfo struct, since it contains information about the provider or decorator invocation, and the feature we are looking for is to get information on the constructed values instead.

I'm having these doubts, specifically:

• the dig.CallbackInfo can contain an error, but for this feature it would never be invoked with an error, since we are only interested in what values are actually constructed
• we must add information on the constructed value inside the dig.CallbackInfo struct. For providers that return multiple values, there are two choices, and both seem a bit forced to me:
  • invoke the callback multiple times, one for each constructed value, passing it the reflect.Type and reflect.Value of each constructed value
  • invoke the callback once, passing it an array of the reflect.Type and reflect.Value couples for each constructed value

[JacobOaks]

Hey @asmeikal!

I'd like to hear what other maintainers think, but I'm not necessarily opposed to creating a top-level fx.Option API that sets a default callback for an app. I'm also not opposed to adding the constructed values into CallbackInfo as slices of types and values. I think that would be generally helpful and not that hard to do. I would be hesitant to have an entirely separate, second callback system. That seems like it will get messy and confusing quick.

As for CallbackInfo containing an error, if you don't want to handle that case in your callback, is there a reason you can't simply return early if err != nil?

[tchung1118]

Before we add a whole new top-level API for registering a global callback for all providers, could you consider adding the constructed values into CallbackInfo? I think it would be a more contained change than what you're proposing and could still satisfy your requirements. For dealing with providers returning multiple results, I would suggest using a slice of reflect.Values, instead of tuples of reflect.Type and reflect.Value. You can still get reflect.Type from reflect.Value if you need to via https://pkg.go.dev/reflect#Value.Type.

[asmeikal]

Hi @tchung1118, thank you and @JacobOaks both for the feedback. I updated the PR to rely on the existing CallbackInfo struct, which now contains a slice of reflect.Values for all the values constructed by the function, following the suggestion from @tchung1118. I also updated the tests for the provider and decorator callbacks, that receive the constructed values too. I'm still adding a new option at the container level, to intercept all constructed values.

[JacobOaks]

Hey @asmeikal thanks for the update! This looks like a step in the right direction.

I do think we should stray away from having a separately stored callback at the scope level though unless there is really strong justification for it as I think it will introduce extra complexity and confusion around callbacks in general. You mention that your use-case is to build a DI framework on top of dig. Is there any reason this framework can't insert the callback w/ every call to dig.Provide and dig.Decorate? Fx for example does this exact thing: https://github.com/uber-go/fx/blob/6ddbd0359be94efe0d729b6799a942936d4374b4/module.go#L198.

Let me know your thoughts!

[asmeikal]

@JacobOaks yeah you're right! I updated the PR by removing the container callback.