AlexStocks commented on code in PR #3231:
URL: https://github.com/apache/dubbo-go/pull/3231#discussion_r2934595438
##########
protocol/dubbo3/dubbo3_invoker.go:
##########
@@ -72,20 +71,21 @@ func NewDubboInvoker(url *common.URL) (*DubboInvoker,
error) {
rt string
consumerService any
)
- // TODO: Temporary compatibility with old APIs, can be removed later
- rt = config.GetConsumerConfig().RequestTimeout
- if consumerConfRaw, ok := url.GetAttribute(constant.ConsumerConfigKey);
ok {
- if consumerConf, ok :=
consumerConfRaw.(*global.ConsumerConfig); ok {
- rt = consumerConf.RequestTimeout
+ rt = url.GetParam(constant.TimeoutKey, "")
Review Comment:
这一行 `url.GetParam(constant.TimeoutKey, "")` 是多余的。下面
`url.GetParamDuration(constant.TimeoutKey, rt)` 内部已经调用了 `GetParam(TimeoutKey,
rt)`，如果 URL 有 TimeoutKey 参数会优先使用。
`rt` 变量只需要作为 `GetParamDuration` 的 fallback default，简化为：
```go
rt := ""
if consumerConfRaw, ok := url.GetAttribute(constant.ConsumerConfigKey); ok {
if consumerConf, ok := consumerConfRaw.(*global.ConsumerConfig); ok &&
consumerConf.RequestTimeout != "" {
rt = consumerConf.RequestTimeout
}
}
if rt == "" {
rt = global.DefaultConsumerConfig().RequestTimeout
}
timeout := url.GetParamDuration(constant.TimeoutKey, rt)
```
##########
protocol/dubbo3/dubbo3_invoker.go:
##########
@@ -112,19 +112,24 @@ func NewDubboInvoker(url *common.URL) (*DubboInvoker,
error) {
opts = append(opts,
triConfig.WithGRPCMaxCallRecvMessageSize(maxCallRecvMsgSize))
opts = append(opts,
triConfig.WithGRPCMaxCallSendMessageSize(maxCallSendMsgSize))
- //TODO: Temporary compatibility with old APIs, can be removed later
- tracingKey := url.GetParam(constant.TracingConfigKey, "")
- if tracingKey != "" {
- tracingConfig := config.GetTracingConfig(tracingKey)
- if tracingConfig != nil {
+ if tracingConfRaw, ok := url.GetAttribute(constant.TracingConfigKey);
ok {
+ tracingConfig, ok := tracingConfRaw.(*global.TracingConfig)
+ if !ok {
+ logger.Warnf("invalid tracing config type %T, expected
*global.TracingConfig", tracingConfRaw)
+ } else if tracingConfig != nil {
if tracingConfig.Name == "jaeger" {
- if tracingConfig.ServiceName == "" {
- tracingConfig.ServiceName =
config.GetApplicationConfig().Name
+ serviceName := tracingConfig.ServiceName
Review Comment:
`dubbo3_invoker.go` 和 `dubbo3_protocol.go` 两处相同逻辑：如果
`tracingConfig.ServiceName` 和 URL 参数 `ApplicationKey`
都为空，`WithJaegerConfig(address, "", useAgent)` 会传入空 service name，Jaeger 收到的
trace 数据无法按服务归类。
建议空时跳过或至少打个 warn：
```go
if serviceName == "" {
logger.Warnf("jaeger tracing skipped: no service name available for %s",
url.String())
} else {
opts = append(opts, triConfig.WithJaegerConfig(tracingConfig.Address,
serviceName, useAgent))
}
```
--
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]
