From 37b97b3a682ea8496a98e62d3f2b89906a3eaa64 Mon Sep 17 00:00:00 2001
From: Conrad Ludgate <conrad@neon.tech>
Date: Tue, 19 Nov 2024 13:58:11 +0000
Subject: [PATCH] chore(local_proxy): reduce some startup logging (#9798)

Currently, local_proxy will write an error log if it doesn't find the
config file. This is expected for startup, so it's just noise. It is an
error if we do receive an explicit SIGHUP though.

I've also demoted the build info logs to be debug level. We don't need
them in the compute image since we have other ways to determine what
code is running.

Lastly, I've demoted SIGHUP signal handling from warn to info, since
it's not really a warning event.

See https://github.com/neondatabase/cloud/issues/10880 for more details
---
 proxy/src/bin/local_proxy.rs | 37 ++++++++++++++++++++++++++++++------
 proxy/src/jemalloc.rs        |  2 +-
 proxy/src/signals.rs         |  4 ++--
 3 files changed, 34 insertions(+), 9 deletions(-)

diff --git a/proxy/src/bin/local_proxy.rs b/proxy/src/bin/local_proxy.rs
index fbdb1dec15..41b0e11e85 100644
--- a/proxy/src/bin/local_proxy.rs
+++ b/proxy/src/bin/local_proxy.rs
@@ -32,11 +32,12 @@ project_git_version!(GIT_VERSION);
 project_build_tag!(BUILD_TAG);
 
 use clap::Parser;
+use thiserror::Error;
 use tokio::net::TcpListener;
 use tokio::sync::Notify;
 use tokio::task::JoinSet;
 use tokio_util::sync::CancellationToken;
-use tracing::{error, info, warn};
+use tracing::{debug, error, info, warn};
 use utils::sentry_init::init_sentry;
 use utils::{pid_file, project_build_tag, project_git_version};
 
@@ -124,8 +125,8 @@ async fn main() -> anyhow::Result<()> {
 
     Metrics::install(Arc::new(ThreadPoolMetrics::new(0)));
 
-    info!("Version: {GIT_VERSION}");
-    info!("Build_tag: {BUILD_TAG}");
+    debug!("Version: {GIT_VERSION}");
+    debug!("Build_tag: {BUILD_TAG}");
     let neon_metrics = ::metrics::NeonMetrics::new(::metrics::BuildInfo {
         revision: GIT_VERSION,
         build_tag: BUILD_TAG,
@@ -305,26 +306,46 @@ fn build_auth_backend(
     Ok(Box::leak(Box::new(auth_backend)))
 }
 
+#[derive(Error, Debug)]
+enum RefreshConfigError {
+    #[error(transparent)]
+    Read(#[from] std::io::Error),
+    #[error(transparent)]
+    Parse(#[from] serde_json::Error),
+    #[error(transparent)]
+    Validate(anyhow::Error),
+}
+
 async fn refresh_config_loop(path: Utf8PathBuf, rx: Arc<Notify>) {
+    let mut init = true;
     loop {
         rx.notified().await;
 
         match refresh_config_inner(&path).await {
             Ok(()) => {}
+            // don't log for file not found errors if this is the first time we are checking
+            // for computes that don't use local_proxy, this is not an error.
+            Err(RefreshConfigError::Read(e))
+                if init && e.kind() == std::io::ErrorKind::NotFound =>
+            {
+                debug!(error=?e, ?path, "could not read config file");
+            }
             Err(e) => {
                 error!(error=?e, ?path, "could not read config file");
             }
         }
+
+        init = false;
     }
 }
 
-async fn refresh_config_inner(path: &Utf8Path) -> anyhow::Result<()> {
+async fn refresh_config_inner(path: &Utf8Path) -> Result<(), RefreshConfigError> {
     let bytes = tokio::fs::read(&path).await?;
     let data: LocalProxySpec = serde_json::from_slice(&bytes)?;
 
     let mut jwks_set = vec![];
 
-    for jwks in data.jwks.into_iter().flatten() {
+    fn parse_jwks_settings(jwks: compute_api::spec::JwksSettings) -> anyhow::Result<JwksSettings> {
         let mut jwks_url = url::Url::from_str(&jwks.jwks_url).context("parsing JWKS url")?;
 
         ensure!(
@@ -367,7 +388,7 @@ async fn refresh_config_inner(path: &Utf8Path) -> anyhow::Result<()> {
             }
         }
 
-        jwks_set.push(JwksSettings {
+        Ok(JwksSettings {
             id: jwks.id,
             jwks_url,
             provider_name: jwks.provider_name,
@@ -381,6 +402,10 @@ async fn refresh_config_inner(path: &Utf8Path) -> anyhow::Result<()> {
         })
     }
 
+    for jwks in data.jwks.into_iter().flatten() {
+        jwks_set.push(parse_jwks_settings(jwks).map_err(RefreshConfigError::Validate)?);
+    }
+
     info!("successfully loaded new config");
     JWKS_ROLE_MAP.store(Some(Arc::new(EndpointJwksResponse { jwks: jwks_set })));
 
diff --git a/proxy/src/jemalloc.rs b/proxy/src/jemalloc.rs
index 0fae78b60c..9888458ee2 100644
--- a/proxy/src/jemalloc.rs
+++ b/proxy/src/jemalloc.rs
@@ -38,7 +38,7 @@ where
 
 impl MetricRecorder {
     pub fn new() -> Result<Self, anyhow::Error> {
-        tracing::info!(
+        tracing::debug!(
             config = config::malloc_conf::read()?,
             version = version::read()?,
             "starting jemalloc recorder"
diff --git a/proxy/src/signals.rs b/proxy/src/signals.rs
index 514a83d5eb..0b675683c0 100644
--- a/proxy/src/signals.rs
+++ b/proxy/src/signals.rs
@@ -2,7 +2,7 @@ use std::convert::Infallible;
 
 use anyhow::bail;
 use tokio_util::sync::CancellationToken;
-use tracing::warn;
+use tracing::{info, warn};
 
 /// Handle unix signals appropriately.
 pub async fn handle<F>(
@@ -22,7 +22,7 @@ where
         tokio::select! {
             // Hangup is commonly used for config reload.
             _ = hangup.recv() => {
-                warn!("received SIGHUP");
+                info!("received SIGHUP");
                 refresh_config();
             }
             // Shut down the whole application.