apache / rocketmq-client-go

Apache RocketMQ go client
https://rocketmq.apache.org/
Apache License 2.0
1.29k stars 415 forks source link

[Panic] init trace client, the same intstance #1062

Open iGoogle-ink opened 1 year ago

iGoogle-ink commented 1 year ago

The issue tracker is ONLY used for the go client (feature request of RocketMQ need to follow RIP process). Keep in mind, please check whether there is an existing same report before your raise a new one.

Alternately (especially if your communication is not a bug report), you can send mail to our mailing lists. We welcome any friendly suggestions, bug fixes, collaboration, and other improvements.

Please ensure that your bug report is clear and that it is complete. Otherwise, we may be unable to understand it or to reproduce it, either of which would prevent us from fixing the bug. We strongly recommend the report(bug report or feature request) could include some hints as to the following:

BUG REPORT

  1. Other information (e.g. detailed explanation, logs, related issues, suggestions on how to fix, etc): panic log: image

problem code: trace.NewTraceDispatcher() InstanceName is same,so clientID is same,traceConfig can not set InstanceName or UnitName

image

dispatcher not nil,but traceCli is nil,

image

the Start() method,td.running = true panic

image

FEATURE REQUEST

  1. Please describe the feature you are requesting. ClientID() method change to below, id add GroupName image
francisoliverlee commented 1 year ago

can you send a PR to fix it ?

iGoogle-ink commented 1 year ago

can you send a PR to fix it ? OK

cserwen commented 1 year ago

也许通过traceConfig 设置不同的groupName 可以解决这个问题 @iGoogle-ink

iGoogle-ink commented 1 year ago

也许通过traceConfig 设置不同的groupName 可以解决这个问题 @iGoogle-ink

试过,不行,trace用的是 default的,默认写死的一个 group

cserwen commented 1 year ago

也许通过traceConfig 设置不同的groupName 可以解决这个问题 @iGoogle-ink

试过,不行,trace用的是 default的,默认写死的一个 group

你能发下相关的代码在哪里吗,我看的是根据传入的groupName 设置的 @iGoogle-ink

iGoogle-ink commented 1 year ago

也许通过traceConfig 设置不同的groupName 可以解决这个问题 @iGoogle-ink

试过,不行,trace用的是 default的,默认写死的一个 group

你能发下相关的代码在哪里吗,我看的是根据传入的groupName 设置的 @iGoogle-ink

当设置多个 client 时,每个 client 都设置 tracer 时,机会出现这样的问题,因为 tracer 的 client 初始化的InstanceName参数是固定的 "INNER_TRACE_CLIENT_DEFAULT",外界无法配置。如下图: image 但是,我看代码 internal.GetOrNewRocketMQClient() 函数中,actual, loaded := clientMap.LoadOrStore(client.ClientID(), client) 这段代码,client.ClientID() 获取规则里,并没有用到您说的 GroupName,所以即使在初始化 tracer 时设置了 GroupName 也没用,两个 tracer 会冲突报错,返回空 client,于是返回的 dispacher 就是个空指针了panic。 image 如果修改 clientid 变动较大,我可以考虑修改 consumer.WithTrace() 方法的入参 primitive.TraceConfig 结构体,增加 InstanceName 项,并且在初始化方法内,cliOp.InstanceName = "INNER_TRACE_CLIENT_DEFAULT" 改为 if traceCfg.InstanceName != "", cliOp.InstanceName = traceCfg.InstanceName

cserwen commented 1 year ago

@iGoogle-ink 我仔细看下了你的报错截图,你应该是两个客户端使用了不同的namesrv地址导致的,具体报错在这一行 https://github.com/apache/rocketmq-client-go/blob/a15c7771e84b00a33ba6f054d8c31d85654e2dac/internal/client.go#L209 可以在tarcer的instanceName 上默认加上 namesrv地址,具体可以参考 Java SDK 的实现,https://github.com/apache/rocketmq/blob/develop/client/src/main/java/org/apache/rocketmq/client/trace/AsyncTraceDispatcher.java#L108

iGoogle-ink commented 1 year ago

@iGoogle-ink 我仔细看下了你的报错截图,你应该是两个客户端使用了不同的namesrv地址导致的,具体报错在这一行

https://github.com/apache/rocketmq-client-go/blob/a15c7771e84b00a33ba6f054d8c31d85654e2dac/internal/client.go#L209

可以在tarcer的instanceName 上默认加上 namesrv地址,具体可以参考 Java SDK 的实现,https://github.com/apache/rocketmq/blob/develop/client/src/main/java/org/apache/rocketmq/client/trace/AsyncTraceDispatcher.java#L108

你们的源码我看过了,首先panic的直接原因是你们未做判断兜底,没判断是否为nil就直接赋值了。 另外,我们使用的RocketMQ使用的是 阿里云 提供的服务,场景就是可能连接多实例,每个实例确实是不同的 NameSrvs ,而且是未知的,我回头看看吧,你们也不会针对阿里云处理吧?如果不行,我就只能不用 trace 组件了

snail-plus commented 1 year ago

TraceConfig 结构体增加 UnitName 外部使用不同Namesrv 使用不同UnitName 可以fix 这个问题吗?

iGoogle-ink commented 1 year ago

TraceConfig 结构体增加 UnitName 外部使用不同Namesrv 使用不同UnitName 可以fix 这个问题吗?

我是这么改的,只不过加的是 InstanceName,但是好像得不到认同 PR:https://github.com/apache/rocketmq-client-go/pull/1065