Refactor: remove config in grpc and dubbo3 by Snow-kal · Pull Request #3231 · apache/dubbo-go

Snow-kal · 2026-03-06T05:26:34Z

更改概览和 TODO 更改情况

protocol/dubbo3/dubbo3_invoker.go 已移除

TODO RequestTimeout：默认超时读取路径变更，改为从 global.DefaultConsumerConfig() 读取，经 url 的 ConsumerConfigKey 覆盖后，统一用 TimeoutKey 解析最终值
TODO consumerService：删除旧兼容逻辑，仅从url的RpcServiceKey 属性获取服务实例
TODO tracing：不再从 config 读取，改为直接读取 url 的 TracingConfigKey，并补充类型校验和空值兜底
TODO TLSConfig：移除全局配置分支，仅保留从 url 的 TLSConfigKey 属性初始化 TLS

protocol/dubbo3/dubbo3_protocol.go 已移除

TODO deprecated：删除废弃的注释代码，保留初始化逻辑
TODO Export()：删除旧API兼容逻辑，移除 config.GetProviderService，改为仅从url的 RpcServiceKey 属性获取
TODO tracing：不再从 config 读取，移除对 config.GetTracingConfig/GetApplicationConfig 的依赖
TODO TLSConfig：移除 root-config 的全局配置分支，保留配置链路

protocol/grpc/client.go 已移除

TODO: remove config TLSConfig：已删除 config TLS 分支
TODO: remove this else：已移除该 else 分支。
TODO: Temporary compatibility...（consumerService）：已移除旧 API 查找，保留 RpcServiceKey attribute。
TODO: Temporary compatibility...（protocolConf）：已移除 rootConfig/dubbo compat 路径，保留 ProtocolConfigKey attribute

protocol/grpc/server.go 已移除

TODO:remove config TLSConfig：已删除 root-config TLS 分支，保留TLS 路径。
TODO:remove this else：已去掉原 else 分支
TODO Package dependency optimization #2741 old config compatibility（getProviderServices）：已去掉 config 读取与兼容转换，保留 ProviderConfigKey
TODO Package dependency optimization #2741 old config compatibility（waitGrpcExporter）：兼容注释已清理，逻辑保留为等待 exporter
TODO Package dependency optimization #2741 old config compatibility（registerService）：已去掉旧 config 服务查找链路
Temporary compatibility with old APIs：已删除 config.GetProviderService(key)，改从 invoker URL 的 RpcServiceKey 获取服务对象

Description

issue #3204

Checklist

I confirm the target branch is develop
Code has passed local testing
I have added tests that prove my fix is effective or that my feature works

Copilot

Pull request overview

This PR continues the config-package removal effort (issues #2741 / #3204) by refactoring the gRPC and Dubbo3 (triple) protocol implementations to source runtime behavior from global.* defaults and URL attributes instead of config/*.

Changes:

Remove legacy config compatibility branches for TLS/service/protocol/tracing in gRPC client/server.
Refactor Dubbo3 protocol/invoker to read timeout, tracing, TLS, and service instances from URL attributes (with some type checks and fallbacks).
Clean up deprecated/compat code paths and related imports.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File	Description
`protocol/grpc/server.go`	Drops config-based provider/TLS lookup; relies on `URL` attributes and global TLS helpers.
`protocol/grpc/client.go`	Drops config-based TLS/protocol/service lookup; uses `URL` attributes and safer invoker reflection handling.
`protocol/dubbo3/dubbo3_protocol.go`	Removes config dependency; tracing/TLS/service resolved from `URL` attributes.
`protocol/dubbo3/dubbo3_invoker.go`	Removes config dependency; timeout/tracing/TLS/service resolved from `global` + `URL` attributes.

Comments suppressed due to low confidence (4)

protocol/grpc/server.go:114

In Start(), if TLSConfigKey is present but not a *global.TLSConfig, the function returns early after net.Listen() without closing the listener, and the server never starts. Consider logging the type assertion failure and continuing with the already-configured insecure creds (or explicitly closing lis before returning) to avoid leaking the bound port.

	if tlsConfRaw, ok := url.GetAttribute(constant.TLSConfigKey); ok {
		// use global TLSConfig handle tls
		tlsConf, ok := tlsConfRaw.(*global.TLSConfig)
		if !ok {
			logger.Errorf("gRPC Server initialized the TLSConfig configuration failed")
			return
		}

protocol/grpc/server.go:155

getProviderServices() returns nil when ProviderConfigKey is missing/invalid, but Start() has already created the listener and server and the goroutine will exit early, leaving the listener bound with no serving loop. To avoid leaking resources / hanging startup, consider either: (1) moving net.Listen()/grpc.NewServer() until after provider services are validated, (2) returning an empty map and allowing Serve() to run, or (3) explicitly closing the listener and stopping the server on this error path.

// getProviderServices retrieves provider services from URL attributes.
func getProviderServices(url *common.URL) map[string]*global.ServiceConfig {
	if providerConfRaw, ok := url.GetAttribute(constant.ProviderConfigKey); ok {
		if providerConf, ok := providerConfRaw.(*global.ProviderConfig); ok && providerConf != nil {
			return providerConf.Services
		}
		logger.Error("illegal provider config")
	}
	return nil
}

protocol/grpc/client.go:100

The returned error message uses "DUBBO3 Client ..." even though this is the gRPC client path, and it doesn't match the preceding log line. Please align the error text with the component (gRPC) so users can diagnose mis-typed TLSConfigKey attributes more easily.

		tlsConf, ok := tlsConfRaw.(*global.TLSConfig)
		if !ok {
			logger.Errorf("Grpc Client initialized the TLSConfig configuration failed")
			return nil, errors.New("DUBBO3 Client initialized the TLSConfig configuration failed")
		}

protocol/dubbo3/dubbo3_invoker.go:87

NewDubboInvoker() now relies solely on url attribute RpcServiceKey for consumerService, but it can remain nil when the attribute isn't set; that nil is then passed into triple.NewTripleClient(). Adding an explicit check with a clear error (including interface/service key) before constructing the client would make failures deterministic and easier to debug.

	timeout := url.GetParamDuration(constant.TimeoutKey, rt)
	// for triple pb serialization. The bean name from provider is the provider reference key,
	// which can't locate the target consumer stub, so we use interface key.
	consumerService = nil
	if rpcService, ok := url.GetAttribute(constant.RpcServiceKey); ok {
		consumerService = rpcService
	}

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

protocol/dubbo3/dubbo3_protocol.go

…w in openServer

codecov-commenter · 2026-03-06T05:55:14Z

Codecov Report

❌ Patch coverage is 12.16216% with 65 lines in your changes missing coverage. Please review.
✅ Project coverage is 47.99%. Comparing base (60d1c2a) to head (5f69867).
⚠️ Report is 750 commits behind head on develop.

Files with missing lines	Patch %	Lines
protocol/dubbo3/dubbo3_protocol.go	0.00%	21 Missing ⚠️
protocol/dubbo3/dubbo3_invoker.go	0.00%	16 Missing ⚠️
protocol/grpc/client.go	37.50%	13 Missing and 2 partials ⚠️
protocol/grpc/server.go	0.00%	13 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #3231      +/-   ##
===========================================
+ Coverage    46.76%   47.99%   +1.22%     
===========================================
  Files          295      463     +168     
  Lines        17172    33800   +16628     
===========================================
+ Hits          8031    16223    +8192     
- Misses        8287    16264    +7977     
- Partials       854     1313     +459

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

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Alanxtl · 2026-03-06T06:08:58Z

protocol/dubbo3/dubbo3_protocol.go

 		service = rpcService
 	}
+	if service == nil {
+		logger.Errorf("no rpc service found in url attribute %s for service key: %s", constant.RpcServiceKey, key)


Suggested change

logger.Errorf("no rpc service found in url attribute %s for service key: %s", constant.RpcServiceKey, key)

logger.Errorf("[Triple Protocol] No rpc service found in url attribute %s for service key: %s", constant.RpcServiceKey, key)

OK 已按照建议更改

Alanxtl · 2026-03-06T06:13:53Z

protocol/grpc/server.go

@@ -139,12 +121,7 @@ func (s *Server) Start(url *common.URL) {
 				logger.Infof("gRPC Server initialized the TLSConfig configuration")
 				serverOpts = append(serverOpts, grpc.Creds(credentials.NewTLS(cfg)))
 			}
-		} else {
-			serverOpts = append(serverOpts, grpc.Creds(insecure.NewCredentials()))
 		}
-	} else {
-		// TODO: remove this else
-		serverOpts = append(serverOpts, grpc.Creds(insecure.NewCredentials()))
 	}


这里不应该是有TLSConfigKey的时候append credentials.NewTLS(cfg)，没有的时候grpc.Creds(insecure.NewCredentials())吗
现在的逻辑是有没有都grpc.Creds(insecure.NewCredentials())

OK 感谢建议已经更改

新逻辑总结一下是这样的可以吗

transportCreds = insecure.NewCredentials() // 默认值：无 TLS if url 有 TLSConfigKey && TLS 有效 && cfg != nil { transportCreds = credentials.NewTLS(cfg) // 覆盖为 TLS } serverOpts = append(serverOpts, grpc.Creds(transportCreds)) // 只 append 一次

Alanxtl · 2026-03-06T06:15:26Z

protocol/grpc/client.go

@@ -127,12 +108,7 @@ func NewClient(url *common.URL) (*Client, error) {
 				logger.Infof("Grpc Client initialized the TLSConfig configuration")
 				dialOpts = append(dialOpts, grpc.WithTransportCredentials(credentials.NewTLS(cfg)))
 			}
-		} else {
-			dialOpts = append(dialOpts, grpc.WithTransportCredentials(insecure.NewCredentials()))
 		}
-	} else {
-		// TODO: remove this else
-		dialOpts = append(dialOpts, grpc.WithTransportCredentials(insecure.NewCredentials()))
 	}


已经更改感谢建议这个和以上的 server 的类似

总结一下就是

transportCreds = insecure.NewCredentials() // 默认值：无 TLS if url 有 TLSConfigKey { if tlsConf 类型断言失败 { return error // 配置异常，直接返回 } if IsClientTLSValid(tlsConf) { if GetClientTlSConfig 出错 { return error // 获取 TLS 配置失败，直接返回 } if cfg != nil { transportCreds = credentials.NewTLS(cfg) // 覆盖为 TLS } } } dialOpts = append(dialOpts, grpc.WithTransportCredentials(transportCreds)) // 只 append 一次

Alanxtl · 2026-03-06T06:16:37Z

protocol/grpc/client.go

-		if rpcService, ok := url.GetAttribute(constant.RpcServiceKey); ok {
-			consumerService = rpcService
-		}
+	var consumerService any


需不需要置为nil呀

请问一下是改成
var consumerService any = nil这个意思吗
现在的 var consumerService any 零值就是 nil
还是别的意思

AlexStocks

阻塞问题：

protocol/grpc/client.go 在 getInvoker 返回 nil 时仍返回可用 client，对应 invoker 是零值 reflect.Value，后续调用会在运行时 panic。这里应该直接返回 error，不要延迟失败。
TLS 逻辑默认先 append insecure credentials，再尝试 append TLS。现在语义上是 fail-open，TLS 属性缺失/类型错误时会静默回落明文，风险高。建议改成互斥分支：有合法 TLS 就只用 TLS；配置声明 TLS 但构造失败时直接失败。

AlexStocks · 2026-03-07T06:14:32Z

protocol/grpc/client.go

 			grpc.MaxCallRecvMsgSize(maxCallRecvMsgSize),
 			grpc.MaxCallSendMsgSize(maxCallSendMsgSize),
 		),
+		grpc.WithTransportCredentials(insecure.NewCredentials()),


insecure.NewCredentials() 被无条件加入 dialOpts，之后若命中 TLS 分支，又 append 了 credentials.NewTLS(cfg)。gRPC 不允许同一连接同时配置两套 transport credentials，TLS 场景完全失效。原代码是互斥的 else if ... else 结构，PR 把互斥性破坏了。server.go 存在同样问题。

建议恢复互斥结构：

if tlsConfRaw, ok := url.GetAttribute(constant.TLSConfigKey); ok { // TLS 路径 } else { dialOpts = append(dialOpts, grpc.WithTransportCredentials(insecure.NewCredentials())) }

好的感谢建议请问一下这样更改可以吗

简单总结一下就是

var transportCreds credentials.TransportCredentials transportCreds = insecure.NewCredentials() // 默认值 if url 有 TLSConfigKey && TLS 有效 && cfg != nil { transportCreds = credentials.NewTLS(cfg) // 覆盖，不是 append } dialOpts = append(dialOpts, grpc.WithTransportCredentials(transportCreds)) // 只 append 一次

如果不行的话我就直接改回去了

AlexStocks · 2026-03-07T06:14:47Z

protocol/grpc/client.go

+	if rpcService, ok := url.GetAttribute(constant.RpcServiceKey); ok {
+		consumerService = rpcService
 	}
 	invoker := getInvoker(consumerService, conn)


getInvoker 内部：reflect.ValueOf(nil).MethodByName("GetDubboStub") 当 consumerService 为 nil 时会直接 panic，根本不会返回 nil 供调用方检查。if invoker != nil 这个 nil guard 加在 panic 之后，毫无意义。另外返回 reflect.Value{} 后续调用 invoker.Call(...) 同样会 panic。

建议：在调用 getInvoker 前检查：

if consumerService == nil { conn.Close() return nil, fmt.Errorf("grpc: no rpc service found for interface=%s", key) }

感谢建议已更改

AlexStocks · 2026-03-07T06:15:04Z

protocol/dubbo3/dubbo3_protocol.go

 	}
+	if service == nil {
+		logger.Errorf("no rpc service found in url attribute %s for service key: %s", constant.RpcServiceKey, key)
+		return nil


Export 提前 return nil 导致 exporter 没有写入 map，grpc/server.go 的 waitGrpcExporter 会等待 exporter 数量与 providerServices 数量一致，因超时最终触发 panic("wait grpc exporter timeout")，真正原因（缺少 RpcServiceKey）被完全隐藏，排查极难。

调用方（server/action.go、config/service_config.go）会把 nil 当错误向上传递，但中间层 filter chain 不一定做了 nil 检查，存在隐患。

感谢建议已更改

现将 service == nil 时的处理从 logger.Errorf + return nil 改为 panic 直接暴露出原因

AlexStocks · 2026-03-07T06:15:21Z

protocol/dubbo3/dubbo3_invoker.go

 	)
-	// TODO: Temporary compatibility with old APIs, can be removed later
-	rt = config.GetConsumerConfig().RequestTimeout
+	rt = global.DefaultConsumerConfig().RequestTimeout


原来 config.GetConsumerConfig().RequestTimeout 若无配置返回空字符串，由 url.GetParamDuration 处理默认值，用户可以明确禁用超时（传零值）。改成 global.DefaultConsumerConfig().RequestTimeout 后始终是 "3s"，超时无法被禁用，语义变了。

建议：先检查 ConsumerConfig attribute 是否存在，不存在时才使用 DefaultConsumerConfig()。

感谢建议目前已经是这个逻辑了可以看看是否有问题

rt = global.DefaultConsumerConfig().RequestTimeout // 先用默认值 "3s" if consumerConfRaw, ok := url.GetAttribute(constant.ConsumerConfigKey); ok { if consumerConf, ok := consumerConfRaw.(*global.ConsumerConfig); ok { rt = consumerConf.RequestTimeout // 有用户配置则覆盖 } }

AlexStocks · 2026-03-07T06:15:42Z

protocol/grpc/server.go

+		}

 		ds.SetProxyImpl(invoker)
 		server.RegisterService(ds.ServiceDesc(), service)


类型断言 ds, ok := service.(DubboGrpcService) 已经得到了 ds，但最终传给 server.RegisterService 的仍是 service（类型 any）而非 ds。gRPC 的 RegisterService 需要第二个参数满足 service descriptor handler 签名，传 any 类型不一定满足，运行时可能失败。原代码传的是经过 config.GetProviderService(key) 拿到的具体对象，类型一致性有保障。

建议改为：

server.RegisterService(ds.ServiceDesc(), ds)

