feat: adding a log file support #200

alocay · 2025-07-16T13:19:45Z

Adding support for logging to a file via config option logging.path. When provided the server will attempt to build the log path and then initialize a RollingFileAppender log writer. If any of this failed or if not provided, logging defaults to stderr.

Note: Decided to default to stderr (when not provided) since the server will only be able to log out to specific places which it has permissions for. Since this isn't not easily known, the easiest thing is to defualt to stderr and let the user setup file logging, pointing to a location that it can log to.

apollo-librarian · 2025-07-16T13:19:57Z

✅ Docs preview ready

The preview is ready to be viewed. View the preview

File Changes

0 new, 1 changed, 0 removed

* (developer-tools)/apollo-mcp-server/(latest)/quickstart.mdx

Build ID: 580029315949ed2b60d33392

URL: https://www.apollographql.com/docs/deploy-preview/580029315949ed2b60d33392

crates/apollo-mcp-server/src/runtime.rs

…s own module

crates/apollo-mcp-server/src/log/logging.rs

crates/apollo-mcp-server/src/log.rs

alocay · 2025-07-22T14:00:17Z

Moved logging.rs to src/ and combined with logging setup functions that were added

…bmodules

nicholascioli

Looks good! I left a few nits.

nicholascioli · 2025-07-22T20:31:29Z

crates/apollo-mcp-server/src/main.rs

@@ -1,19 +1,18 @@
 use std::path::PathBuf;

+use crate::runtime::logging::init::setup_logging;


Nit: Re-export the setup_logging method under the logging module and rename it to setup so that it doesn't embed the name of the module into it. Also merge it with the existing use on line 14.

Then you could use it like:

// On line 14 use runtime::{logging, IdOrDefault}; // Wherever you used `setup_logging` logging::setup(...);

Actually, refer to the comment on the logging struct. This should probably be a member function of logging.

nicholascioli · 2025-07-22T20:35:07Z

crates/apollo-mcp-server/src/runtime.rs

 //!
 //! This module is only used by the main binary and provides helper code
 //! related to runtime configuration.

 mod config;
 mod graphos;
 mod introspection;
-mod logging;
+pub(crate) mod logging;


Nit: This can probably be not public if the setup function is a member on the logging struct.

nicholascioli · 2025-07-22T20:36:01Z

crates/apollo-mcp-server/src/runtime/config.rs

+use super::{
+    OperationSource, SchemaSource, graphos::GraphOSConfig, introspection::Introspection,
+    overrides::Overrides,
+};
+use crate::runtime::logging::Logging;


Nit: These can be merged by just using super::logging:Logging

Also, any reason why the super imports were moved here?

Hmm I was altering things but I don't remember moving any imports myself. Maybe it was RustRover or clippy after I made some change (which was removed later).

nicholascioli · 2025-07-22T20:36:41Z

crates/apollo-mcp-server/src/runtime/logging.rs

+//!
+//! This module is only used by the main binary and provides logging config structures and setup
+//! helper functions
+pub(crate) mod init;


Nit: This can be private.

crates/apollo-mcp-server/src/runtime/logging.rs

nicholascioli · 2025-07-22T20:42:02Z

crates/apollo-mcp-server/src/runtime/logging/init.rs

+pub fn setup_logging(config: &Config) -> Result<Option<WorkerGuard>, anyhow::Error> {
+    let mut env_filter = EnvFilter::from_default_env().add_directive(config.logging.level.into());
+
+    if config.logging.level == Level::INFO {
+        env_filter = env_filter
+            .add_directive("rmcp=warn".parse()?)
+            .add_directive("tantivy=warn".parse()?);
+    }
+
+    if let Some(path) = &config.logging.path {
+        setup_file_logging(path, env_filter, config.logging.rotation.clone())
+    } else {
+        setup_stderr_logging(env_filter)
+    }
+}


This method should probably be a member function on Logging. You can either have an impl Logging block in this file or merge it with the parent module.

nicholascioli · 2025-07-22T20:52:42Z

crates/apollo-mcp-server/src/runtime/logging/init.rs

+fn ensure_log_dir_exists(dir: PathBuf) -> std::io::Result<()> {
+    std::fs::create_dir_all(dir)
+}


Nit: This can probably be deleted.

nicholascioli · 2025-07-22T21:03:57Z

crates/apollo-mcp-server/src/runtime/logging/init.rs

+    match ensure_log_dir_exists(log_path.clone()) {
+        Ok(..) => {}
+        Err(_err) => {
+            eprintln!("Could not build log path - falling back to stderr");
+            return setup_stderr_logging(env_filter);
+        }
+    }
+
+    let (non_blocking_writer, guard) = match RollingFileAppender::builder()
+        .rotation(log_rotation.into())
+        .filename_prefix("apollo_mcp_server")
+        .filename_suffix("log")
+        .build(log_path)
+    {
+        Ok(appender) => tracing_appender::non_blocking(appender),
+        Err(_error) => {
+            eprintln!("Log file setup failed - falling back to stderr");
+            return setup_stderr_logging(env_filter);
+        }
+    };
+
+    tracing_subscriber::registry()
+        .with(env_filter)
+        .with(
+            tracing_subscriber::fmt::layer()
+                .with_writer(non_blocking_writer)
+                .with_ansi(false)
+                .with_target(false),
+        )
+        .init();
+
+    Ok(Some(guard))


