feat : add script routing functionality #2669

YarBor · 2024-05-10T11:22:52Z

JS Standard

The expect to recv js-script data is like:

(function route(invokers, invocation, context) {
    var result = [];
    for (var i = 0; i < invokers.length; i++) {
        if ("127.0.0.1" === invokers[i].GetURL().Ip) {
            if (invokers[i].GetURL().Port !== "20000") {
                invokers[i].GetURL().Ip = "anotherIP"
                result.push(invokers[i]);
            }
        }
    }
    return result;
}(invokers, invocation, context));

The expected way to return value is like:

    var result = [];
    result.push(invokers[i]);
    return result;

Supports method calling.
Parameter methods are mapped by the passed in type.
The called method first letter is capitalized (does not comply with js specifications).

e.g. invokers[i].GetURL().SetParam("testKey","testValue")

Like the Go language, it supports direct access to exportable variables within parameters.

e.g. invokers[i].GetURL().Port

publish config like :

scriptRouteConfig := data, err := os.ReadFile(pathToConfigfile)
conf, _ := config.GetRootConfig().ConfigCenter.GetDynamicConfiguration()
_ = conf.PublishConfig("dubbo.io"+constant.ScriptRouterRuleSuffix, "dubbo", scriptRouteConfig)

config file like :

configVersion: v3.0
key: dubbo.io
type: javascript
enabled: true
script: |
  (function route(invokers,invocation,context) {
  	var result = [];
  	for (var i = 0; i < invokers.length; i++) {
  	    if ("10.30.0.218" === invokers[i].GetURL().Ip) {
  			if (invokers[i].GetURL().Port === "20000"){
  				invokers[i].GetURL().Port = "20001" 
  				result.push(invokers[i]);
  			}
  	    }
  	}
  	return result;
  }(invokers,invocation,context));

Important

In the js-support.

Input args are invokers, invocation, context,
The following methods are not allowed.
- invokers.Invoke(...)
- invokers.Destroy()
- invokers.IsAvailable() will always return true
Wrong input is program acceptable, it will not return an error or go panic , it will make an undefined error.
for example:

(function route(invokers, invocation, context) {
    var result = [];
    for (var i = 0; i < invokers.length; i++) {
        if ("127.0.0.1" === invokers[i].GetURL().Ip) {
            if (invokers[i].GetURL(" Err Here ").Port !== "20000") {
//---------------------------------^  here wont go err , it will trans to ()
                invokers[i].GetURL().Ip = "anotherIP"
                result.push(invokers[i]);
            }
        }
    }
    return result;
}(invokers, invocation, context));

(function route(invokers, invocation, context) {
    var result = [];
    for (var i = 0; i < invokers.length; i++) {
        if ("127.0.0.1" === invokers[i].GetURL().Ip) {
            if (invokers[i].GetURL(" Err Here ").Port !== "20000") {
                invokers[i].GetURL().AddParam( null , "key string", "value string")
//---------------------------------------------^  here wont make err , it will trans to ("" , "key string" , "value string")
                result.push(invokers[i]);
            }
        }
    }
    return result;
}(invokers, invocation, context));

(function route(invokers, invocation, context) {
    var result = [];
    for (var i = 0; i < invokers.length; i++) {
        if ("127.0.0.1" === invokers[i].GetURL().Ip) {
            if (invokers[i].GetURL(" Err Here ").Port !== "20000") {
                invokers[i].GetURL().AddParam( invocation , "key string", "value string")
//---------------------------------------------^  here wont make err , it will trans to ("[object Object]" , "key string" , "value string")
                result.push(invokers[i]);
            }
        }
    }
    return result;
}(invokers, invocation, context));

Wrong methods-call will go error.
for example:

(function route(invokers, invocation, context) {
    var result = [];
    for (var i = 0; i < invokers.length; i++) {
        if ("127.0.0.1" === invokers[i].GetURL().Ip) {
            if (invokers[i].GetURLS(" Err Here ").Port !== "20000") {
//--------------------------^ // here will make err 
                invokers[i].GetURLS().Ip = "anotherIP"
                result.push(invokers[i]);
            }
        }
    }
    return result;
}(invokers, invocation, context));

Signed-off-by: YarBor <[email protected]>

baerwang · 2024-05-10T11:37:21Z

cluster/router/script/instance/instances_pool.go

+	if err != nil {
+		return nil, err
+	}
+	result := make([]protocol.Invoker, 0)


map add cap

Signed-off-by: YarBor <[email protected]>

add comment to jsInstance Signed-off-by: YarBor <[email protected]>

AlexStocks · 2024-05-11T05:58:03Z

cluster/router/script/instance/instances_pool.go

