Skip to content

Fix(config center)apollo context path error#3331

Merged
AlexStocks merged 9 commits into
apache:developfrom
NeverENG:fix(config_center)apollo_context_path_error
May 27, 2026
Merged

Fix(config center)apollo context path error#3331
AlexStocks merged 9 commits into
apache:developfrom
NeverENG:fix(config_center)apollo_context_path_error

Conversation

@NeverENG
Copy link
Copy Markdown

@NeverENG NeverENG commented May 18, 2026

Description

Fixes #3330

What is the purpose of the change

Fix Apollo config center address handling when the Apollo server is deployed under a context path, for example:

config-center:
  protocol: apollo
  address: 127.0.0.1:8080/config

Previously, dubbo-go parsed /config into url.Path, but the Apollo config center only used url.Location when building the agollo server address. As a result, the final Apollo address became http://127.0.0.1:8080, and the context path was dropped.

Brief changelog

  • Preserve url.Path when building the Apollo config center address.
  • Keep existing behavior unchanged for Apollo addresses without a context path.

Verifying this change

go test ./config_center/apollo

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

Comment thread tools/dubbogo-cli/cmd/testGenCode/template/newApp/go.sum Outdated
Comment thread tools/dubbogo-cli/cmd/testGenCode/template/newDemo/go.sum Outdated
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented May 19, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 52.40%. Comparing base (60d1c2a) to head (45767bb).
⚠️ Report is 805 commits behind head on develop.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #3331      +/-   ##
===========================================
+ Coverage    46.76%   52.40%   +5.64%     
===========================================
  Files          295      492     +197     
  Lines        17172    37788   +20616     
===========================================
+ Hits          8031    19804   +11773     
- Misses        8287    16380    +8093     
- Partials       854     1604     +750     

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

Comment thread config_center/apollo/impl_test.go
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes Apollo config center address construction so that a configured context path (e.g. 127.0.0.1:8080/config) is preserved when building the final Agollo server URL, addressing #3330.

Changes:

  • Append common.URL.Path to each Apollo config center address part when generating the http://... address list.
  • Add unit tests covering address generation with/without a context path, including multi-address input.

Reviewed changes

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

File Description
config_center/apollo/impl.go Preserve the parsed URL path when building Apollo server addresses.
config_center/apollo/impl_test.go Add tests for getAddressWithProtocolPrefix context-path behavior.

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

Comment thread config_center/apollo/impl.go Outdated
Comment thread config_center/apollo/impl.go Outdated
for _, part := range parts {
addr := part
if url.Path != "" {
addr = strings.TrimRight(addr, "/") + "/" + strings.TrimLeft(url.Path, "/")
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] 这里会把仅表示“根路径”的 url.Path == "/" 也当成有效 context path 追加进去。这样像 apollo://127.0.0.1:8080/ 这种输入会从原来的 http://127.0.0.1:8080 变成 http://127.0.0.1:8080/,后续再拼 Apollo 固定 endpoint 时很容易出现多余 /,属于边界回归。建议把全空或全 / 的 path 视为未设置,并补一条 trailing-slash 用例锁住行为。

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@Alanxtl
Copy link
Copy Markdown
Contributor

Alanxtl commented May 21, 2026

pls fix ci fail

1 similar comment
@Alanxtl
Copy link
Copy Markdown
Contributor

Alanxtl commented May 21, 2026

pls fix ci fail

@sonarqubecloud
Copy link
Copy Markdown

Copy link
Copy Markdown
Contributor

@Alanxtl Alanxtl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

for _, part := range parts {
addr := part
if path != "" {
addr = strings.TrimRight(addr, "/") + "/" + path
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] 多地址场景下 url.Path 的归属有歧义。

当配置为 apollo://127.0.0.1:8080,192.168.1.1:8080/config 时,Go 的 net/url 会把 /config 解析到 url.Path,但 127.0.0.1:8080 作为 url.Host 只有第一个地址。当前逻辑会把 path 统一拼到所有逗号分隔的地址上,这是预期行为。

但有个边界 case 需要确认:如果用户配置的是 apollo://127.0.0.1:8080/config1,192.168.1.1:8080/config2(不同节点部署在不同 context path),当前实现会把两个 path 都丢掉(因为 url.Host 只取第一个,url.Path 只取最后一个节点的 path)。

建议在文档或注释中明确说明:context path 必须对所有节点相同,或者考虑在代码中检查并 warn 这种不一致配置。

if path != "" {
addr = strings.TrimRight(addr, "/") + "/" + path
}
if !strings.HasPrefix(part, apolloProtocolPrefix) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] strings.TrimRight(addr, "/") 会把地址中所有尾部 / 都去掉,包括 http:// 协议前缀添加前的原始地址。

当前流程:

  1. addr = part(如 127.0.0.1:8080/
  2. addr = strings.TrimRight(addr, "/") + "/" + path127.0.0.1:8080/config
  3. http:// 前缀 → http://127.0.0.1:8080/config

这个顺序是对的。但如果 part 本身已经带了 http:// 前缀(如 http://127.0.0.1:8080/),TrimRight 不会误伤到 http:// 中的 /,因为 TrimRight 只从右端开始 trim,http:/// 不在右端。

不过建议加个防御性注释说明这个顺序的依赖关系,避免后续维护者调整顺序时引入 bug。

@AlexStocks AlexStocks merged commit 5b2ca83 into apache:develop May 27, 2026
8 checks passed
Alanxtl pushed a commit that referenced this pull request May 30, 2026
* feat(test):Add TestGetAddressWithProtocolPrefixKeepsContext and find the error when user bring context path

* fix(apollo):Fix the test func(getAddressWithProtocolPrefix) fix(context_path):fix getAddressWithProtolPrefix didn't handle context path

* refator(config_center):cleanup-redundant-test

* fix(test):删除不应该存在的文件

* feat():恢复测试并添加多种case

* Potential fix for pull request finding

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

* fix:修复url.Path = /问题并添加边缘测试;修复原来的不符合gofmt格式以通过CI

---------

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Alanxtl pushed a commit that referenced this pull request May 31, 2026
* fix(config): remove legacy protocol timeout fallback (#3326)

* fix(config): remove legacy protocol timeout fallback

* fix(config): avoid default timeout allocation

* fix(config): preserve consumer timeout in reference config

* test(config): satisfy testifylint in reference timeout test

* fix(config): make protocol timeout default explicit

* fix(config): centralize consumer timeout default

* fix(config): keep consumer timeout default in global

* Fix(config center)apollo context path error (#3331)

* feat(test):Add TestGetAddressWithProtocolPrefixKeepsContext and find the error when user bring context path

* fix(apollo):Fix the test func(getAddressWithProtocolPrefix) fix(context_path):fix getAddressWithProtolPrefix didn't handle context path

* refator(config_center):cleanup-redundant-test

* fix(test):删除不应该存在的文件

* feat():恢复测试并添加多种case

* Potential fix for pull request finding

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

* fix:修复url.Path = /问题并添加边缘测试;修复原来的不符合gofmt格式以通过CI

---------

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

* fix(logger): sync dubbo-go logger facade during logger initialization (#3345)

* fix(logger): sync dubbo-go logger facade in LoggerConfig.Init()

* fix(logger): sync dubbo-go logger facade in config_loader init()

* fix(logger): sync dubbo-go logger facade in initGlobalLogger()

* test(logger): verify dubbo-go facade is synced after logger initialization

* style(logger): fix import formatting

* refactor(logger): standardize logger format across codebase (#3344)

* refactor(logger): standardize logger format in graceful_shutdown

- Add [GracefulShutdown] prefix to all logger calls
- Remove decoration symbols (---) from log messages
- Change key format from "error: %v" / "--- %v" to "err=%v"
- Lowercase first letter of all log message bodies

* refactor(logger): standardize logger format in internal, metadata, metrics, otel

- Add module prefixes: [Internal], [Metadata], [MetadataRPC], [MetadataReport][Etcd/Nacos/Zookeeper], [Metrics], [Metrics][Probe/Prometheus/RPC], [OTel][Trace]
- Unify key format: "error: %v" / ": %v" / "err: %s" → "err=%v", "url: %s" → "url=%s"
- Lowercase first letter of all log message bodies
- Fix bug: logger.Error with non-string or extra args → logger.Errorf (listener.go, server.go, exporter.go)
- Fix bug: logger.Errorf with no format args → logger.Error (metadata_service.go)
- Fix bug: logger.Infof/Debugf with no format args → logger.Info/Debug (config.go, report.go)
- Fix: err.Error() + %s → err + %v (nacos/report.go x2)

* refactor(logger): standardize logger format in protocol directory

- Unify prefixes as [Protocol], [Dubbo], [Dubbo][Codec/Hessian2/Impl/Exporter/Invoker], [Dubbo3], [GRPC], [GRPC][Client/Server/Exporter/Invoker], [Jsonrpc], [Jsonrpc][Server/Exporter/Invoker], [ProtocolWrapper], [Rest], [Rest][Config/Exporter/Server], [Triple], [Triple][Client/Server/Exporter/Invoker/CORS/Codec/Handler/Negotiation/Protocol/Health/OpenAPI]
- Unify key format: "error: %v" / ": %v" / "error:{%v}" → "err=%v", "err: %v" → "err=%v"
- Lowercase first letter of all log message bodies
- Remove %+v format for non-Debug levels
- Fix bug: logger.Error with error type → logger.Errorf (dubbo_codec.go, dubbo_protocol.go)
- Fix bug: logger.Error/Info with extra args → logger.Errorf/Infof (rpc_status.go, jsonrpc/server.go)
- Fix bug: logger.Infof/Debugf without format args → logger.Info/Debug (multiple files)
- Fix bug: logger.Debug without format args → logger.Debugf (openapi/service.go)
- Fix: err.Error() + %s → err + %v (dubbo3_protocol.go)

* refactor(logger):standardize logger prefixes in metadata, metadata-report, Dubbo, and Triple protocol modules

* refactor(logger): unify log format in proxy and registry packages

- Add consistent [Proxy]/[Registry]/[Registry][<impl>] prefix to all log messages
- Adopt key=value parameter style (err=%v, url=%s) for structured logging
- Remove redundant descriptions and normalize punctuation across 30 files

* refactor(logger): unify log format in remoting, server and loader modules

- Add consistent [Loader]/[Server]/[Getty]/[Zookeeper]/[Etcdv3] prefixes
- Adopt key=value parameter style (err=%v, url=%s) for structured logging
- Remove trailing punctuation and redundant wording across 21 files

* feat(tools): add loggercheck linter for log format consistency

Introduce a static analysis tool that enforces the standardized logger
conventions across the codebase:

- Rule 1: logger.Info with format verbs → use logger.Infof
- Rule 2: logger.Infof without format verbs → use logger.Info
- Rule 3: logger.Infof(fmt.Sprintf(...)) → redundant, flatten to Infof
- Rule 4: trailing \n / ... / ! in log messages → remove

Integrated into make lint as the logger-check target.

* refactor(logger): final touch, unfiy logger format in other places

* fix(tools): remove unused variables in loggercheck to pass golangci-lint

* refactor: revert (feat(tools): add loggercheck) and simplify registry log prefixes

Remove loggercheck tool and its Makefile integration. Use [Registry] instead of [Registry][Protocol]/[Registry][Exposed] in core registry files; add [Registry][Mock] for mock registry

* fix(logger): correct format verbs, variable names, and function calls

* refactor(logger):modify prefix format

---------

Co-authored-by: CAICAII <3360776475@qq.com>
Co-authored-by: Yuxuan Lv <3656828039@qq.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Nene7ko_ <141395478+XnLemon@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3.3.2 version 3.3.2 ☢️ Bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] apollo配置中心不支持context path 的 Apollo 部署,如10.202.244.91:8080/config地址

5 participants