dapr/dapr

Should we pass go context.Context to all the components?

Open

#2,716 opened on Jan 26, 2021

View on GitHub
 (12 comments) (7 reactions) (0 assignees)Go (25,672 stars) (2,064 forks)batch import
P2good first issuesize/Striaged/resolved

Description

In golang, context.Context is widely used and it's always be the first argument of go method and passed everywhere.

In dapr current implementation, for example, pkg/grpc/api.go, all the public method called by gRPC have the context object, and some them (service incation, actor ) pass context in the next step (always calling the component):

func (a *api) CallLocal(ctx context.Context, in *internalv1pb.InternalInvokeRequest) (*internalv1pb.InternalInvokeResponse, error) {
	callAllowed, errMsg := a.applyAccessControlPolicies(ctx, operation, httpVerb, a.appProtocol)
	resp, err := a.appChannel.InvokeMethod(ctx, req)
}

func (a *api) CallActor(ctx context.Context, in *internalv1pb.InternalInvokeRequest) (*internalv1pb.InternalInvokeResponse, error) {
	resp, err := a.actor.Call(ctx, req)
}
func (a *api) InvokeService(ctx context.Context, in *runtimev1pb.InvokeServiceRequest) (*commonv1pb.InvokeResponse, error) {
	resp, err := a.directMessaging.Invoke(ctx, in.Id, req)
}
func (a *api) GetActorState(ctx context.Context, in *runtimev1pb.GetActorStateRequest) (*runtimev1pb.GetActorStateResponse, error) {
	resp, err := a.actor.GetState(ctx, &req)
}

And some of them (pub-sub / binding / state ) don't pass context:

func (a *api) PublishEvent(ctx context.Context, in *runtimev1pb.PublishEventRequest) (*emptypb.Empty, error) {
	err = a.pubsubAdapter.Publish(&req)
}
func (a *api) SubscribeEvent(srv runtimev1pb.Dapr_SubscribeEventServer) error {
	err := curPubsub.Subscribe(req, func(msg *pubsub.NewMessage) error 
}
func (a *api) InvokeBinding(ctx context.Context, in *runtimev1pb.InvokeBindingRequest) (*runtimev1pb.InvokeBindingResponse, error) {
	resp, err := a.sendToOutputBindingFn(in.Name, req)
}
func (a *api) GetState(ctx context.Context, in *runtimev1pb.GetStateRequest) (*runtimev1pb.GetStateResponse, error) {
	getResponse, err := store.Get(&req)
}

So, there are big difference between components.

For example, actor api, all the method start with context argument :

type Actors interface {
	Call(ctx context.Context, req *invokev1.InvokeMethodRequest) (*invokev1.InvokeMethodResponse, error)
	Init() error
	Stop()
	GetState(ctx context.Context, req *GetStateRequest) (*StateResponse, error)
	TransactionalStateOperation(ctx context.Context, req *TransactionalRequest) error
	GetReminder(ctx context.Context, req *GetReminderRequest) (*Reminder, error)
	CreateReminder(ctx context.Context, req *CreateReminderRequest) error
	DeleteReminder(ctx context.Context, req *DeleteReminderRequest) error
	CreateTimer(ctx context.Context, req *CreateTimerRequest) error
	DeleteTimer(ctx context.Context, req *DeleteTimerRequest) error
	IsActorHosted(ctx context.Context, req *ActorHostedRequest) bool
	GetActiveActorsCount(ctx context.Context) []ActiveActorsCount
}

But state api, no context at all:

type Store interface {
	BulkStore
	Init(metadata Metadata) error
	Delete(req *DeleteRequest) error
	Get(req *GetRequest) (*GetResponse, error)
	Set(req *SetRequest) error
}

type BulkStore interface {
	BulkDelete(req []DeleteRequest) error
	BulkGet(req []GetRequest) (bool, []BulkGetResponse, error)
	BulkSet(req []SetRequest) error
}

I suggest to pass the context to all the components in golang style.

In fact, now I'm working to add tracting support in our state and bindings component, and I find that I can't get the context from the public API method of dapr gRPC service, I have to find other way (using metadata, again, we use metadata to do almost everything that dapr don't support) to pass the tracing information like traceid.

Contributor guide

Should we pass go context.Context to all the components? · dapr/dapr#2716 | Good First Issue