Optimize application #9

glmapper · 2019-05-13T08:21:52Z

增加应用面板信息展示. #10

codecov-io · 2019-05-13T08:30:58Z

Codecov Report

Merging #9 into master will decrease coverage by 1.68%.
The diff coverage is 25.77%.

@@             Coverage Diff              @@
##             master       #9      +/-   ##
============================================
- Coverage     41.64%   39.95%   -1.69%     
- Complexity      179      189      +10     
============================================
  Files            37       52      +15     
  Lines          1263     1907     +644     
  Branches        153      210      +57     
============================================
+ Hits            526      762     +236     
- Misses          676     1074     +398     
- Partials         61       71      +10

Impacted Files	Coverage Δ	Complexity Δ
.../java/com/alipay/sofa/dashboard/impl/ZkHelper.java	`85% <ø> (+75%)`	`6 <0> (+4)`	⬆️
...fa/dashboard/constants/SofaDashboardConstants.java	`0% <ø> (ø)`	`0 <0> (ø)`	⬇️
...ipay/sofa/dashboard/impl/ZkCommandPushManager.java	`78.23% <ø> (+75.51%)`	`27 <0> (+25)`	⬆️
...dashboard/app/actuator/ActuatorMonitorManager.java	`0% <0%> (ø)`	`0 <0> (?)`
...ard/app/zookeeper/ZookeeperApplicationManager.java	`0% <0%> (ø)`	`0 <0> (?)`
...sofa/dashboard/model/monitor/AvailableTagInfo.java	`0% <0%> (ø)`	`0 <0> (?)`
...ipay/sofa/dashboard/model/monitor/MetricsInfo.java	`0% <0%> (ø)`	`0 <0> (?)`
...ay/sofa/dashboard/app/task/AppDynamicInfoTask.java	`0% <0%> (ø)`	`0 <0> (?)`
...lipay/sofa/dashboard/model/monitor/HealthInfo.java	`0% <0%> (ø)`	`0 <0> (?)`
...ofa/dashboard/model/monitor/MemoryNonHeapInfo.java	`100% <100%> (ø)`	`0 <0> (?)`
... and 30 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 71e66dd...8727b27. Read the comment docs.

# Conflicts: # sofa-dashboard-backend/sofa-dashboard-core/src/main/java/com/alipay/sofa/dashboard/constants/SofaDashboardConstants.java # sofa-dashboard-backend/sofa-dashboard-governance/src/main/java/com/alipay/sofa/dashboard/cache/RegistryDataCache.java # sofa-dashboard-backend/sofa-dashboard-web/src/main/java/com/alipay/sofa/dashboard/controller/ApplicationController.java

Master glmapper

# Conflicts: # sofa-dashboard-backend/sofa-dashboard-governance/src/main/java/com/alipay/sofa/dashboard/listener/sofa/SofaRegistryRestClient.java

caojie09 · 2019-05-15T07:19:37Z

...application/src/main/java/com/alipay/sofa/dashboard/app/actuator/ActuatorMonitorManager.java