+package instance
+
+import (
+	"dubbo.apache.org/dubbo-go/v3/protocol"


I don't understand what the problem is, I ran go fmt and it doesn't work, looks the same.

I don't understand what the problem is, I ran go fmt and it doesn't work, looks the same.

Go fmt will not change the order of the import, it will only format locks and spaces. You can use goimports instead,https://pkg.go.dev/golang.org/x/tools/cmd/goimports
Generally speaking, the standard import format is:
import (
go standard packages (sorted alphabetically)
(space)
Three-party packages (sorted alphabetically)
)

我们 dubbogo 有自己的 import format 工具 https://github.com/dubbogo/tools/tree/master/cmd/imports-formatter

编译出二进制，在项目根目录下执行一次，就可以 format 整个工程的 import 顺序。比你们找的这个工具高级多了。

用好前人积累的工具。

AlexStocks · 2024-05-11T05:58:44Z

cluster/router/script/instance/instances_pool.go

+)
+
+type ScriptInstances interface {
+	RunScript(rawScript string, invokers []protocol.Invoker, invocation protocol.Invocation) ([]protocol.Invoker, error)


ScriptInstances struct 里面已经有 Script 了，Run() 这个函数名就 ok 了

AlexStocks · 2024-05-11T05:59:20Z

cluster/router/script/instance/instances_pool.go

+	factory[tpName] = instance
+}
+
+func init() {


init 函数挪到 struct 定义下面

AlexStocks · 2024-05-11T05:59:30Z

cluster/router/script/instance/js_instance.go

+package instance
+
+import (
+	"dubbo.apache.org/dubbo-go/v3/protocol"


AlexStocks · 2024-05-11T05:59:50Z

cluster/router/script/instance/js_instance.go

+	}
+}
+
+func (i *jsInstances) RunScript(_ string, invokers []protocol.Invoker, invocation protocol.Invocation) ([]protocol.Invoker, error) {


AlexStocks · 2024-05-11T06:00:05Z

cluster/router/script/route.go

+package script
+
+import (
+	ins "dubbo.apache.org/dubbo-go/v3/cluster/router/script/instance"


The current problem is that the parameter passed in is a reference. When the URL is modified, the original copy will be modified, which will lead to undefined behavior. Signed-off-by: YarBor <[email protected]>

codecov-commenter · 2024-05-11T07:30:30Z

Codecov Report

Attention: Patch coverage is 21.18644% with 93 lines in your changes are missing coverage. Please review.

Project coverage is 47.21%. Comparing base (2f5143a) to head (5b51cc0).
Report is 17 commits behind head on main.

❗ Current head 5b51cc0 differs from pull request most recent head a650f8a

Please upload reports for the commit a650f8a to get more accurate results.

Files	Patch %	Lines
cluster/router/script/instance/js_instance.go	2.94%	65 Missing and 1 partial ⚠️
cluster/router/script/instance/instances_pool.go	47.36%	20 Missing ⚠️
common/url.go	41.66%	5 Missing and 2 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2669      +/-   ##
==========================================
- Coverage   47.38%   47.21%   -0.17%     
==========================================
  Files         341      343       +2     
  Lines       25122    25286     +164     
==========================================
+ Hits        11904    11940      +36     
- Misses      12074    12188     +114     
- Partials     1144     1158      +14

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Signed-off-by: YarBor <[email protected]>

chickenlj · 2024-05-16T01:48:13Z

监听的规则 key 应该是 provider 应用名，而不是当前进程自身应用名。可参考 tagrouter 的 notify 方法
第1个逻辑改了后，Script Instance 实例的对应关系也要改一下，当前是全局共享的
要确认下 dynamicconfuguration.AddListener() 的逻辑对不对（zookeeper/nacos），因为有多个 ScriptRouter 实例可能对应到一个 key (provider应用名)，dynamicconfuguration.AddListener()应该支持调用多次且记录多个listener --- zookeeper实现看起来没问题，nacos实现好像有问题。
instance.Compile 并发调用有没有问题

…nces. Signed-off-by: YarBor <[email protected]>

Signed-off-by: YarBor <[email protected]>

chickenlj · 2024-05-17T06:49:48Z

cluster/router/script/instance/js_instance.go

+		}
+		i.pgLock.Unlock()
+		return nil
+	}


The lock used here is a bit strange, try the following steps:

check without lock

if exists, return

if not exists, get the lock
3.1 check again
3.2 if exists return
3.3 if not exists compile

I previously thought that "compiling" was a time-consuming operation that was not suitable to be performed while occupying a lock.

i will rewite it.

I see. I thought the following lock was not necessary, it seems to be necessary in this case.

