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

pubsub: Memory leak if tracing is enabled together with sentry #11186

Closed
toaiduongdh opened this issue Nov 25, 2024 · 2 comments · Fixed by #11193
Closed

pubsub: Memory leak if tracing is enabled together with sentry #11186

toaiduongdh opened this issue Nov 25, 2024 · 2 comments · Fixed by #11193
Assignees
Labels
api: pubsub Issues related to the Pub/Sub API. triage me I really want to be triaged.

Comments

@toaiduongdh
Copy link

toaiduongdh commented Nov 25, 2024

Client

PubSub

Environment

go version go1.23.1 linux/amd64

Code and Dependencies

package main

import (
	sentryotel "github.com/getsentry/sentry-go/otel"
	"go.opentelemetry.io/contrib/propagators/b3"
	"go.opentelemetry.io/otel"
	"go.opentelemetry.io/otel/attribute"
	"go.opentelemetry.io/otel/exporters/jaeger" //nolint:staticcheck
	"go.opentelemetry.io/otel/propagation"
	"go.opentelemetry.io/otel/sdk/resource"
	sdktrace "go.opentelemetry.io/otel/sdk/trace"
	semconv "go.opentelemetry.io/otel/semconv/v1.9.0"
	"go.opentelemetry.io/otel/trace"
	"cloud.google.com/go/pubsub"
)

  func main() {
	var exporter sdktrace.SpanExporter
	var err error

	splitted := strings.Split("collectorurl", ":")
	exporter, err := jaeger.New(
		jaeger.WithAgentEndpoint(
			jaeger.WithAgentHost(splitted[0]),
			jaeger.WithAgentPort(splitted[1]),
		),
	)

	if err != nil {
		return nil, nil, errors.Wrap(err, "failed to initialize trace exporter")
	}
	batchProcessor := sdktrace.NewBatchSpanProcessor(exporter)
	sampler := sdktrace.ParentBased(sdktrace.TraceIDRatioBased(opts.SampleRate))
	propagator := propagation.NewCompositeTextMapPropagator(
		propagation.TraceContext{},
		b3.New(b3.WithInjectEncoding(b3.B3SingleHeader|b3.B3MultipleHeader)), // For Istio compatibility
	)

	attr := []attribute.KeyValue{
		semconv.ServiceNameKey.String(opts.TracerName),
		semconv.ServiceVersionKey.String(opts.TracerVersion),
		attribute.String(environmentLabel, opts.Environment)}

	attr = append(attr, opts.Attributes...)

	tp := sdktrace.NewTracerProvider(
		sdktrace.WithSampler(sampler),
		sdktrace.WithSpanProcessor(batchProcessor),
		sdktrace.WithSpanProcessor(sentryotel.NewSentrySpanProcessor()),
		sdktrace.WithResource(resource.NewWithAttributes(
			semconv.SchemaURL,
			attr...,
		)),
	)

	otel.SetTracerProvider(tp)
	otel.SetTextMapPropagator(propagator)


	var topic *pubsub.Topic
	// This will cause memory leak
	topic.Publish(context.Background(),nil)
}
go.mod
module modname

go 1.23.0

require (
	cloud.google.com/go/pubsub v1.42.0
	github.com/getsentry/sentry-go v0.25.0
	github.com/getsentry/sentry-go/otel v0.25.0
)

Expected behavior

Span created is correctly garbage collected

Actual behavior

The memory allocated for Span objects never get GCed

Screenshots

image

Additional context

Started after upgrading pubsub v1.38.0 to v1.42.0

The issue may be in either of the project, but this memory leak happen immediately for us after upgrading pubsub client to v1.42.0 which starts to support tracing, and perhaps in some logic the spans were not correctly ended

@toaiduongdh toaiduongdh added the triage me I really want to be triaged. label Nov 25, 2024
@product-auto-label product-auto-label bot added the api: pubsub Issues related to the Pub/Sub API. label Nov 25, 2024
@duongcongtoai
Copy link
Contributor

Potential cause:

_, batcherSpan = startSpan(ctx, batcherSpanName, "")

here batcher span is always started regardless the config enableTracing
but this span only ends within the block checking enableTracing

m.batcherSpan.End()

@hongalex
Copy link
Member

hongalex commented Dec 3, 2024

Thanks for filing this and figuring out where the logic was faulty. Will try to get this merged in this week.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: pubsub Issues related to the Pub/Sub API. triage me I really want to be triaged.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants