modify objects already provided in the graph #653
Comments
|
Option C of #531 is more or less what I am proposing and would elegantly solve my problem. One concern seems to be that the order in which such decorators will be applied. Can't they be like Invoke, i.e. in the order provided? |
|
Any plans to implement this? |
|
Hey, we haven't had a chance to fully evaluate how this affects the rest of We may have some time to look into this in a few weeks. |
|
I am unable to understand how named values and value groups differ from the graph's perspective they are like any other value, aren't they? Could you please explain. And I suppose this should be supported by dig first. Should I open an issue there? |
|
Yes, happy to clarify! From the point of view of
I think we would have to figure out a good plan for the above and any other Yes, this would need to be implemented in dig first. We don't need to |
|
So I was thinking something more like this, Since the purpose of decorators is to modify values already available and instantiated in the graph, named values, value groups (the actual slice), and normal values all count as a unique value available in the graph. Let me elaborate with examples and try to lay out rules to better explain what I am proposing.
fx.Provide(
func (...) X {
// ...
},
func (...) F {
// ...
},
),
fx.Decorator(
func(x X, f F) F { ... },
)
fx.Provide(
fx.Annotated{Group: "foo", Target: producer1},
fx.Annotated{Group: "foo", Target: producer2},
fx.Annotated{Group: "foo", Target: producer3},
),
fx.Decorate(
func(in struct {
fx.In
Fs []F `group:"foo"`
}) struct{
fx.Out
Fs []F `group:"foo"`
} { ... },
),This is OK because currently fx does not make any guarantee in terms of order of the values so modifying them all at once should not be a problem. The first point also supports this argument as we are only trying to modify the inputs, value groups are resolved to slices when provided and hence should be modified in that form. What do you think of this? |
|
That sounds like a good plan. Decorators will reference values/named values The implementation of Decorate will almost definitely need a check which |
|
@abhinav shall I start implementing what we just discussed? |
This worries me. We can technically runtime safeguard against this, but not sure if that's something we should stick our hand in. |
|
@srikrsna We're having some active discussion about fx.Decorate ordering. I @akshayjshah @glibsm Let's discuss ordering for the case where multiple Also, @rhang-uber, do you think we would change the visualization in Dig for |
|
@srikrsna After a bit more than a year of Fx use, our internal ecosystem has a pretty deep graph of dependencies, across multiple teams (and multiple offices around the world). Many Fx modules are written by engineers who don't have the time or inclination to read all the Fx docs. Today, this works pretty well because On the other hand, this is a really valuable feature - we've discussed it multiple times now. Before you put a ton of effort into implementing this, let me think on this for a bit. |
|
Just thinking out loud here, what about Instead of dealing with the instantiated objects to modify their state, we would allow to replace the constructor that provided the type? Instead of massively complicating the dig/fx internals it would put the onus on library authors to make sure their modules use public constructors (as per our guidelines). fx.Provide(func() A { ... })
fx.Provide(func() B { ... })
fx.Replace(func () B { .. some other B function }) // errors out if B doesn't exist or signatures don't match
fx.Invoke(func (a A, b B) { ... })I'm thinking that dealing purely with constructor functions manipulation, rather than dropping down to instantiated objects may be a better road to take and should still allow the usages that Dig could queue up all replaces and run them in between Provide and Invoke. |
|
We use gRPC for developing all our services. And almost all services are written in Go. I developed a simple authorisation library (internal) that generates gRPC server implementation that check if the user has the right to access a specific rpc and call the underlying server implementation. type RightsEchoServer struct {
pb.EchoServer // interface
....
}
func NewRightsEchoServer(s pb.EchoServer, ...) pb.EchoServer {
return &RightsEchoServer{s, ...}
}
func (s *RightsEchoServer) Echo(ctx context.Context, req pb.EchoRequest) (*pb.EchoResponse, error) {
var hasRight bool
// Check Rights ...
if !hasRight {
return nil, status.Error(codes.Unauthorized, ...)
}
return s.EchoServer.Echo(ctx, req)
}Now in case of proposed fx.Provide(NewEchoServer),
fx.Decorate(NewRightsEchoServer),In case of fx.Provide(NewEchoServer),
fx.Replace(func (d D, ...) (pb.EchoServer, error) {
s, err := NewEchoServer(d)
if err != nil {
return
}
return NewRightsServer(s, ...), nil
}),And in case of multiple such decorators the code would only increase. But in case of ordered decorators it would be just adding them to the list. |
Decorator pattern itself is order dependent, isn't it? |
|
Did you guys get any time to evaluate this? |
|
Hello, @srikrsna! Sorry about the delay. We've had a busy few weeks. We discussed this a bit further. To recap: We like the idea of supporting One solution we're considering is having a way of scoping the effect of The blast radius of an We foresee need for specifying a name for this scope. (This would, for Since we've already been using the term "Fx module" everywhere, it might make Again, Would this satisfy your needs? (Sorry, just to be explicit about this: We're just spitballing ideas here. |
|
I like the idea, the example of decorators effecting other modules will definitely be a concern and really hard to track down. Limiting the scope seems like a great idea. |
|
Any update on this? |
|
We're getting these, but we've been a little underwater - sorry! Please bear with us. We'll try to come back to this issue in the next week and clarify what the next steps are. |
|
Thank you for the update and sorry for pinging so much! Looking forward to next week! |
|
If anyone is really desperate and need something similar right now, the following might work: // Replace replaces the given target with the passed value.
// The target needs to be a pointer to the interface that needs to be replaced.
func Replace(target interface{}, replacement interface{}) fx.Option {
var sh *fx.Shutdowner
shutdownerType := reflect.TypeOf(sh).Elem() // the plan is to steal the app from the shutdowner
targetType := reflect.TypeOf(target).Elem()
targetTypes := []reflect.Type{targetType, shutdownerType}
// Build a function signature that looks like:
// func(t1 targetType, shutdowner fx.Shutdowner)
fnType := reflect.FuncOf(targetTypes, nil, false /* variadic */)
fn := reflect.MakeFunc(fnType, func(args []reflect.Value) []reflect.Value {
values := args[1].
Elem().
Elem().
FieldByName("app").
Elem().
FieldByName("container").
Elem().
FieldByName("values")
values = trickReflection(values)
keys := values.MapKeys()
found := false
for _, k := range keys {
vType := values.MapIndex(k).Type()
if vType.Implements(targetType) {
values.SetMapIndex(k, reflect.ValueOf(reflect.ValueOf(replacement)))
found = true
break
}
}
if !found {
panic("Value to replace not found in the values map")
}
return nil
})
return fx.Invoke(fn.Interface())
}
// trickReflection tricks reflection to hide the fact that the value has been obtained through an unexported field
func trickReflection(v reflect.Value) reflect.Value {
return reflect.NewAt(v.Type(), unsafe.Pointer(v.UnsafeAddr())).Elem()
} |
|
Is there a decision on this? |
Hey! We generally have agreement that the design from this comment is the func Module(name string, opts ...Option) Option
func Decorate(funcs ...interface{}) OptionWhere |
That's great.
I can work on this. I should probably start with dig? |
That would be excellent! Yes, you would have to start with There are two pieces of functionality needed here:
// Module defines a named set of Fx options that collectively provide
// functionality to an application. Name must not be empty.
//
// var LoggingModule = fx.Module("logging", fx.Provide(newLogger))
//
// Modules may be nested arbitrarily deep.
//
// var ServiceModule = fx.Module("service", LoggingModule, MetricsModule)
//
// The top-level fx.New call implicitly acts as a module with an empty name.
func Module(name string, opts ...Option) OptionFor something like this, my first guess is that dig will need a concept of // Child returns a named child of this container. The child container has
// full access to the parent's types, and any types provided to the child
// will be made available to the parent.
//
// The name of the child is for observability purposes only. As such, it
// does not have to be unique across different children of the container.
func (c *Container) Child(name string) *ContainerSo perhaps child := c.Child("foo")
for _, opt := range opts {
opt.apply(child)
}Similarly, // Decorate is an Fx option that allows modifying or replacing objects in the
// container. Decorate is called with zero or more functions, each of which
// accepts some number of objects from the container and produces objects to
// replace all or some of them.
//
// fx.Decorate(
// func(rt http.RoundTripper, metrics *Metrics) http.RoundTripper {
// return &metricsRoundTripper{rt: rt, metrics: metrics}
// },
// )
//
// As with constructors, decoration may optionally fail with an error,
// causing the application startup to halt.
//
// A decoration is scoped to the innermost module in which the fx.Decorate
// call appeared. That is, an fx.Decorate call can only affect values
// produced by the module in which the call appears or its descendants. So in
// the following example, the fx.Decorate call can only affect values
// produced by the "bar" and "baz" modules.
//
// fx.Module("foo",
// fx.Provide(...),
// fx.Module("bar",
// fx.Provide(...),
// fx.Decorate(...),
// fx.Module("baz", ...),
// ),
// )
//
// Note that because the top-level fx.New is implicitly a module, an
// fx.Decorate call appearing at the top-level is able to affect any value in
// the application.
//
// fx.New(
// MetricsModule,
// LoggingModule,
// fx.Decorate(...),
// )
func Decorate(funcs ...interface{}) OptionThe corresponding API in dig is more straightforward. // Decorate allows modifying or replacing a value in the container. It
// must be provided a function f which accepts one or more values from the
// container and produces objects to replace all or some of them. f may
// return an error as its last result to indicate that decoration failed.
// f may not return any types that are not already in the container.
//
// Decoration applies to this container only. Parent containers will not
// be affected.
func (*Container) Decorate(f interface{}, opts ...DecorateOption) error(As with Provide and Invoke, we likely want to preemptively add functional So an for _, f := range fs {
if err := c.Decorate(f); err != nil {
...
}
}These two pieces of functionality can be implemented independently but we |
|
Thank you that's really helpful. I'll start working on it. |
|
@srikrsna Are you working on this or this task is up for grabs? |
|
One possible extension is to have the support for optional objects like Spring does. The idea is that the framework provides objects, but if application level code specifies the conflicting provider the framework provided object is automatically ignored. This would avoid need to call Decorate for default objects if there is no need to wrap them. The more advanced feature is to provide support for conditional objects. |
|
@mfateev I somehow missed this. I have a working implementation. We are using it inside our org for some internal deployments. I am just waiting for review. |
|
@srikrsna Do you ever plan to release this to the open source? |
|
This adds `fx.Decorate`, which lets you specify decorators to an fx app. A decorator can take in one or more dependencies that have already been `Provide`d to the app, and produce one or more values that will be used as replacements in the object graph.
For example, suppose there is a simple app like this:
```go
fx.New(
fx.Provide(func() *Logger {
return &Logger{Name: "logger"}
}),
fx.Invoke(func(l *Logger) {
fmt.Println(l.Name)
}),
)
```
Running this app will print "logger" on the console.
Now let us suppose a decorator was provided:
```go
fx.New(
fx.Provide(...), // Provide same function as above
fx.Decorate(func(l *Logger) *Logger {
return &Logger{Name: "decorated " + l.Name}
}),
fx.Invoke(...), // Invoke same function as above
)
```
The decorator here will take in the provided Logger and replace it with another logger whose `Name` is `decorated logger`. The `Invoke`d function is then executed with this replacement value, so running this app will print "decorated logger" on the console.
In terms of implementation, a decorator is represented by the target decorator function and the call stack it was provided from, similar to a provider. `module` contains a list of decorators that were specified within its scope.
The dig dependency had to be updated to the latest master branch of Dig to ensure the fix for uber-go/dig#316 is in.
Following this PR, there are two additional pieces I will be adding:
1. An eventing system for fx.Decorate.
2. fx.Replace, which takes in a value instead of a function to replace a value in the object graph. This is similar to what fx.Supply is to fx.Provide.
This PR along with the two PRs above should make the long-awaited feature of graph modifications in fx finally possible.
---
Refs #653, #649, #825, uber-go/dig#230, GO-1203, GO-736
|
Hey all, This has been implemented with |
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?
The text was updated successfully, but these errors were encountered: