zipkin-go icon indicating copy to clipboard operation
zipkin-go copied to clipboard

Published 20 hours ago • verified publisher icon openzipkin

kafka reporter sendBatch

Open sixiaobai opened this issue 4 years ago • 6 comments

Signed-off-by: wanglongfei [email protected]

sixiaobai avatar Sep 25 '19 11:09 sixiaobai

Coverage Status

Coverage decreased (-7.0%) to 68.456% when pulling 9907f9b6c866b3f93d8b6ff9a662cc47290073d6 on sixiaobai:master into c29478e51bfb2e9c93e0e9f5e015e5993a490399 on openzipkin:master.

coveralls avatar Sep 25 '19 11:09 coveralls

Thanks for this contribution @sixiaobai, this PR has unbounded goroutines issue that was fixed for the http one at https://github.com/openzipkin/zipkin-go/pull/146 so ideally we should address it here.

I have some other concerns here that I could not find out answer myself for:

  1. Should we enable batching/non batching reporting?
  2. Are the default values for message size good enough?

Any thoughts on this @adriancole @basvanbeek?

jcchavezs avatar Sep 25 '19 12:09 jcchavezs

Will check this PR with @jeqo this wednesday.

jcchavezs avatar Oct 14 '19 10:10 jcchavezs

Should we align this number also in here @jeqo ?

jcchavezs avatar Nov 11 '19 16:11 jcchavezs

@jcchavezs I think so. Will give a try.

jeqo avatar Nov 11 '19 16:11 jeqo

@jcchavezs in order to apply a similar boundary as java reporter, we will need to batch based on bytes size, instead of number of elements. We can check this on a follow up PR.

jeqo avatar Nov 11 '19 18:11 jeqo