AlexStocks commented on code in PR #3665:
URL: https://github.com/apache/dubbo-go/pull/3665#discussion_r3788488245


##########
metrics/options.go:
##########
@@ -26,14 +26,25 @@ import (
        "dubbo.apache.org/dubbo-go/v3/global"
 )
 
+// Options holds the configuration of the metrics module.
+// It wraps the global MetricsConfig, and can be built programmatically
+// with NewOptions and a set of Option functions.
+//
+// The metrics module is disabled by default; use WithEnabled to turn it on.

Review Comment:
   [P2] 请区分 NewOptions() 的构造期零值和实例初始化后的有效默认值
   
   global.DefaultMetricsConfig() 明确不填充带 default tag 的字段,当前新增测试也断言 NewOptions() 
得到的 Port、Path、Protocol 都为空,Enable 为 nil;9090、/metrics、prometheus 等值是在更外层 
client/server 初始化执行 defaults.Set 后才生效。这里以及 WithPort、WithPath、WithPrometheus 
等注释直接写“默认值”,会让直接调用 NewOptions() 的用户误以为返回对象已经具备这些值。请明确这些是运行时实例初始化默认值;如果 API 设计要求 
NewOptions() 立即返回它们,则应在该入口实际应用 defaults.Set 并同步测试。



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to