Proposal: add tracing support

[yuhui-lin]

Hey, I tried to add tracing for sarama and LevelDB using these two libraries: https://godoc.org/gopkg.in/DataDog/dd-trace-go.v1/contrib/Shopify/sarama https://godoc.org/gopkg.in/DataDog/dd-trace-go.v1/contrib/syndtr/goleveldb/leveldb

but I failed to do so because producer/consumer/leveldb.DB are not exposed directly. #77 adding header to goka context may be helpful but we still can't get traces of emitter/view/groupTable.

I think it would be good to add opentracing support to the options of Emitter/processor/view: https://github.com/opentracing/opentracing-go

func WithEmitterTracer(tracer opentracing.Tracer) EmitterOption
func WithProcessorTracer(tracer opentracing.Tracer) EmitterOption
func WithViewTracer(tracer opentracing.Tracer) ViewOption

any suggestion on how to add tracing to goka for now?

[db7]

The storage is an interface, so you should be able to simply replace it with your extended version of the storage. Take a look in the builders in storage subpackage. The same applies to the Sarama consumer and producer (kafka package).

If possible I'd suggest to introduce the tracing via builders instead of adding another dependency to the main package.

Sami has been working in a rewrite of the storage package. @SamiHiltunen, do you think that the rewrite would help introducing tracing?

[yuhui-lin]

@db7 thanks for the information. I didn't realize they are all interfaces and I'll look into it. I guess it would still make sense if goka provides open-tracing support out of box.

[yuhui-lin]

I looked into storage and kafka subpackage. I am able to apply tracing to storage. but I got two concerns:

• for kafka/consumer, there is a lot of related code: groupConsumer, simpleConsumer, event... these are all unexposed struct. it might be fluky to copy these code for tracing purpose. plus, there is no tracing wrapper for sarama-cluster yet. As you mentioned, builder may be a good way to introduce tracing. I can see adding builders like these may make sense:

func ProducerBuilderWithTracer(config *cluster.Config, tracer opentracing.Tracer) ProducerBuilder
func ConsumerBuilderWithTracer(config *cluster.Config, tracer opentracing.Tracer) ConsumerBuilder

• the other concern is that we are not able to create a child span when emitting a message in processor callback because goka doesn't propagate kafka headers from consumer to producer. #77 issue seems related.

[db7]

Good points. I'd be more than happy reviewing PRs for this issue and #77. If you'd like to try, please let me know. I can support you via chat if you need help to get started.

[yuhui-lin]

sounds good. I'll let you know when I get time for working on this issue.

[frairon]

I'll close it for now, reopen or create a new issue if it comes up again.

[adw1n]

I think there is definitely an interest in using tracing with goka. Right now to use https://godoc.org/gopkg.in/DataDog/dd-trace-go.v1/contrib/Shopify/sarama with a producer you need to implement a custom ProducerBuiler which involves copy pasting the whole producer.go file and changing just one line https://github.com/lovoo/goka/blob/3d39c5afcd0a0d967c49318208e2b313bc0dfc8c/producer.go#L32 to

producer: saramatrace.WrapAsyncProducer(config, aprod),

There is deffinitely an overhead caused by having to maintain copy pasted producer.go file.

@frairon @db7 please could you reopen this issue? Will the goka core team accept a PR that improves the tracing support?

[frairon]

@adw1n sorry for the late response, we're currently all on vacation and I honestly forgot to reply to you before. I'd be happy to review any PRs that would add that. Thanks in advance for the contribution :)