i.pgLock.RLock() pg, ok = i.program[rawScript] i.pgLock.RUnlock()

Signed-off-by: YarBor <[email protected]>

sonarcloud · 2024-05-17T08:25:05Z

Quality Gate passed

Issues
9 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
1.4% Duplication on New Code

See analysis details on SonarCloud

chickenlj

LGTM.

YarBor added 4 commits May 9, 2024 22:52

add jsScript Instance test

2b10398

Signed-off-by: YarBor <[email protected]>

impl script half logic

5a2694c

Signed-off-by: YarBor <[email protected]>

add license

5f9440c

Signed-off-by: YarBor <[email protected]>

add script-router process logic

19215b4

Signed-off-by: YarBor <[email protected]>

baerwang reviewed May 10, 2024

View reviewed changes

YarBor and others added 3 commits May 10, 2024 19:46

fix ci

bdb9f15

Signed-off-by: YarBor <[email protected]>

Merge branch 'apache:main' into main2

d1f5571

add package path to imports/import

927b76a

add comment to jsInstance Signed-off-by: YarBor <[email protected]>

YarBor changed the title ~~Add script routing functionality~~ feat : add script routing functionality May 11, 2024

AlexStocks reviewed May 11, 2024

View reviewed changes

The basic filtering function of invoker has been tested and completed.

4f6c5d9

The current problem is that the parameter passed in is a reference. When the URL is modified, the original copy will be modified, which will lead to undefined behavior. Signed-off-by: YarBor <[email protected]>

YarBor added 7 commits May 11, 2024 22:36

package inputed Invoker, to make script operate url safly

0fcf224

Signed-off-by: YarBor <[email protected]>

set panic safy

99752ef

Signed-off-by: YarBor <[email protected]>

add more test case, remove unuse field,struct,interface

7ea5367

Signed-off-by: YarBor <[email protected]>

fix CI, golangci-lint

27d0d0a

Signed-off-by: YarBor <[email protected]>

fix Config Update, make enable field useful

2b5d48f

Signed-off-by: YarBor <[email protected]>

remove extra comment

fa480e9

Signed-off-by: YarBor <[email protected]>

reformat imports

c17eb4a

Signed-off-by: YarBor <[email protected]>

baerwang requested a review from justxuewei May 15, 2024 14:25

YarBor added 3 commits May 16, 2024 12:07

Rewrite the monitoring logic and modify the rules of get script insta…

5c7b696

…nces. Signed-off-by: YarBor <[email protected]>

rewrite comment, fix ci-lint

17f951a

Signed-off-by: YarBor <[email protected]>

make nacosDynamicConfiguration support multiple listeners

5bb551d

Signed-off-by: YarBor <[email protected]>

chickenlj reviewed May 17, 2024

View reviewed changes

YarBor added 3 commits May 17, 2024 15:08

fix comment

c9328e1

Signed-off-by: YarBor <[email protected]>

update scriptRouter recycle logic

26ca110

Signed-off-by: YarBor <[email protected]>

fix misspell

a650f8a

Signed-off-by: YarBor <[email protected]>

chickenlj approved these changes May 18, 2024

View reviewed changes

chickenlj merged commit 955c6d6 into apache:main May 18, 2024
5 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat : add script routing functionality #2669

feat : add script routing functionality #2669

YarBor commented May 10, 2024 •

edited

Loading

baerwang May 10, 2024

AlexStocks May 11, 2024

YarBor May 13, 2024

EndlessSeeker May 13, 2024

YarBor May 13, 2024

AlexStocks May 16, 2024

AlexStocks May 11, 2024

AlexStocks May 11, 2024

AlexStocks May 11, 2024

AlexStocks May 11, 2024

AlexStocks May 11, 2024

codecov-commenter commented May 11, 2024 •

edited

Loading

chickenlj commented May 16, 2024

chickenlj May 17, 2024 •

edited

Loading

YarBor May 17, 2024

YarBor May 17, 2024

chickenlj May 17, 2024

sonarcloud bot commented May 17, 2024

chickenlj left a comment

feat : add script routing functionality #2669

feat : add script routing functionality #2669

Conversation

YarBor commented May 10, 2024 • edited Loading

JS Standard

Important

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov-commenter commented May 11, 2024 • edited Loading

Codecov Report

chickenlj commented May 16, 2024

chickenlj May 17, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sonarcloud bot commented May 17, 2024

Quality Gate passed

chickenlj left a comment

Choose a reason for hiding this comment

YarBor commented May 10, 2024 •

edited

Loading

codecov-commenter commented May 11, 2024 •

edited

Loading

chickenlj May 17, 2024 •

edited

Loading