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

PrometheusBuilder::install panics without a thread-local runtime #122

Closed
str4d opened this issue Nov 11, 2020 · 0 comments · Fixed by #123
Closed

PrometheusBuilder::install panics without a thread-local runtime #122

str4d opened this issue Nov 11, 2020 · 0 comments · Fixed by #123
Labels
C-exporter Component: exporters such as Prometheus, TCP, etc. E-simple Effort: simple. T-bug Type: bug.

Comments

@str4d
Copy link
Contributor

str4d commented Nov 11, 2020

PrometheusBuilder::install calls PrometheusBuilder::build_with_exporter to configure the exporter, then creates a tokio runtime and uses it to run the exporter.

In #92 PrometheusBuilder::build (now build_with_exporter) was refactored to call Server::try_bind before starting the server. This means that hyper operations happen outside the exporter future, whereas previously all hyper operations happened inside Runtime::block_on.

The problem with this is that Server::try_bind eventually calls tokio::net::tcp::TcpListener::from_std, which panics if thread-local runtime is not set:

    /// # Panics
    ///
    /// This function panics if thread-local runtime is not set.
    ///
    /// The runtime is usually set implicitly when this function is called
    /// from a future driven by a tokio runtime, otherwise runtime can be set
    /// explicitly with [`Handle::enter`](crate::runtime::Handle::enter) function.

I believe the solution is to create the runtime first, and then enter its context when building the exporter:

    pub fn install(self) -> Result<(), Error> {
        let mut runtime = runtime::Builder::new()
            .basic_scheduler()
            .enable_all()
            .build()?;

        let (recorder, exporter) = runtime.enter(|| self.build_with_exporter())?;
        metrics::set_boxed_recorder(Box::new(recorder))?;
str4d added a commit to str4d/metrics that referenced this issue Nov 11, 2020
@tobz tobz added C-exporter Component: exporters such as Prometheus, TCP, etc. E-simple Effort: simple. T-bug Type: bug. labels Nov 11, 2020
@tobz tobz closed this as completed in #123 Nov 11, 2020
mnpw pushed a commit to mnpw/metrics that referenced this issue Mar 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-exporter Component: exporters such as Prometheus, TCP, etc. E-simple Effort: simple. T-bug Type: bug.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants