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: Recover periodic metric readers after forking #1823

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

chrisholmes
Copy link
Contributor

@chrisholmes chrisholmes commented Mar 6, 2025

This PR provides a fix to PeriodicMetricReader so that it continues working after a Ruby process is forked. This problem currently surfaces itself on applications that use forking webservers.

When forking a Ruby process only the thread creating the child process is coped across from the parent process, which means that background threads need to be created for monitoring tools to continue working. This means that, before this fix, PeriodicMetricReader would not work on the child process as the background thread is not copied.

The fix is implemented by overriding the Process._fork method provided in Ruby versions 3.1+. Overriding this method allows the authors of monitoring tools to introduce callbacks on fork events.

An alternative approach, required prior to 3.1, would require clients to call hooks directly from their forking webserver's configuration such as use on_worker_boot in puma.rb. Overriding Process._fork removes the need for this boilerplate.

When resetting the PeriodicMetricReader, the after_fork hook will first read and discard metrics that would have been published by the parent process. This is in order to avoid duplicate publishing of metrics that will be handled by the parent process.

The parent process' PeriodicMetricReader is left running after forking as it is assumed that clients will want to publish metrics from it still.

@chrisholmes chrisholmes changed the title Recover periodic metric readers from forking fix: Recover periodic metric readers from forking Mar 6, 2025
@chrisholmes chrisholmes force-pushed the recover-periodic-metric-readers-from-forking branch 2 times, most recently from 312d1df to 213d166 Compare March 7, 2025 10:42
@chrisholmes chrisholmes force-pushed the recover-periodic-metric-readers-from-forking branch from 213d166 to 857b773 Compare March 7, 2025 10:55
@chrisholmes chrisholmes changed the title fix: Recover periodic metric readers from forking fix: Recover periodic metric readers after forking Mar 7, 2025
@chrisholmes chrisholmes marked this pull request as ready for review March 7, 2025 12:22
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.

1 participant