apache / dubbo-go

Go Implementation For Apache Dubbo .
https://dubbo.apache.org/
Apache License 2.0
4.69k stars 917 forks source link

feat : add script routing functionality #2669

Closed YarBor closed 4 months ago

YarBor commented 4 months ago

JS Standard

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

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


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


  - config file like :
```yaml
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.

  1. Input args are invokers, invocation, context, The following methods are not allowed.
    • invokers.Invoke(...)
    • invokers.Destroy()
    • invokers.IsAvailable() will always return true
  2. 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));
  3. 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));
codecov-commenter commented 4 months ago

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.

:exclamation: 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 :warning:
cluster/router/script/instance/instances_pool.go 47.36% 20 Missing :warning:
common/url.go 41.66% 5 Missing and 2 partials :warning:
Additional details and impacted files ```diff @@ 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 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

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

Quality Gate Passed 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