Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

contextualizing Tell #69

Closed
rogeralsing opened this issue Jan 11, 2017 · 4 comments
Closed

contextualizing Tell #69

rogeralsing opened this issue Jan 11, 2017 · 4 comments

Comments

@rogeralsing
Copy link
Collaborator

In order to fully support passing contextual information the current PID.Tell model will not work.
One suggestion is to move the Tell operation over to the actor Context.

e.g.

//instead of 
pid.Tell(msg)
//we would instead have
ctx.Tell(pid, msg)

This would allow us to build things like tracing using e.g. Zipkin.
It would also greatly simplyfy having multiple actor systems running at the same time. e.g. for parallel testing.

ctx would then be like a phone and the PID would just be the phone number.
Currently PID is both.

This would be a major breaking change, so I'm not completely sold on this idea myself, but I do see the added value it brings

@rogeralsing
Copy link
Collaborator Author

rogeralsing commented Jan 12, 2017

If we go for this approach, we also need to solve how to talk to PID's from outside an actor.
this could be done by having a Tell func directly in actor

actor.Tell(pid, msg)

That would be a "contextless" Tell.
If you have some sort of context, for example Zipkin data, and you want to call into the actor world using this context.
We could have something like this:

ctx := actor.NewRootContext( whatever context data)
defer ctx.Close()
ctx.Tell(pid, msg)

I think this would be fairly clean, even if this approach is more verbose than what we have currently.

@cpx86
Copy link
Contributor

cpx86 commented Jan 13, 2017

I'm not too familiar with how the context and PID interact yet, but from a user perspective I think it's important that there are as few ways as possible of doing something, preferably just one if possible.

Would it be a bad coupling if the PID knew about the context it belongs to? So calling pid.Tell would notify the context of the Tell operation, so it can do tracing or whatever it is it needs to do.

@rogeralsing
Copy link
Collaborator Author

rogeralsing commented Jan 13, 2017

making the PID know about the current context is possible in e.g C# using AsyncLocal and all that black magic.
It is less doable in Go as Go has its own concept of Go routines, there are hacks to peek into thread local like storage for this, but I don't want to go there.

(The same PID could flow through multiple contexts in e.g. a message, so it is not tied to the context in any way)

Another option could be to provide the context into the pid.Tell(msg, context)

that would be less intrusive on the current design, right?
the last context param might be an arglist of context providers. passing nothing would mean telling from the root context.

Just tossing around ideas here

@rogeralsing
Copy link
Collaborator Author

rogeralsing commented Jan 31, 2017

Discussed API:

context.Tell(pid, message)

We could either go for:
context.Tell(pid, message)
context.TellRaw(pid, bytes) (or somilar)

or
context.Tell(pid, Message{Object: msg})
context.Tell(pid, Message{Bytes: bytes})

Related tasks

  • Introducing a message envelope type carrying message headers, this type can also replace the current messageSender type

e.g.

type struct Message {
     Headers MessageHeaders // string -> string map
     Object interface{} 
     Bytes []byte
     Sender *PID //this could be tricky as PID lives in actor package
}
  • Make all queues carry this message envelope instead of interface{}
  • Introduce way of reading message headers from actor Context
  • Introduce outbound middleware allowing code to read from actor context and write to outgoing message headers
  • Introduce a RootContext which can be used when integrating with actors from the outside world

e.g.

func main() {
    pid := ...
    ...
    ctx := NewRootContext()
    ctx.Headers().Add("TraceID", "123") //the rootcontext can be configured with some initial values
    //or possibly, root context headers should be set on creation?

    ctx.Tell(pid, message)
}

One question we need to decide on,
Do we keep Tell on the PID for convenience? e.g. when interacting with actors from the outside.
or do we provide a default empty root context through actor.Tell or something similar?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants