We are seeing panics in 0.39.0

[dontlaugh]

Go version 1.25.1

Sentry SDK version: 0.39.0

This doesn't show exactly what's wrong, but this is how we're logging currently.

		defer func() {
			if r := recover(); r != nil {
				logrus.WithField("panic", r).Warn("panic recovered while sending to Sentry")
			}
		}()

		fields := entry.Data
		if fields != nil && fields["dd.trace_id"] != nil {
			fields["trace_url"] = fmt.Sprintf("https://us5.datadoghq.com/apm/trace/%d", fields["dd.trace_id"])
		}
		sentry.WithScope(func(scope *sentry.Scope) {
			scope.SetContext("additional_fields", fields)
			sentry.CaptureException(err)
		})

We're seeing panics from sentry-go tracing code, and it seems recent.

Given the snippet above, what we think is happening is we're passing in a custom error type (or perhaps something in the fields map) and one of those objects causes a panic in json serialization.

Looking at the changes in 0.39.0, the only other idea I have at the moment is a possibly nil transport.

[linear]

GO-103

[dontlaugh]

I am attaching a subset of logs as a TSV. These show the stack trace in this library.

panic-logs.tsv

[giortzisg]

Hey @dontlaugh,

This might be happening due to the new transport we introduced with v0.38.0. You can try and use the flag disableTelemetryBuffer and see if that mitigates the issue for now, while we try and find the cause.

[giortzisg]

Are you by any change mutating the fields passed on SetContext? We don't deep-copy the values passed, so that might explain the panic.

[dontlaugh]

Are you by any change mutating the fields

I will need to check on this. We do build up a map of fields incrementally before calling our instrumentation.

But we've been logging the same way for quite some time.

[jerbob92]

I'm seeing the same. Afaik we don't do anything special.
We create a hub using sentry.CurrentHub().Clone() and put it on the context using sentry.SetHubOnContext() in a middleware function, this is basically the code of the Sentry error handler middleware.

	err := doInner(ctx) // doInner is the rest of the middleware/job execution.
	if err == nil {
		return
	}
	hub := sentry.GetHubFromContext(ctx)
	hub.CaptureException(errors.WithStack(err))

We also use the same hub in the tracing part in a separate middleware.

	hub := sentry.GetHubFromContext(ctx)
	hub.ConfigureScope(func(scope *sentry.Scope) {
		scope.SetRequestBody(job.EncodedArgs)
		scope.SetTag("task_type", job.Kind)
		scope.SetTag("task_id", fmt.Sprintf("river_%d", job.ID))
	})

	transactionSource := sentry.SourceTask

	options := []sentry.SpanOption{
		sentry.WithOpName("queue.task"),
		sentry.WithTransactionSource(transactionSource),
		sentry.WithDescription(job.Kind),
	}

	transaction := sentry.StartTransaction(ctx,
		job.Kind,
		options...,
	)

	defer func() {
		transaction.Finish()
	}()

	transaction.SetData("task_id", fmt.Sprintf("river_%d", job.ID))
	ctx = transaction.Context()

	err := doInner(ctx) // doInner is the rest of the middleware/job execution.
	if err != nil {
		if errors.Is(err, &river.JobCancelError{}) {
			transaction.Status = sentry.SpanStatusCanceled
		} else {
			transaction.SetData("error", err.Error())
			transaction.Status = sentry.SpanStatusInternalError
			transaction.Sampled = sentry.SampledTrue
		}
	} else {
		transaction.Status = sentry.SpanStatusOK
	}

Note: I'm on version v0.38.0

[bpapillon]

You can try and use the flag disableTelemetryBuffer and see if that mitigates the issue for now, while we try and find the cause.

For the record, I believe that this change mitigated the issue for us for now (I work with @dontlaugh).

I do think there are some scenarios wherein we may be mutating log fields; e.g. defining a fields map to use across multiple log lines and adding additional fields to the map as new relevant-to-logging information emerges within the process. We have a light wrapper around the logger and could fairly copy the map before logging to avoid this.

[jerbob92]

I tried to make a reproducer but I couldn't do it