NATS transporter and compatibility with moleculer-js

dehypnosis commented 5 years ago

1. Related issues

is NATS supported currently? #53 Moleculer JS compatibility #55

2. Includes

2-1. NATS transporter implementation

"nats://uri" transporter configuration shall be respected

2-2. moleculer-js compatibility and network stability

Fix event payload field to "data" from "params"
- by https://github.com/moleculerjs/moleculer/blob/master/docs/PROTOCOL.md#event-1
Fix "services" field of INFO packet
- modify actions field to map[string]interface{} from []interface{}
- modify action "name" field from "action.name" to "my.service.action.name"
- add "rawName" field to action which would like "action.name"
INFO packet shall not embed "seq", "cpuSeq" fields by https://github.com/moleculerjs/moleculer/blob/master/docs/PROTOCOL.md#info
INFO packet should return internal services as well ($node) I'm curious about the reason you hided internal services from packet. There might be the reason I do not know, but actually this commit is needed for our teams internal application, which collects all the node of cluster and gathers health status.
implement $node.health and $node.options very incompletely
make remoteNodeInfoReceived() to be able to parse "actions", "events" map[string]interfaces{} fields from INFO packet
temporarily fix checkOfflineNodes() timeout range for network stability among moleculer-js. moleculer-js set 10min as default configuration for this. It would be better other client option can be provided.
Adjust DefaultConfig.HeartbeatFrequency/HeartbeatTimeout Set these default options as 5s/15s for network stability among moleculer-go and moleculer-js nodes. Without this, moleculer-go nodes continue to be connected and disconnected from moleculer-js nodes.

2-3. Miscellaneous

Update discoverNodeID() to follow convention of moleculer-js as like "{hostname}-{integer:5}"
Prevent localNodeID being changed upon runtime. (bug)

3. Compatibility status

I made a moleculer-go service with forked version of mine with NATS transporter.

works

discovery works well and networking is stable for recent 48 hours
event subscription, emit, broadcast works well
action req/res works well

needs

event subscription with pattern matching like "my.event.**"
access context.meta from action handler
compatible error format

IT IS AMZAING!

4. Suggestion

I know you are currently working on ActionSchema thing which can validate action params. This feature is amazing and our team also using this feature for every actions.

Actually I made a API gateway (which is different from moleculer-web) and the console application which has a feature making API documents from service schema and action schema like as below. Surely this feature needs a normalized action params schema.

if action.params schema can follow icebob/fastest-validator which is included in moleculer-js, It would be great. Because I think params itself can be a kind of protocol for moleculer service schema.

5. Sorry about

I couldn't make any test cases for above commits and codes for compatibility lack deep comprehension about moleculer-go because i am currently so busy rapidly implementing our companies' services with forked version of moleculer-go.

So this PR may cannot be accepted, and it would be no problem. I just hope this PR can be even a hint for moleculer-js compatibility.

And really appreciate for your hard work!! Thanks!

slaterx commented 5 years ago

I've began to develop a fix for https://github.com/moleculer-go/moleculer/issues/54, but I will wait for your merge to be completed, so I can incorporate your NATS transporter into the work.

pentateu commented 5 years ago

@dehypnosis thank you for the contribution.

I'll have a look at the code and try to retrofit the test cases. IF that is too much work, I'll use your code as a starting point to fix the compatibility issues and other issues you have pointed out.

Regardless of the approach, you will have a moleculer-go version soon that is 100% compatible and stable with moleculer JS.

We still need to write our contribution guidelines, but for future PR please try to do one feature/fix per each PR. That makes us able to approve/fix PRs quickly and move faster :)

pentateu commented 5 years ago

@dehypnosis NATS transporter looks great. I'll merge that as part of PR #57 and will create a new one for Moleculer JS compatibility.

pentateu commented 5 years ago

@dehypnosis all issues you have highlighted here, and requests were implemented on PR #59

Exceptions:

Event subscription with pattern matching like "my.event.**" (#60 )
Action param validation

These 2 requires a bit of design consideration, but they will come out soon.

moleculer-go / moleculer