Skip to content

Commit

Permalink
ingest storage: propagate only sampled traces (#8436)
Browse files Browse the repository at this point in the history
We've discovered internally at GL with another user of franz-go that the tracing info can be a significant overhead for small records.

This shouldn't be the case for ingest storage, but to be on the safe side, I'm proposing this change now that we know it may be a problem.

While testing this internally I couldn't spot a significant change in the number of bytes transferred.

Signed-off-by: Dimitar Dimitrov <[email protected]>
  • Loading branch information
dimitarvdimitrov authored Aug 19, 2024
1 parent e71a562 commit d380897
Showing 1 changed file with 14 additions and 1 deletion.
15 changes: 14 additions & 1 deletion pkg/storage/ingest/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import (
"github.com/twmb/franz-go/plugin/kotel"
"github.com/twmb/franz-go/plugin/kprom"
"go.opentelemetry.io/otel/propagation"
"go.opentelemetry.io/otel/trace"
)

var (
Expand All @@ -40,6 +41,18 @@ func IngesterPartitionID(ingesterID string) (int32, error) {
return int32(ingesterSeq), nil
}

type onlySampledTraces struct {
propagation.TextMapPropagator
}

func (o onlySampledTraces) Inject(ctx context.Context, carrier propagation.TextMapCarrier) {
sc := trace.SpanContextFromContext(ctx)
if !sc.IsSampled() {
return
}
o.TextMapPropagator.Inject(ctx, carrier)
}

func commonKafkaClientOptions(cfg KafkaConfig, metrics *kprom.Metrics, logger log.Logger) []kgo.Opt {
opts := []kgo.Opt{
kgo.ClientID(cfg.ClientID),
Expand Down Expand Up @@ -85,7 +98,7 @@ func commonKafkaClientOptions(cfg KafkaConfig, metrics *kprom.Metrics, logger lo
}

tracer := kotel.NewTracer(
kotel.TracerPropagator(propagation.NewCompositeTextMapPropagator(propagation.TraceContext{})),
kotel.TracerPropagator(propagation.NewCompositeTextMapPropagator(onlySampledTraces{propagation.TraceContext{}})),
)
opts = append(opts, kgo.WithHooks(kotel.NewKotel(kotel.WithTracer(tracer)).Hooks()...))

Expand Down

0 comments on commit d380897

Please sign in to comment.