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

DisabledHub.StartTransaction now returns NoOpTransaction #3581

Merged
merged 3 commits into from
Sep 3, 2024

Conversation

jamescrosswell
Copy link
Collaborator

Resolves #829

Replaces #3574 (cherry picked commits to target main branch instead of the version-5.0.0 branch.

@bitsandfoxes
Copy link
Contributor

macOS is not happy today

@jamescrosswell
Copy link
Collaborator Author

jamescrosswell commented Aug 30, 2024

macOS is not happy today

Yeah I'm not even seeing any errors in the logs. It runs dotnet restore and then just stops logging after a while.

  • We can't turn on diagnostic logging, since then we run out of disk space 😢
  • We can't upgrade to a larger runner since the larger macOs runners just come with more memory/cpu (not more disk space) 😭

That leaves us in a bit of a pickle... unless we want to host our own runners.

Copy link
Contributor

@bitsandfoxes bitsandfoxes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@jamescrosswell jamescrosswell merged commit 2a0a905 into main Sep 3, 2024
22 checks passed
@jamescrosswell jamescrosswell deleted the disabledhub-nooptransaction-main branch September 3, 2024 21:45
@Joshhua5
Copy link

Can we expand on this and use the NoOpTransaction based on a sampling rate at the start of a transaction and not the end?
Sentry is close to half of all our memory allocations and reducing the sample rate isn't effective at reducing that.

@bitsandfoxes
Copy link
Contributor

Can we expand on this and use the NoOpTransaction based on a sampling rate at the start of a transaction and not the end?

That's a very fair request that I put in #3636. We're moving towards something of a spans-only world but maybe we can sneak this in somewhere in between to make life easier?

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.

DisabledTransaction or DisabledSpan if SDK is disabled, eg empty DSN
3 participants