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

Make sure that the process can be started in the errored state #94

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

Conversation

mrcnkoba
Copy link

@mrcnkoba mrcnkoba commented Apr 23, 2024

Make sure that TelemetryMetricsStatsD can be started if host can't be resolved

If the StatsD collector is not up for some reason and the application can't resolve
the name by calling :inet.gethostbyname(host) then it prevents the entire application from starting.

This commit makes sure that the TelemetryMetricsStatsD can be started in the errored state
and depending on the host_resolution_interval it can be recovered or not.

If host_resolution_interval isn't passed, then TelemetryMetricsStatsD will not try to resolve the host again.
However, when it is passed, it can try to recover it in host_resolution_interval milliseconds

@mrcnkoba mrcnkoba force-pushed the fix/dont_crash_when_statsd_is_down branch from e615e0b to 9bcf689 Compare April 23, 2024 10:39
… resolved

If the StatsD collector is not up for some reason and the application can't resolve
the name by calling `:inet.gethostbyname(host)` then it prevents the entire application from starting.

This commit makes sure that the TelemetryMetricsStatsD can be started in the errored state
and depending on the `host_resolution_interval` it can be recovered or not.

If `host_resolution_interval` isn't passed, then TelemetryMetricsStatsD will not try to resolve the host again.
However, when it is passed, it can try to recover it in `host_resolution_interval` milliseconds.
@mrcnkoba mrcnkoba force-pushed the fix/dont_crash_when_statsd_is_down branch from 9bcf689 to a7fb8d1 Compare April 24, 2024 13:23
juanperi added a commit to juanperi/telemetry_metrics_statsd that referenced this pull request May 21, 2024
This PR is much less ambitious than beam-telemetry#94. In this case, we just fix the
issue when the process is already started, but an error in the
resolution of the hostname leads to an incorrect internal state.

Without this patch, the `new_state` will be `:ok`, as that is the return
from the logger function.
Afterwards, when going to the `terminate` callback, then we try to call
`EventHandler.detach(state.handler_ids)`, which translates to
`EventHandler.detach(:ok.handler_ids)` and fails
arkgil pushed a commit that referenced this pull request Jun 2, 2024
This PR is much less ambitious than #94. In this case, we just fix the
issue when the process is already started, but an error in the
resolution of the hostname leads to an incorrect internal state.

Without this patch, the `new_state` will be `:ok`, as that is the return
from the logger function.
Afterwards, when going to the `terminate` callback, then we try to call
`EventHandler.detach(state.handler_ids)`, which translates to
`EventHandler.detach(:ok.handler_ids)` and fails
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

1 participant