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

fix(pubsub): move start span logic for publish batch #11193

Merged
merged 3 commits into from
Dec 3, 2024

Conversation

duongcongtoai
Copy link
Contributor

@duongcongtoai duongcongtoai commented Nov 25, 2024

Fix: #11186

Having this span started while tracing is disabled do not directly cause memory leak, but if otel sdk is used together with other sdk such as sentry (which internally store a global map of otel span ID -> sentry span ID), if the span never ends the entries in this map is never GCed, and thus all the span object that they refers to never get GCed

@duongcongtoai duongcongtoai requested review from shollyman and a team as code owners November 25, 2024 21:55
Copy link

google-cla bot commented Nov 25, 2024

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

Copy link

conventional-commit-lint-gcf bot commented Nov 25, 2024

🤖 I detect that the PR title and the commit message differ and there's only one commit. To use the PR title for the commit history, you can use Github's automerge feature with squashing, or use automerge label. Good luck human!

-- conventional-commit-lint bot
https://conventionalcommits.org/

@product-auto-label product-auto-label bot added the api: pubsub Issues related to the Pub/Sub API. label Nov 25, 2024
@duongcongtoai duongcongtoai changed the title fix(pubsub): only init batch span if trace enabled fix(pubsub): memory leak because of unintentional span start logic Nov 25, 2024
@duongcongtoai duongcongtoai changed the title fix(pubsub): memory leak because of unintentional span start logic fix(pubsub): Remove unintentional span start logic Nov 25, 2024
@hongalex hongalex added the kokoro:run Add this label to force Kokoro to re-run the tests. label Dec 2, 2024
@kokoro-team kokoro-team removed the kokoro:run Add this label to force Kokoro to re-run the tests. label Dec 2, 2024
@hongalex hongalex enabled auto-merge (squash) December 3, 2024 00:30
@hongalex hongalex added automerge Merge the pull request once unit tests and other checks pass. kokoro:force-run Add this label to force Kokoro to re-run the tests. labels Dec 3, 2024
@kokoro-team kokoro-team removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Dec 3, 2024
@hongalex hongalex changed the title fix(pubsub): Remove unintentional span start logic fix(pubsub): move start span logic for publish batch Dec 3, 2024
@hongalex hongalex added the kokoro:run Add this label to force Kokoro to re-run the tests. label Dec 3, 2024
@kokoro-team kokoro-team removed the kokoro:run Add this label to force Kokoro to re-run the tests. label Dec 3, 2024
@hongalex hongalex merged commit f843d50 into googleapis:main Dec 3, 2024
5 checks passed
@gcf-merge-on-green gcf-merge-on-green bot removed the automerge Merge the pull request once unit tests and other checks pass. label Dec 3, 2024
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

pubsub: Memory leak if tracing is enabled together with sentry
3 participants