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

Extendable headers field in communication (REQ, EVENT) #6

Open
icebob opened this issue Feb 28, 2021 · 0 comments
Open

Extendable headers field in communication (REQ, EVENT) #6

icebob opened this issue Feb 28, 2021 · 0 comments
Labels
Change: Breaking Module: Core Stage: 3 Delegated, it will be implemented in the next version

Comments

@icebob
Copy link
Member

icebob commented Feb 28, 2021

  • Start Date: 2021-02-28
  • Target Version: (0.15)
  • Protocol version: v5
  • Reference Issues: -
  • Implementation PR:

These changes affect breaking change in the protocol as well.

Summary

Currently, there is 2 payload fields in the communication packets, params / data and meta. The first mentioned stored user-defined data or user-input from one service to another service. The meta stored business logic data and transferred it to all services and vice-versa in a request chain.
However, sometimes the middlewares want to add new transferrable fields to the protocol (which reaches only the targetted service (in other words the lifetime is only the current request), but it's not available. It means, if we want to improve the e.g. tracing, timeout, retry middlewares, our hands are tied because every new field adding causes breaking change in the protocol and in the core module.

Why not add the new field to the ctx.params?
We can't because the params is processed by a user-created service, and not by the middlewares. I know, the middleware can wrap the localAction, get certain properties and remove them from params, but it's hacking.... not very beautiful.

Why not add the new field to the ctx.meta?
As I mentioned, the meta is transferred to all services in the chain and with the merged changes it comes back to the caller service. IF you want to add an extra tracing ID to the request, it won't be good, because other services will receive it, not just the target service of the request.

I suggest a new headers field in the communication in the REQ, EVENT and in the Context class. The functionality is the same as the HTTP request/response headers. It can contain predefined or custom key-value pairs.

This new field is able to improve the flexibility of the communication protocol. In the future, we can extend it without protocol breaking change.

Detailed design

The headers field is similar to meta it contains business logic data and not user-input data. So you can use it to bypass authentication/authorization, disable cache finding,

The headers will be available inside the services via ctx.headers. It means the developer can use it to transfer extra meta information to the target service. Therefore I suggest the (generally known) $ (dollar sign) to sign the internal header keys. So it won't collide with the user-defined header keys.

Protocol changes

We should move some existing middleware-used fields that are already defined in separated fields in the packets (e.g. timeout, stream, requestID, tracing information) into the headers.

Current REQ packet

{
  "ver": "4",
  "id": "41238213-da6b-4313-9909-e6edd0e40a96",
  "sender": "nodeID-1",
  "action": "greeter.hello",
  "params": {},
  "meta": {},
  "timeout": 10000,
  "level": 1,
  "tracing": null,
  "parentID": null,
  "requestID": "41238213-da6b-4313-9909-e6edd0e40a96",
  "caller": null,
  "stream": false
}

New REQ packet

{
  "ver": "4",
  "id": "41238213-da6b-4313-9909-e6edd0e40a96",
  "sender": "nodeID-1",
  "action": "greeter.hello",
  "params": {},
  "meta": {},
  "headers": {
    "$timeout": 10000,
    "$sampled": true,
    "$traceID": "41238213-da6b-4313-9909-e6edd0e40a96",
    "$spanID": "41238213-da6b-4313-9909-e6edd0e40a96",
    "$parentSpanID": "41238213-da6b-4313-9909-e6edd0e40a96",
    "$streamFields": {}
  },
  "level": 1,
  "caller": null,
  "stream": false,
}

Current EVENT packet

{
  "ver": "4",
  "id": "e102630b-c702-4ff9-a0a1-52428395d57a",
  "sender": "nodeID-1",
  "event": "some.test",
  "data": {
    "name": "John"
  },
  "groups": ["greeter"],
  "broadcast": false,
  "meta": {},
  "level": 1,
  "tracing": null,
  "parentID": null,
  "requestID": "e102630b-c702-4ff9-a0a1-52428395d57a",
  "caller": null,
  "needAck": null
}

New EVENT packet

{
  "ver": "4",
  "id": "e102630b-c702-4ff9-a0a1-52428395d57a",
  "sender": "nodeID-1",
  "event": "some.test",
  "data": {
    "name": "John"
  },
  "groups": ["greeter"],
  "broadcast": false,
  "meta": {},
  "level": 1,
  "headers": {
    "$sampled": true,
    "$traceID": "41238213-da6b-4313-9909-e6edd0e40a96",
    "$spanID": "41238213-da6b-4313-9909-e6edd0e40a96",
    "$parentSpanID": "41238213-da6b-4313-9909-e6edd0e40a96",
  },
  "caller": null,
  "needAck": null,
}

Usage in service code as consumer

module.exports = {
    name: "posts",

    actions: {
        async get(ctx) {
            const followers = await ctx.call("followers.list", ctx.params.id, {
                //It's the ctx calling options
                meta: {},
                headers: {
                    byPassAuth: true
                }
            })
        }
    }
}

Usage in service code as producer

module.exports = {
    name: "posts",

    actions: {
        async insert(ctx) {
            const post = await this.adapter.insert(ctx.params);

            if (ctx.headers.sendNotification === true) {
                ctx.broadcast("post.added", post);
            }

            return post;
        }
    }
}

Usage in middleware

module.exports = {
    name: "TracingMiddleware",

    localAction(next, action) {
        return ctx => {
                const span = ctx.startSpan(spanName);
                ctx.headers.$traceID = span.traceID;
                ctx.headers.$spanID = span.id;
                ctx.headers.$parentSpanID = span.parentSpanID;
                ctx.headers.$sampled = span.sampled;
        };
    }
}

Further changes

Separated tracing information from context IDs

There is some use-case where the parent tracing span comes from an external service (e.g. load balancer or browser). In this case, the parentSpanID exists and should be used. Currently, it's not easy, because the parent handling is inside the Context class. Therefore, it would be better to detach the Tracing middleware fields from the Context class and move the tracing fields in the REQ and EVENT into the new headers field. After that, anybody can write a custom tracing module and can transfer additional data via transporters (without any breaking change). It means

Move timeout fields into headers

The Timeout middleware uses the timeout field in the protocol. We should move it into the headers.

Add stream file upload extra fields to the headers

Currently it's a problem that we can't add extra information when sending streaming data to a service. With the headers it can be solved (hopefully).

Unresolved questions

What about caching keys?

Should we take into account the headers key-value pairs in the caching keys? If we allow the developer to use the headers in the services, they can use to send data which affects the results of the request. In this case, the cache key generator should able to read keys from the headers as well.

E.g.:

module.exports = {
    actions: {
        list: {
            cache: {
                keys: [
                    // From `ctx.params`
                    "limit",
                    "offset",
                    // From `ctx.meta`
                    "#tenantID",
                    // From `ctx.headers`
                    "@withPopulates"
                ]
            }
        }
    }
}
@icebob icebob added Stage: 3 Delegated, it will be implemented in the next version and removed Stage: 1 Only proposal labels Feb 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Change: Breaking Module: Core Stage: 3 Delegated, it will be implemented in the next version
Projects
None yet
Development

No branches or pull requests

1 participant