kit icon indicating copy to clipboard operation
kit copied to clipboard

Tracing: NATS support

Open sergey-suslov opened this issue 5 years ago • 7 comments

Hi. I would like to add NATS support in the go-kit tracing.

How to pass a span context?

Unlike gRPC or HTTP packages, NATS doesn't have any headers or metadata (as I know) to pass content besides request body. Due to this, I see the only solution is to pass the span context within the message data.

There are two options, that come to mind:

  • pass the context and the data in separate fields (tracing has to be plugged in on the other end)
type natsMessageWithContext struct {
	Sc   model.SpanContext `json:"sc"`
	Data interface{}       `json:"data"`
}
  • pass the context among other data fields (doesn't look good, from my point of view)

For the rest, I suggest to do by analogy with the gRPC tracing and HTTP tracing solutions.

Is there something, that may be a problem? Is this solution acceptable?

Samples: Publisher - https://github.com/sergey-suslov/apartments-booking/blob/feature/nats-tracing-as-options/services/booking/pkg/nats-tracing/nats.go Subscriber - https://github.com/sergey-suslov/apartments-booking/blob/feature/nats-tracing-as-options/services/apartments/pkg/nats-tracing/nats.go

sergey-suslov avatar Aug 19 '20 12:08 sergey-suslov

I think it's a good idea, but I don't think the code necessarily needs to live in the Go kit repo. Can you prototype it somewhere and point me to it for review? If it's making sense, I'm happy to link to it from Go kit in the relevant places.

peterbourgon avatar Aug 19 '20 15:08 peterbourgon

I think it's a good idea, but I don't think the code necessarily needs to live in the Go kit repo. Can you prototype it somewhere and point me to it for review? If it's making sense, I'm happy to link to it from Go kit in the relevant places.

Ok. I'll prototype it and notify you when it's ready.

sergey-suslov avatar Aug 20 '20 00:08 sergey-suslov

I quickly glanced at some items... For pub/sub you can use the producer and consumer kinds.

For more info on pubsub tracing with Zipkin, check out: https://zipkin.io/pages/instrumenting (specifically the Message Tracing section).

basvanbeek avatar Aug 20 '20 11:08 basvanbeek

I quickly glanced at some items... For pub/sub you can use the producer and consumer kinds.

For more info on pubsub tracing with Zipkin, check out: https://zipkin.io/pages/instrumenting (specifically the Message Tracing section).

Thanks.

sergey-suslov avatar Aug 20 '20 12:08 sergey-suslov

Hi @sergey-suslov, just as a heads up that after almost 10 years the NATS protocol has been updated to support headers (here: https://github.com/nats-io/nats.go/pull/560), although this has not been released yet in the server nor have we tagged the client that includes it. Also your approach might be the most compatible with previous versions of the client without having to rely on the latest version of the NATS client, but just thought I'd share that native support for headers is coming in one of the next releases as well.

wallyqs avatar Aug 21 '20 04:08 wallyqs

Hi @sergey-suslov, just as a heads up that after almost 10 years the NATS protocol has been updated to support headers (here: nats-io/nats.go#560), although this has not been released yet in the server nor have we tagged the client that includes it. Also your approach might be the most compatible with previous versions of the client without having to rely on the latest version of the NATS client, but just thought I'd share that native support for headers is coming in one of the next releases as well.

Big thanks. Anyway, I am gonna implement the "in-message context transporting" approach. Then, I believe, using headers as a context holder should be added as an option.

sergey-suslov avatar Aug 21 '20 07:08 sergey-suslov

@peterbourgon I have made a prototype https://github.com/sergey-suslov/go-kit-nats-zipkin-tracing What do you think?

sergey-suslov avatar Aug 22 '20 10:08 sergey-suslov