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

Infinite Loop in the Transaction Profiler #724

Closed
mpass99 opened this issue Oct 1, 2023 · 4 comments · Fixed by #725
Closed

Infinite Loop in the Transaction Profiler #724

mpass99 opened this issue Oct 1, 2023 · 4 comments · Fixed by #725
Assignees

Comments

@mpass99
Copy link

mpass99 commented Oct 1, 2023

Summary

The transaction profiler profiler.go loops infinite if the transaction duration exceeds the profilerRuntimeLimit, causing the application to terminate due to OOM.

Steps To Reproduce

func main() {
	err := sentry.Init(sentry.ClientOptions{EnableTracing: true, ProfilesSampleRate: 1})
	if err != nil {
		fmt.Printf("sentry.Init: %s", err.Error())
		return
	}

	span := sentry.StartSpan(context.Background(), "main", sentry.WithSpanSampled(sentry.SampledTrue))
	time.Sleep(35 * time.Second)
	span.Finish()
}

Expected Behavior

Either:

  • The Sentry SDK is able to profile also transactions lasting longer than 30 seconds without crashing the application.
  • The Sentry SDK discards the profiling of transactions exceeding the profilerRuntimeLimit.

Environment

SDK

  • sentry-go version: v0.24.1
  • Go version: 1.21.1
  • Using Go Modules? yes

Sentry

  • No Sentry Backend required to reproduce the behavior.

Additional context

The following loop runs infinite:

sentry-go/profiler.go

Lines 243 to 251 in 7e37ede

for start.Value != nil {
var bucket = start.Value.(*profileSamplesBucket)
if bucket.relativeTimeNS < relativeStartNS {
break
}
samplesCount += len(bucket.goIDs)
buckets = append(buckets, bucket)
start = start.Prev()
}

The profilerRuntimeLimit defines a limit of 30 seconds. If the transaction lasts longer, the start-30-second-ring-structure is filled completely which causes both break conditions to not apply.

@cleptric
Copy link
Member

cleptric commented Oct 1, 2023

@vaind Can you take a look, please?

@cjcormack
Copy link

#725 fixes this issue for me. Thank you for getting a PR with the fix out so quickly!

@cleptric
Copy link
Member

cleptric commented Oct 4, 2023

The fix was released in 0.25.0. Thanks for confirming the fix @cjcormack 🙏

@mpass99
Copy link
Author

mpass99 commented Oct 4, 2023

I can confirm that release 0.25.0 resolves our issue. Thank you for your quick solution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants