feat(port): port from ent to core #26996

philjb · 2025-11-27T00:24:34Z

This ports a number of changes from enterprise to core. Primarily this adds a new telemetry field for how influxdb3 has been started (aka invocatio method). Most of the changes are to sync core code with the shape of ent code to make future ports easier. There are some error reporting improvements and logging improvements mixed in there too.

follows feat: port from enterprise to core #26991

some of the enterprise prs:

This ports a number of changes from enterprise to core. Primarily this adds a new telemetry field for how influxdb3 has been started (aka invocation method). Most of the changes are to sync core code with the shape of ent code to make future ports easier. There are some error reporting improvements and logging improvements mixed in there too. * follows #26991

Copilot

Pull request overview

This PR ports several changes from InfluxDB Enterprise to Core, primarily adding telemetry tracking for how InfluxDB3 is invoked (via explicit command, quickstart, Docker, etc.). The changes also include code structure synchronization between Enterprise and Core codebases, improved error messages, and better logging.

Adds ServeInvocationMethod enum to track how the serve command was invoked (explicit, quickstart, Docker, etc.)
Improves error messages and logging throughout the codebase
Refactors argument parsing and command handling to better support the default quickstart mode

Reviewed changes

Copilot reviewed 12 out of 13 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
influxdb3_telemetry/src/lib.rs	Adds `ServeInvocationMethod` enum with parsing, display, and serialization support
influxdb3_telemetry/src/store.rs	Integrates `ServeInvocationMethod` into telemetry store creation and payload
influxdb3_telemetry/src/sender.rs	Adds `serve_invocation_method` field to telemetry payload
influxdb3_telemetry/Cargo.toml	Alphabetizes dependencies
influxdb3/src/lib.rs	Refactors startup logic to separate serve and non-serve command handling, adds CLI examples
influxdb3/src/commands/serve.rs	Adds `serve_invocation_method` parameter, improves error messages, adds runtime configuration
influxdb3/src/commands/serve/cli_params.rs	Adds `serve-invocation-method` to non-sensitive parameters list
influxdb3_clap_blocks/src/object_store.rs	Improves error messages, adds conditional default for data directory, fixes logging level for credential errors
influxdb3_load_generator/src/main.rs	Changes `println!` to `eprintln!` for error output
influxdb3_clap_blocks/Cargo.toml	Adds `clap_builder` dependency and alphabetizes dependencies
influxdb3/Cargo.toml	Adds `clap_builder` dependency and alphabetizes dependencies
Cargo.toml	Adds `clap_builder` dependency and alphabetizes workspace dependencies
Cargo.lock	Updates lockfile with new dependencies

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.

Copilot · 2025-11-27T00:42:36Z

influxdb3/src/lib.rs

+    influxdb3 serve --object-store file --data-dir ~/.influxdb3 --node_id my_node_name
+
+    # Display all commands short form
+    influxdb3 -h
+
+    # Display all commands long form
+    influxdb3 --help
+
+    # Run the InfluxDB 3 Core server with extra verbose logging
+    influxdb3 serve -v --object-store file --data-dir ~/.influxdb3 --node_id my_node_name
+
+    # Run InfluxDB 3 Core with full debug logging specified with LOG_FILTER
+    LOG_FILTER=debug influxdb3 serve --object-store file --data-dir ~/.influxdb3 --node_id my_node_name


The flag name in the examples uses underscores (--node_id) but the actual flag uses hyphens (--node-id). This inconsistency could confuse users. Update the examples to use --node-id to match the actual CLI flag.

Addressed in 4a38489

Copilot · 2025-11-27T00:42:36Z

influxdb3/src/lib.rs