感谢建议已更改

nanjiek · 2026-03-07T14:44:53Z

protocol/grpc/client.go

+	} else {
+		logger.Warnf("grpc client invoker is nil, interface=%s", key)
+	}


logger只能记录，程序在初始化 invoker（RPC 调用执行器）时发现对象为 nil应该终止，返回 error，阻止 Client 被创建。

nanjiek · 2026-03-07T14:45:57Z

protocol/grpc/client.go

+	if invoker != nil {
+		invokerValue = reflect.ValueOf(invoker)
+	} else {
+		logger.Warnf("grpc client invoker is nil, interface=%s", key)


Suggested change

logger.Warnf("grpc client invoker is nil, interface=%s", key)

err := fmt.Errorf("grpc client invoker is nil, interface=%s", key)

logger.Errorf("failed to get grpc client invoker: %v", err)

return nil, err

感谢建议已更改

nanjiek

核心环节须 Fail-fast，严禁带着错误状态继续运行触发 Panic。

sonarqubecloud · 2026-03-08T07:59:57Z

Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

Snow-kal added 5 commits March 2, 2026 21:23

feat(dubbo3): remove config dependency in invoke

ba40a9d

refactor(protocol): remove config dependency in protocol

6949875

refactor(protocol/grpc): remove config dependency from grpc client

1edfba6

refactor(protocol/grpc): remove config dependency from grpc server

52c424e

style: imports-formatter

a3141de

Copilot AI review requested due to automatic review settings March 6, 2026 05:26

Copilot started reviewing on behalf of Snow-kal March 6, 2026 05:27 View session

Copilot AI reviewed Mar 6, 2026

View reviewed changes

protocol/dubbo3/dubbo3_protocol.go Show resolved Hide resolved

Snow-kal added 3 commits March 6, 2026 13:40

fix(dubbo3): Fix inconsistent state issue in Export method

e69f65a

fix(dubbo3): defer exporter registration and resolve tracing ok shado…

52c5f7c

…w in openServer

style: imports-formatter

929f78b

Alanxtl linked an issue Mar 6, 2026 that may be closed by this pull request

[Enhancement] Complete delete config package #3204

Open

Alanxtl added 🧹 Updates 3.3.2 version 3.3.2 labels Mar 6, 2026

Alanxtl reviewed Mar 6, 2026

View reviewed changes

feat: improve log message

fa72155

AlexStocks requested changes Mar 7, 2026

View reviewed changes

AlexStocks reviewed Mar 7, 2026

View reviewed changes

nanjiek reviewed Mar 7, 2026

View reviewed changes

nanjiek suggested changes Mar 7, 2026

View reviewed changes

Snow-kal added 4 commits March 8, 2026 13:32

fix(grpc): fix TLS credentials not being applied

ba2865d

fix(grpc): fix TLS credentials not being applied in client

a5e3872

fix(grpc): nil check consumerService before getInvoker to avoid panic

61db6a4

fix(grpc): pass typed ds instead of any to RegisterService

fa90d90

Snow-kal added 4 commits March 8, 2026 15:40

fix(grpc): return error when invoker is nil

7ed7b6d

style: imports-formatter

8d61f98

fix(grpc): set RpcServiceKey in tests to fix nil invoker

ff4bf1d

style: imports-formatter

5f69867

	logger.Errorf("no rpc service found in url attribute %s for service key: %s", constant.RpcServiceKey, key)
	logger.Errorf("[Triple Protocol] No rpc service found in url attribute %s for service key: %s", constant.RpcServiceKey, key)

Conversation

Snow-kal commented Mar 6, 2026 • edited by Alanxtl Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Checklist

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

codecov-commenter commented Mar 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AlexStocks left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Snow-kal Mar 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nanjiek Mar 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nanjiek left a comment

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud bot commented Mar 8, 2026

Quality Gate passed

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Snow-kal commented Mar 6, 2026 •

edited by Alanxtl

Loading

codecov-commenter commented Mar 6, 2026 •

edited

Loading

Snow-kal Mar 8, 2026 •

edited

Loading

nanjiek Mar 7, 2026 •

edited

Loading