fix(router): call BuildRouterChain method when creating directory. by yangpixi · Pull Request #3225 · apache/dubbo-go

yangpixi · 2026-02-28T07:39:47Z

Description

Call BuildRouterChain method when creating static directory to ensure router capabilities work properly.
Currently, dubbo-go does not support static router configuration, with the exception of the Tag Router. (Pending #3219 to support this).

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

…ory.

codecov-commenter · 2026-02-28T09:24:16Z

Codecov Report

❌ Patch coverage is 16.66667% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 47.93%. Comparing base (35ea886) to head (248a796).
⚠️ Report is 15 commits behind head on develop.

Files with missing lines	Patch %	Lines
cluster/directory/static/directory.go	25.00%	2 Missing and 1 partial ⚠️
client/action.go	0.00%	2 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #3225      +/-   ##
===========================================
- Coverage    47.99%   47.93%   -0.06%     
===========================================
  Files          463      463              
  Lines        33727    33816      +89     
===========================================
+ Hits         16187    16210      +23     
- Misses       16237    16297      +60     
- Partials      1303     1309       +6

☔ 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.

Copilot

Pull request overview

This PR fixes static-directory routing initialization by building the router chain at static.NewDirectory creation time, so router-based behaviors (notably Tag Router) can take effect when no registry is used (Fixes #3222).

Changes:

Call BuildRouterChain during static directory construction instead of only setting invokers on the default chain.
Update multiple cluster tests to blank-import the Tag Router so router factories are registered during test runs.
Tidy go.mod to mark github.com/cenkalti/backoff/v4 as a direct dependency.

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
go.mod	Moves `cenkalti/backoff/v4` to direct `require` (matches direct imports in code).
cluster/directory/static/directory.go	Builds router chain during static directory creation.
cluster/directory/static/directory_test.go	Adds Tag Router blank import for router-factory registration in tests.
cluster/cluster/available/cluster_invoker_test.go	Adds Tag Router blank import for router-factory registration in tests.
cluster/cluster/broadcast/cluster_invoker_test.go	Adds Tag Router blank import for router-factory registration in tests.
cluster/cluster/failback/cluster_test.go	Adds Tag Router blank import for router-factory registration in tests.
cluster/cluster/failfast/cluster_test.go	Adds Tag Router blank import for router-factory registration in tests.
cluster/cluster/failover/cluster_test.go	Adds Tag Router blank import for router-factory registration in tests.
cluster/cluster/failover/cluster_invoker_test.go	Adds Tag Router blank import for router-factory registration in tests.
cluster/cluster/failsafe/cluster_test.go	Adds Tag Router blank import for router-factory registration in tests.
cluster/cluster/forking/cluster_test.go	Adds Tag Router blank import for router-factory registration in tests.
cluster/cluster/zoneaware/cluster_invoker_test.go	Adds Tag Router blank import for router-factory registration in tests.

Comments suppressed due to low confidence (1)

cluster/directory/static/directory_test.go:34

The tests in this file still only assert that List() returns all invokers; they don’t validate the regression fixed by this PR (router chain being built for static directories). Consider extending TestStaticDirList (or adding a new test) to configure tag-based routing (e.g., set dubbo.tag on invoker URLs and set invocation attachment dubbo.tag) and assert that List() returns only the routed subset. That would fail under the old implementation and pass with BuildRouterChain.

import (
	_ "dubbo.apache.org/dubbo-go/v3/cluster/router/tag"
	"dubbo.apache.org/dubbo-go/v3/common"
	"dubbo.apache.org/dubbo-go/v3/protocol/base"
	"dubbo.apache.org/dubbo-go/v3/protocol/invocation"
)

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

Copilot · 2026-02-28T16:47:22Z

cluster/directory/static/directory.go

+	err := dir.BuildRouterChain(invokers, url)
+	if err != nil {
+		logger.Error(err)


BuildRouterChain returns an error for len(invokers)==0, and NewDirectory logs it as an error. If NewDirectory can be called with an empty invoker list (e.g., during startup or transient states), this will introduce noisy error logs for a non-exceptional condition. Consider skipping router-chain building (and/or logging at debug level) when len(invokers)==0.

Suggested change

err := dir.BuildRouterChain(invokers, url)

if err != nil {

logger.Error(err)

if len(invokers) > 0 {

if err := dir.BuildRouterChain(invokers, url); err != nil {

logger.Error(err)

}

Copilot · 2026-02-28T16:47:22Z

cluster/cluster/failover/cluster_test.go

 	clusterpkg "dubbo.apache.org/dubbo-go/v3/cluster/cluster"
 	"dubbo.apache.org/dubbo-go/v3/cluster/directory/static"
 	"dubbo.apache.org/dubbo-go/v3/cluster/loadbalance/random"
+	_ "dubbo.apache.org/dubbo-go/v3/cluster/router/tag"


The new blank import of cluster/router/tag is duplicated within the same failover test package (it’s also imported in cluster_invoker_test.go). Only one blank import is needed per package; consider keeping it in a single shared _test.go (e.g., init_test.go) to avoid repetition.

Suggested change

_ "dubbo.apache.org/dubbo-go/v3/cluster/router/tag"

Copilot · 2026-02-28T16:47:23Z

cluster/directory/static/directory.go

-	dir.RouterChain().SetInvokers(invokers)
+	err := dir.BuildRouterChain(invokers, url)
+	if err != nil {
+		logger.Error(err)


NewDirectory now calls BuildRouterChain and only logs on error, but does not fall back to setting the router chain invokers. If chain.NewRouterChain(url) fails (e.g., no router factories registered), dir.RouterChain() remains the default empty chain.RouterChain{} with no invokers, so List() will route against an empty chain and can return an empty slice even though dir.invokers is populated. Consider always calling dir.RouterChain().SetInvokers(invokers) (or setting invokers on the existing chain) as a fallback when BuildRouterChain fails, so behavior matches the previous implementation when routing is unavailable.

Suggested change

logger.Error(err)

logger.Error(err)

// Fallback: ensure any existing router chain still sees the invokers

if routerChain := dir.RouterChain(); routerChain != nil {

routerChain.SetInvokers(invokers)

}

Alanxtl · 2026-03-01T13:17:26Z

cluster/directory/static/directory.go

+		if routerChain := dir.RouterChain(); routerChain != nil {
+			routerChain.SetInvokers(invokers)


如果routerChain == nil是不是需要log一下

这里貌似还有点问题，我再考虑下

…equals 0

sonarqubecloud · 2026-03-03T13:37:32Z

Quality Gate passed

Issues
9 New issues
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

AlexStocks · 2026-03-03T18:41:56Z

cluster/directory/static/directory.go

-		if routerChain := dir.RouterChain(); routerChain != nil {
-			routerChain.SetInvokers(invokers)
-		}
+		dir.RouterChain().SetInvokers(invokers)


没看出来这行改动的意义

如果BuildRouterChain失败了，dir里面的routerChain就是nil，触发不了if语句了

Alanxtl

lgtm

yangpixi added 3 commits February 28, 2026 15:29

fix(router): call BuildRouterChain method when creating static direct…

9566420

…ory.

style: use imports-formatter to format code

eb71153

ci: register router factory method in unit test.

5f2fa04

yangpixi changed the title ~~fix(router): call BuildRouterChain method when creatiing directory.~~ fix(router): call BuildRouterChain method when creating directory. Feb 28, 2026

ci: import tag router package to resolve no router factory problem.

664bce8

AlexStocks requested review from Alanxtl, No-SilverBullet and Copilot and removed request for Alanxtl and Copilot February 28, 2026 16:43

AlexStocks added the 3.3.2 version 3.3.2 label Feb 28, 2026

Copilot started reviewing on behalf of AlexStocks February 28, 2026 16:43 View session

Copilot AI reviewed Feb 28, 2026

View reviewed changes

fix: add failback, remove unnecessary import

ea4b37f

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

[BUG] 创建staticDirectory的时候routerChain未完全初始化 #3222

Closed

1 task

Alanxtl added the ☢️ Bug label Mar 1, 2026

Alanxtl reviewed Mar 1, 2026

View reviewed changes

fix: refactor failback mechanic, return the exception that len(URLs) …

248a796

…equals 0

yangpixi requested a review from Alanxtl March 3, 2026 14:25

AlexStocks reviewed Mar 3, 2026

View reviewed changes

Alanxtl approved these changes Mar 6, 2026

View reviewed changes

AlexStocks approved these changes Mar 6, 2026

View reviewed changes

AlexStocks merged commit 9d59133 into apache:develop Mar 6, 2026
8 checks passed

yangpixi deleted the fix-static-routerChain branch March 9, 2026 13:18

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(router): call BuildRouterChain method when creating directory.#3225

fix(router): call BuildRouterChain method when creating directory.#3225
AlexStocks merged 6 commits intoapache:developfrom
yangpixi:fix-static-routerChain

yangpixi commented Feb 28, 2026

Uh oh!

codecov-commenter commented Feb 28, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Feb 28, 2026

Uh oh!

Copilot AI Feb 28, 2026

Uh oh!

Copilot AI Feb 28, 2026

Uh oh!

Alanxtl Mar 1, 2026

Uh oh!

yangpixi Mar 3, 2026

Uh oh!

sonarqubecloud bot commented Mar 3, 2026

Uh oh!

AlexStocks Mar 3, 2026

Uh oh!

yangpixi Mar 4, 2026

Uh oh!

Alanxtl left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

		if routerChain := dir.RouterChain(); routerChain != nil {
		routerChain.SetInvokers(invokers)

Conversation

yangpixi commented Feb 28, 2026

Description

Checklist

Uh oh!

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

Uh oh!

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 28, 2026

Choose a reason for hiding this comment

Uh oh!

Alanxtl Mar 1, 2026

Choose a reason for hiding this comment

Uh oh!

yangpixi Mar 3, 2026

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud bot commented Mar 3, 2026

Quality Gate passed

Uh oh!

AlexStocks Mar 3, 2026

Choose a reason for hiding this comment

Uh oh!

yangpixi Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

Alanxtl left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

codecov-commenter commented Feb 28, 2026 •

edited

Loading