-                };
+        commands::serve::command(serve_config, user_params).await
+    }) {
+        eprintln!("Serve command failed: {e}\n");


The error message includes a trailing newline (\n) which is inconsistent with other error messages in the same file (e.g., line 266, 335-395) that don't include trailing newlines. Consider removing the \n for consistency.

Suggested change

eprintln!("Serve command failed: {e}\n");

eprintln!("Serve command failed: {e}");

Addressed in 4a38489

Copilot · 2025-11-27T00:42:37Z

influxdb3_clap_blocks/src/object_store.rs

 fn default_data_dir() -> OsString {
    home::home_dir()
-        .expect("No data-dir specified and could not find user's home directory")
+        .expect("Could not find user's home directory")
        .join(DEFAULT_DATA_DIRECTORY_NAME)
        .into_os_string()
 }


The default_data_dir() function panics if the home directory cannot be found. With the new conditional default (line 430-431), this function will only be called when --object-store=file, which is better than before. However, if someone explicitly sets --object-store=file but the home directory is unavailable (e.g., in certain container environments), the application will panic. Consider returning a Result or handling this more gracefully, especially since the error message doesn't mention that it's only needed when using file-based storage.

I think panicking is okay here. Not being able to find a home directory seems prohibitive in this case.

Copilot · 2025-11-27T00:42:37Z

influxdb3/src/commands/serve.rs

+                .map(|mut builder| {
+                    builder.enable_all();
+                    builder


The builder.enable_all() call is used for the first executor but not for the write_path_executor (line 882). This inconsistency could lead to different runtime behavior between the two executors. Consider applying enable_all() to both executors if that's the intended behavior, or document why they should differ.

FWIW, the docs on enable_all say:

/// Enables both I/O and time drivers. /// /// Doing this is a shorthand for calling `enable_io` and `enable_time` /// individually. If additional components are added to Tokio in the future, /// `enable_all` will include these future components. /// /// # Examples /// /// ``` /// # #[cfg(not(target_family = "wasm"))] /// # { /// use tokio::runtime; /// /// let rt = runtime::Builder::new_multi_thread() /// .enable_all() /// .build() /// .unwrap(); /// # } /// ``` pub fn enable_all(&mut self) -> &mut Self {

Here (where is using enable_all) is for initializing the executor created for the query path.

Where it does not use enable_all is for initializing the executor created for the write path.

I don't know why we would not enable IO on one but not the other.

The code is currently the same between Core and Enterprise, so I am hesitant to follow this suggestion on this PR, but it may warrant further investigation.

Copilot · 2025-11-27T00:42:37Z

influxdb3_telemetry/src/lib.rs

+            ServeInvocationMethod::InstallScript => f.write_str("install-script"),
+            ServeInvocationMethod::DockerHub => f.write_str("docker-hub"),
+            ServeInvocationMethod::DockerOther => f.write_str("docker-other"),
+            ServeInvocationMethod::Custom(i) => f.write_str(format!("{}", i).as_str()),


Using format!("{}", i).as_str() creates an intermediate String that is immediately converted to &str. This allocates memory unnecessarily. Consider using write!(f, "{}", i) instead, which writes directly to the formatter without the intermediate allocation.

Suggested change

ServeInvocationMethod::Custom(i) => f.write_str(format!("{}", i).as_str()),

ServeInvocationMethod::Custom(i) => write!(f, "{}", i),

Addressed in 4a38489

Copilot · 2025-11-27T00:42:37Z

influxdb3/src/commands/serve.rs

    InvalidToken(#[from] hex::FromHexError),

-    #[error("failed to initialized write buffer: {0}")]
+    #[error("failed to initialized write buffer: {0:?}")]


Typo in error message: "initialized" should be "initialize" to match tense with similar error messages (lines 121, 124, 127).

Suggested change

#[error("failed to initialized write buffer: {0:?}")]

#[error("failed to initialize write buffer: {0:?}")]

Addressed in 4a38489

hiltontj

I think most of Copilot's suggestions are good, but they can be addressed in follow-ons.

hiltontj · 2025-11-27T16:06:59Z

Ok, I ended up just applying the obvious suggestions from Copilot that I thought worthwhile.

philjb force-pushed the pjb/core-catchup-nov-26-25 branch from c32183a to 85ea29a Compare November 27, 2025 00:27

philjb force-pushed the pjb/core-catchup-nov-26-25 branch from 85ea29a to bf5e989 Compare November 27, 2025 00:33

philjb requested a review from hiltontj November 27, 2025 00:33

philjb marked this pull request as ready for review November 27, 2025 00:37

philjb requested a review from Copilot November 27, 2025 00:37

Copilot started reviewing on behalf of philjb November 27, 2025 00:38 View session

Copilot finished reviewing on behalf of philjb November 27, 2025 00:41

Copilot AI reviewed Nov 27, 2025

View reviewed changes

hiltontj approved these changes Nov 27, 2025

View reviewed changes

refactor: copilot pr feedback

4a38489

	eprintln!("Serve command failed: {e}\n");
	eprintln!("Serve command failed: {e}");

	ServeInvocationMethod::Custom(i) => f.write_str(format!("{}", i).as_str()),
	ServeInvocationMethod::Custom(i) => write!(f, "{}", i),

	#[error("failed to initialized write buffer: {0:?}")]
	#[error("failed to initialize write buffer: {0:?}")]

feat(port): port from ent to core #26996

Are you sure you want to change the base?

feat(port): port from ent to core #26996

Conversation

philjb commented Nov 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Nov 27, 2025

Choose a reason for hiding this comment

Uh oh!

hiltontj Nov 27, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 27, 2025

Choose a reason for hiding this comment

Uh oh!

hiltontj Nov 27, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 27, 2025

Choose a reason for hiding this comment

Uh oh!

hiltontj Nov 27, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 27, 2025

Choose a reason for hiding this comment

Uh oh!

hiltontj Nov 27, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 27, 2025

Choose a reason for hiding this comment

Uh oh!

hiltontj Nov 27, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 27, 2025

Choose a reason for hiding this comment

Uh oh!

hiltontj Nov 27, 2025

Choose a reason for hiding this comment

Uh oh!

hiltontj left a comment

Choose a reason for hiding this comment

Uh oh!

hiltontj commented Nov 27, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

philjb commented Nov 27, 2025 •

edited

Loading