Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

使 AbstractConfigRepository#fireRepositoryChange 参数稳定 #91

Closed
freshchen opened this issue Nov 28, 2024 · 4 comments
Closed

使 AbstractConfigRepository#fireRepositoryChange 参数稳定 #91

freshchen opened this issue Nov 28, 2024 · 4 comments

Comments

@freshchen
Copy link

你的特性请求和某个问题有关吗?请描述

维护者们好,我正在给可观测项目 otel 的 javaagent 中加入对 apollo 的支持 https://github.com/open-telemetry/opentelemetry-java-instrumentation/pull/12794,可以从 1.0 支持到 2.3 版本,目前我把 namespace 作为关键属性放在了链路中,但我发现在 main 分支中我使用的 agent 切入点被改变了,这样我就需要针对 2.3 后的版本再做一次适配
image

清晰简洁地描述一下你希望的解决方案

是否可以确保这个方法的第一个参数是 namespace
com.ctrip.framework.apollo.internals.AbstractConfigRepository#fireRepositoryChange

其它背景

下面是我目前的可观测 demo

代码

@ApolloConfigChangeListener
    private void onChange1(ConfigChangeEvent changeEvent) {
        log.info("Custom onChange1 {}", changeEvent);
        testClient.test();
    }

@ApolloConfigChangeListener
private void onChange2(ConfigChangeEvent changeEvent) {
    log.info("Custom onChange2 {}", changeEvent);
    testClient.test1();
}

当我在 UI 更新配置后的链路

image

关键日志

2024-11-28 21:08:09,246 [tid=b7a437719fdde5de64d9bc29c5e3583d sid=7e738bea44a1fa3c] [Apollo-Config-3][INFO] c.b.mapcloud.one.javaagent.test.App.? - Custom onChange2 com.ctrip.framework.apollo.internals.InterestedConfigChangeEvent@4db17048

2024-11-28 21:08:09,248 [tid=b7a437719fdde5de64d9bc29c5e3583d sid=7e738bea44a1fa3c] [Apollo-Config-2][INFO] c.b.mapcloud.one.javaagent.test.App.? - Custom onChange1 com.ctrip.framework.apollo.internals.InterestedConfigChangeEvent@47cc916c

2024-11-28 21:08:09,249 [tid=b7a437719fdde5de64d9bc29c5e3583d sid=7e738bea44a1fa3c] [Apollo-Config-1][INFO] c.c.f.a.s.p.AutoUpdateConfigChangeListener.? - Auto update apollo changed value successfully, new value: false, key: auto.dump.enabled, beanName: app, field: com.baidu.mapcloud.one.javaagent.test.App$$EnhancerBySpringCGLIB$$31ca0440.dumpEnabled

2024-11-28 21:08:09,451 [tid=b7a437719fdde5de64d9bc29c5e3583d sid=7e738bea44a1fa3c] [pool-7-thread-1][INFO] c.b.m.c.openfeign.logger.FeignLog.? - FeignResp client=TestClient#test() ; httpCode=200 ; url=http://localhost:8103/test

2024-11-28 21:08:09,451 [tid=b7a437719fdde5de64d9bc29c5e3583d sid=7e738bea44a1fa3c] [pool-7-thread-2][INFO] c.b.m.c.openfeign.logger.FeignLog.? - FeignResp client=TestClient#test1() ; httpCode=200 ; url=http://localhost:8103/test/1
... ...
@nobodyiam
Copy link
Member

这是在 #70 中引入的变更,当时评估该方法为 apollo-client 内部使用,所以修改了方法签名。对此给您带来的不便非常抱歉,后续应该不会再变更该方法的签名。

@freshchen
Copy link
Author

大佬客气,非常好的 feature 让人很有升级欲望。

作为内部类可以随便改,但我确实没找到比这个内部类更好的采样点。
既然还没有 release 如果可以的话是不是可以把新增的 appId 参数放在最后一个并注视说明下第一个 namespace 参数不动~

@nobodyiam
Copy link
Member

修改参数顺序对语义会有些影响,建议看下是否在参数捕获的时候可以做个判断/兼容

@freshchen
Copy link
Author

好的,先关闭了

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants