WIP: id and child logger for request and response

[delvedor]

As titled.
Is still a WIP because there are some points that we should discuss.

We are generating an id for every request that will be used also in the response, that id will be used also in the child logger. This is nice, because it will give us a tracker of what is happening to a specific request/response in the cluster, but what strategy we can use to keep that id unique? As you can see for the moment I used a very naive id generation function.

Almost all the test will fail because the response object will have the id and log fields.
Do we need to change the structure of the response object?
Something like:

{
  key: String,
  id: Any,
  payload: Object, // the actual user message
  log: Object
}

[mcollina]

I'm not ok with this change. Most of UpRing was built to move through basic objects.
The moment we start adding custom properties, we would be better off with a proper Context object.

BTW, tentacoli has its own id: https://github.com/mcollina/tentacoli/blob/master/tentacoli.js#L282, maybe you can reuse that?

[delvedor]

Most of UpRing was built to move through basic objects.
The moment we start adding custom properties, we would be better off with a proper Context object.

I'm not sure I've understood this part, can you rephrase?

BTW, tentacoli has its own id: https://github.com/mcollina/tentacoli/blob/master/tentacoli.js#L282, maybe you can reuse that?

I've considered that, but I've chosen to go for this solution because it allows us to change the underlying transport method without the need to change the id handling as well.

[mcollina]

The current API is well design to have no custom properties attached to messages. You send a message (JS), and this comes out on the other side. Attaching metadata (like the id or the log), is against that pattern.

Either we change to a completely different solution, so that we have a Context object that wraps the message, the callback etc, or we stay as it is right now. An hacked solution is not going to work well.

[lucamaraschi]

give us a tracker of what is happening to a specific request/response in the cluster

Given the above mentioned requirement, what's the best way to achieve it then?I guess the goal is to achieve something similar to what TChannel is achieving within Ringpop, traceability as first class resident.

[delvedor]

Either we change to a completely different solution, so that we have a Context object that wraps the message, the callback etc, or we stay as it is right now. An hacked solution is not going to work well.

@mcollina that is what I was proposing here:
Do we need to change the structure of the response object?
Something like:

{
  key: String,
  id: Any,
  payload: Object, // the actual user message (in other words, the old request object)
  log: Object
}

[delvedor]

Hi folks, let's resume this discussion.
If you are ok, I propose to change the message structure to

{
  key: String,
  id: Any,
  payload: Object, // the actual user message (in other words, the old request object)
  log: Object
}

In this way we will be more future proof and enable a nice request tracing.

[mcollina]

I am still -1.
However you can attach the properies via a symbol, so they stay hidden.

[delvedor]

If I may, why are you 👎 ?

[lucamaraschi]

What about the context object then?

[mcollina]

I would like to keep equality between a request and the incoming object.

upring.on('request', (something, reply) => {
  reply(null, {
    a: 'response',
    streams: {
      any: stream
    }
  })
})
upring.request(something)

Both tentacoli and upring are built on that assumption. It might not be the correct one, but this change just causes too much disruption.
If that was direction we would like to go, this change has to start from tentacoli itself, and it would likely need to be rebuilt from scratch to provide a consistent API.
We probably needs it anyway, as I would really like to move this to use HTTP2 as a transport mechanism.

Also, I'm not convinced that pattern matching is the right routing mechanism for this, as it's too complex for most people to grasp, and it is very hard to optimize.

[lucamaraschi]

I agree with @mcollina. I would use these inputs to shape the next version of Upring and evolve it into its own protocol.
I am 100% for rethinking about the usage of "pattern matching" as a routing definition.

@mcollina what about starting a new issue where we define all the requirements and we start define some action points?

[mcollina]

Go ahead and start it. Il giorno lun 29 gen 2018 alle ore 08:52 Luca Maraschi < [email protected]> ha scritto:

…

I agree with @mcollina <https://github.com/mcollina>. I would use these inputs to shape the next version of Upring and evolve it into its own protocol. I am 100% for rethinking about the usage of "pattern matching" as a routing definition. @mcollina <https://github.com/mcollina> what about starting a new issue where we define all the requirements and we start define some action points? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#27 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AADL4wYK8I97ilOOY2NMriZt0mH_6LjRks5tPU5wgaJpZM4RINtF> .