+
+    @Override
+    public List<MetricsInfo> fetchMetrics(Object source) {
+        // do not support


不支持的话直接删除吧。

caojie09 · 2019-05-23T12:12:46Z

...application/src/main/java/com/alipay/sofa/dashboard/app/actuator/ActuatorMonitorManager.java

+
+    private RestTemplate                                     restTemplate       = new RestTemplate();
+
+    public static Map<String, FixedQueue<DetailThreadInfo>>  cacheDetailThreads = new ConcurrentHashMap<>();


这里为什么写成 public static，是否有更面向对象的写法呢？

caojie09 · 2019-05-23T12:16:23Z

...application/src/main/java/com/alipay/sofa/dashboard/app/actuator/ActuatorMonitorManager.java

+    public static Map<String, FixedQueue<MemoryNonHeapInfo>> cacheNonHeapMemory = new ConcurrentHashMap<>();
+
+    @Override
+    public EnvironmentInfo fetchEnvironment(Object source) {


传入的参数全部是 String，为什么把接口声明成 source？

传入的参数全部是 String，为什么把接口声明成 source？
引用数据源理论上是多样的，比如一个 rest 请求地址、文件、或者其他第三方服务。这里 source 类型使用 Object 对象用于描述数据来源，各个实现可以基于具体源定义具体类型，比如 ActuatorMonitorManager 实现 source 是请求地址，其 source 类型即为 String 类型；这样可以提供更加丰富的选择性。在对接其他源时，source 可能会包括更多信息，比如是个 Map 或者 pojo 。

caojie09 · 2019-05-23T15:08:08Z

...backend/sofa-dashboard-core/src/main/java/com/alipay/sofa/dashboard/utils/DashboardUtil.java

+        return sb.toString();
+    }
+
+    public static String simpleDecode(String encodeAddress) {


这个方法能不能加下注释呢，看不懂是什么意思。

caojie09 · 2019-05-23T15:09:00Z

...backend/sofa-dashboard-core/src/main/java/com/alipay/sofa/dashboard/utils/DashboardUtil.java

+     */
+    public static final String            KEY        = "g";
+
+    private static final SimpleDateFormat dateFormat = new SimpleDateFormat("HH:mm:ss");


常量要大写，但是 SimpleDateFormat 不建议作为常量，因为他不是线程安全的。

caojie09 · 2019-05-23T15:17:57Z

...application/src/main/java/com/alipay/sofa/dashboard/app/actuator/ActuatorMonitorManager.java

+        }
+    }
+
+    public static Map doRequest(Object source, String path, RestTemplate restTemplate) {


这里为什么是static，这个类实现了 MonitorManager 接口，不应该只暴露出 MonitorManager 的接口吗，这个接口为什么要放在这个类里面？

caojie09 · 2019-05-23T15:21:13Z

...rd-backend/sofa-dashboard-core/src/main/java/com/alipay/sofa/dashboard/utils/FixedQueue.java

+    }
+
+    public LinkedList<E> getQueue() {
+        return queue;


这里不要把queue对象暴露出去，暴露出去了外部 API 就可以随意添加了，应该新建一个 List 返回。

caojie09 · 2019-05-23T15:32:12Z

...fa-dashboard-governance/src/main/java/com/alipay/sofa/dashboard/cache/RegistryDataCache.java

     */
-    void addConsumers(String serviceName, List<RpcConsumer> consumerList);
+    void addConsumers(String serviceName, List<RpcConsumer> consumers);


这个修改是因为 master 没合过来的 diff 嚒？

这个修改是因为 master 没合过来的 diff 嚒？

代码在Fix concurrency issues 这个 PR 中，还没有合入 master

caojie09 · 2019-05-23T15:33:08Z

...-dashboard-web/src/main/java/com/alipay/sofa/dashboard/controller/AppActuatorController.java

+    private MonitorManager monitorManager;
+
+    @RequestMapping("details")
+    public Map baseDetails(@RequestParam("id") String targetHost) {


如果参数的意思是 targetHost，为什么不直接叫 targetHost，而是叫 id？

如果参数的意思是 targetHost，为什么不直接叫 targetHost，而是叫 id？

这个将 targetHost 改成 id，在 dashboard 中，前端返回的应用实例除“目标主机”概念之外，概念上应该更广义，所以 id 更适合

caojie09 · 2019-05-23T15:35:37Z

sofa-dashboard-backend/sofa-dashboard-web/src/main/resources/application.properties

@@ -15,4 +15,4 @@ mybatis.config-location=classpath:spring/mybatis-configuration.xml
 # server port
 server.port=8099

-spring.profiles.active=localhost
+spring.profiles.active=alipay


这个不需要修改。

chpengzh · 2019-05-24T02:53:27Z

之前看的时候有几个建议:

跨模块实现的接口定义引用的模型最好还是不要用map而是用一个pojo去定义，比如MonitorManager.fetchInfo的返回值
还有就是Actuator的实现主要是靠调用相关应用的接口，这种情况比较好的一个实践是抽离dao层，然后定时任务和manager都去调用dao层的方法；
目前的实现方案是MonitorManager这个service和AppDynamicInfoTask这个task都去直接调用请求；相关的单元测试逻辑就变成了先去填充定时任务的缓存，再去调用MonitorManager的函数。如果有dao层，单元测试就可统一对接口调用做测试了。

glmapper · 2019-05-24T03:09:47Z

之前看的时候有几个建议:

跨模块实现的接口定义引用的模型最好还是不要用 map 而是用一个pojo去定义，比如MonitorManager.fetchInfo的返回值

还有就是Actuator的实现主要是靠调用相关应用的接口，这种情况比较好的一个实践是抽离dao层；
目前的实现方案是MonitorManager这个service和AppDynamicInfoTask这个task都去直接调用请求；相关的单元测试逻辑就变成了先去填充定时任务的缓存，再去调用MonitorManager的函数。如果有dao层，单元测试就可统一对接口调用做测试了。

是的，这部分已经抽出，重新定义了一个 RestTemplateClient 类负责用于与应用之间交互，ActuatorMonitorManager 将专注在数据解析与管理。

fetchInfo 这个在设计时没有使用 pojo 在于应用的 info 信息本身没有固定的属性，比如这样定义：

info.app.name=spring-boot-hello
info.app.version=v1.0.0
info.test.a.b.c=abc
info.test.a.c.b=acb

在解析时会解析成普通的 k-v ，在模型上统一比较难，如果有什么好的建议，欢迎 PR

chpengzh · 2019-06-01T01:37:10Z

请不要使用external library的方式引用 data-set 库，这样会导致离线用户无法使用应用界面。

比较简单的验证方法就是 npm install 之后在offline模式下执行 npm dev。清空浏览器缓存，进入应用detail界面，提示DataSet未定义。

因为该分支没有合并主线，所以如果方便的话请 cherry-pick 这一次提交进行修复 a30c6f

https://github.com/chpengzh/sofa-dashboard/commits/optimize-application

chpengzh · 2019-06-01T01:46:13Z

还有就是因为没有初始化eslint的配置，每次git提交的时候会报eslint的错误，只能通过git commit -n的方式调过预检测。因为我不知道这边项目对eslint的要求如何，如果可以的话在front下执行一次eslint --init。

…veral invoke error

glmapper · 2019-07-20T07:47:15Z

this pr will be closed and divide to multi sub pr

卫恒 added 3 commits May 11, 2019 16:34

application info

53a3d7d

optimize code

db51786

code format

223450e

卫恒 added 7 commits May 13, 2019 19:52

fix test case

836dd94

add test case

5e1e896

code format

4892c7f

code format

1fcce18

sync master

96127b3

fix service goverance

4e01526

glmapper requested review from caojie09 and ujjboy May 22, 2019 06:34

glmapper and others added 7 commits May 22, 2019 17:01

Merge pull request #1 from glmapper/master-glmapper

238717a

Master glmapper

Merge branch 'master-alipay'

be6bd0b

# Conflicts: # sofa-dashboard-backend/sofa-dashboard-governance/src/main/java/com/alipay/sofa/dashboard/listener/sofa/SofaRegistryRestClient.java

Merge branch 'master' into optimize-application

3f882e6

remove dumplicat

882fa6c

do not test model & enum

6cb7a9f

provide comment

da56e9a

add test case

8727b27

caojie09 reviewed May 23, 2019

View reviewed changes

chpengzh added a commit to chpengzh/sofa-dashboard that referenced this pull request Jun 11, 2019

[sofastack#9] Using a standalone client to access actuator and fix se…

bab2588

…veral invoke error

chpengzh added a commit to chpengzh/sofa-dashboard that referenced this pull request Jun 15, 2019

[sofastack#9] Using a standalone client to access actuator and fix se…

05b8619

…veral invoke error

glmapper closed this Jul 20, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Optimize application #9

Optimize application #9

glmapper commented May 13, 2019 •

edited

Loading

codecov-io commented May 13, 2019 •

edited

Loading

caojie09 May 15, 2019

caojie09 May 23, 2019

caojie09 May 23, 2019

glmapper May 24, 2019

caojie09 May 23, 2019

caojie09 May 23, 2019

caojie09 May 23, 2019

caojie09 May 23, 2019

caojie09 May 23, 2019

glmapper May 24, 2019

caojie09 May 23, 2019

glmapper May 24, 2019

caojie09 May 23, 2019

chpengzh commented May 24, 2019 •

edited

Loading

glmapper commented May 24, 2019

chpengzh commented Jun 1, 2019 •

edited

Loading

chpengzh commented Jun 1, 2019

glmapper commented Jul 20, 2019


		private RestTemplate restTemplate = new RestTemplate();

		public static Map<String, FixedQueue<DetailThreadInfo>> cacheDetailThreads = new ConcurrentHashMap<>();

Optimize application #9

Optimize application #9

Conversation

glmapper commented May 13, 2019 • edited Loading

codecov-io commented May 13, 2019 • edited Loading

Codecov Report

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chpengzh commented May 24, 2019 • edited Loading

glmapper commented May 24, 2019

chpengzh commented Jun 1, 2019 • edited Loading

chpengzh commented Jun 1, 2019

glmapper commented Jul 20, 2019

glmapper commented May 13, 2019 •

edited

Loading

codecov-io commented May 13, 2019 •

edited

Loading

chpengzh commented May 24, 2019 •

edited

Loading

chpengzh commented Jun 1, 2019 •

edited

Loading