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

Kotel #200

Closed
wants to merge 15 commits into from
Closed

Kotel #200

wants to merge 15 commits into from

Conversation

yianni
Copy link
Contributor

@yianni yianni commented Sep 9, 2022

This PR attempts to add open telemetry tracing to franz-go as a plugin.

@tobiasbrodersen
Copy link
Contributor

tobiasbrodersen commented Sep 12, 2022

Hi

Currently we are looking at instrumenting some of our clients with OpenTelemetry traces and metrics and we would like to write this as a plugin utilising hooks. In the realm of OpenTelemetry the tracing information is carried with the context and you access the span via the context. Thus, if we need to be able to create, lets say, a span named in-produce-buffer we need to be able to get the producing context in the hooks OnProduceRecordBuffered (starting the span) and OnProduceRecordUnbuffered (ending the span), currently it does not seems like the interface offers any mechanism to access the producing context in the hooks.

On the consuming side we need some way of letting the consumer continue the trace. For http handlers it looks something like this

func httpHandler(w http.ResponseWriter, r *http.Request) {
	ctx, span := tracer.Start(r.Context(), "hello-span") // Notice the r.Context()
	defer span.End()

         // do some work to track with hello-span
}

where the context can be extracted from the request and a middleware plugin is responsible for injecting the appropriate information from the HTTP headers into the context before the request is being called by the handler.

We have been thinking about solutions to these obstacles and we would like to get your take on possible solutions too.

As of now, we work around this by adding a Context field to the Record type and setting this context on the record prior to calling produce, but optimally produce should set the context that it is already taken as input and set as Record.Context. On the consumer side we would set a context.Background() that could the be "enriched"/replaced in a hook that injects the OpenTelemetry data from the Record.Headers.

It is important to us that we can "hide" as much logic as possible in the plugin as we will ask our consumer and producer teams to enable OpenTelemetry in their kafka clients and we need to make the onboarding to OpenTelemetry as easy as possible. What are your thought here and how do you propose we proceed?

edit: I'm a colleague of @yianni

@yianni yianni mentioned this pull request Sep 12, 2022
@yianni yianni closed this Sep 23, 2022
twmb pushed a commit that referenced this pull request Oct 6, 2022
See #201 and #200.

Some packages want to stuff information into a context with WithValue,
and then that information can be used to trace spans (see the open
telemetry packages). Without a Context field or something equivalent, it
is not possible to trace spans once a record is handed off to franz-go.
We want to use these spans in franz-go hooks.

We do not need to do anything on the consumer side, but unfortunately,
we need the Context field to be **writable** so that consumers can
initialize the context (and stuff information into it) with WithValue.
So, we just add a new field to the record.
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

Successfully merging this pull request may close these issues.

2 participants