Nit: This is mostly to show that this could be simplified slightly by functional chaining, but feel free to ignore.

Suggested change

match ensure_log_dir_exists(log_path.clone()) {

Ok(..) => {}

Err(_err) => {

eprintln!("Could not build log path - falling back to stderr");

return setup_stderr_logging(env_filter);

}

}

let (non_blocking_writer, guard) = match RollingFileAppender::builder()

.rotation(log_rotation.into())

.filename_prefix("apollo_mcp_server")

.filename_suffix("log")

.build(log_path)

{

Ok(appender) => tracing_appender::non_blocking(appender),

Err(_error) => {

eprintln!("Log file setup failed - falling back to stderr");

return setup_stderr_logging(env_filter);

}

};

tracing_subscriber::registry()

.with(env_filter)

.with(

tracing_subscriber::fmt::layer()

.with_writer(non_blocking_writer)

.with_ansi(false)

.with_target(false),

)

.init();

Ok(Some(guard))

let (writer, guard) = std::fs::create_dir_all(log_path)

.and_then(RollingFileAppender::builder()

.rotation(log_rotation.into())

.filename_prefix("apollo_mcp_server")

.filename_suffix("log")

.build())

.and_then(|(non_blocking, guard)| (non_blocking, Some(guard)))

.unwrap_or_else(|| {

eprintln!("Log file setup failed - falling back to stderr");

(std::io::stderr, None)

})

tracing_subscriber::registry()

.with(env_filter)

.with(

tracing_subscriber::fmt::layer()

.with_writer(writer)

.with_ansi(true)

.with_target(false),

)

.init();

Ok(guard)

nicholascioli · 2025-07-22T21:04:18Z

crates/apollo-mcp-server/src/runtime/logging/init.rs

+}
+
+/// Sets up stderr logging
+fn setup_stderr_logging(env_filter: EnvFilter) -> Result<Option<WorkerGuard>, anyhow::Error> {


Nit: this is probably not needed.

nicholascioli · 2025-07-22T21:11:51Z

crates/apollo-mcp-server/src/runtime/logging/log_rotation_kind.rs

+    #[rstest]
+    #[case(LogRotationKind::Minutely, Rotation::MINUTELY)]
+    #[case(LogRotationKind::Hourly, Rotation::HOURLY)]
+    #[case(LogRotationKind::Daily, Rotation::DAILY)]
+    #[case(LogRotationKind::Never, Rotation::NEVER)]
+    fn it_maps_to_rotation_correctly(
+        #[case] log_rotation_kind: LogRotationKind,
+        #[case] expected: Rotation,
+    ) {
+        let actual: Rotation = log_rotation_kind.into();
+        assert_eq!(expected, actual);
+    }


Nit: This test seems weird to me since the From impl is just a straight mapping, but feel free to leave it.

I added it mainly for coverage and to break in case someone changes something with the mapping. The test will then at least alert you that you broke some expected mapping between the two.

feat: adding a log file support when configured

e82cce9

alocay requested a review from a team as a code owner July 16, 2025 13:19

alocay changed the title ~~feat: adding a log file support when configured~~ feat: adding a log file support Jul 16, 2025

alocay added 4 commits July 16, 2025 09:22

chore: clippy fixes

e1a96f1

chore: removing unused capability

62980e5

chore: adding comments

54b6b9d

chore: adjusting error messages

c9aaa1a

pubmodmatt reviewed Jul 16, 2025

View reviewed changes

crates/apollo-mcp-server/src/runtime.rs Outdated Show resolved Hide resolved

pubmodmatt reviewed Jul 16, 2025

View reviewed changes

crates/apollo-mcp-server/src/runtime.rs Outdated Show resolved Hide resolved

chore: adding rotation config opt and moving logging functionality it…

d6c202c

…s own module

alocay requested review from pubmodmatt and nicholascioli July 18, 2025 14:15

pubmodmatt reviewed Jul 18, 2025

View reviewed changes

crates/apollo-mcp-server/src/log/logging.rs Outdated Show resolved Hide resolved

crates/apollo-mcp-server/src/log.rs Outdated Show resolved Hide resolved

chore: moving logging to src and combining with logging setup

3badd22

alocay added 4 commits July 22, 2025 10:01

chore: adding comment around worker guard

8fe2e2c

chore: adding internal file comment for logging.rs

b34ae6d

chore: setting visibility to pub crate'

fd439e0

chore: moving non-config logging code to dedicated runtime/logging su…

4dbe1b4

…bmodules

nicholascioli reviewed Jul 22, 2025

View reviewed changes

		@@ -1,19 +1,18 @@
		use std::path::PathBuf;

		use crate::runtime::logging::init::setup_logging;

feat: adding a log file support #200

Are you sure you want to change the base?

feat: adding a log file support #200

Uh oh!

Conversation

alocay commented Jul 16, 2025

Uh oh!

apollo-librarian bot commented Jul 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Docs preview ready

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

alocay commented Jul 22, 2025

Uh oh!

nicholascioli left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

apollo-librarian bot commented Jul 16, 2025 •

edited

Loading