Instrument resolve_config_path, env_config_path, collect_diag_file_layers, and error paths with structured tracing spans and events

[coderabbitai]

Observability

Context

PR #285 (#285) adds explicit config path resolution via resolve_config_path, env_config_path, collect_diag_file_layers, and push_file_layers in src/cli/config_merge.rs. None of these operational pathways currently emit structured log or trace events. When a user provides an explicit --config path that fails to load, or when the precedence logic selects an unexpected source, there is no diagnostic signal in the logs to aid debugging.

Required work

Instrument the following functions using the existing tracing dependency:

1. resolve_config_path – emit a debug event recording which selector was chosen (CLI flag, NETSUKE_CONFIG, NETSUKE_CONFIG_PATH, or None) and the resolved path.
2. env_config_path – emit a trace event when a variable is read and whether it produced a path.
3. collect_diag_file_layers – emit a debug span covering the layer collection; include an event when the explicit path is used and when discovery is performed instead.
4. Error paths – emit a warn event whenever an explicit config load fails, including the path and the error.

Acceptance criteria

• All four functions have at least one tracing call on each significant branch.
• No new dependencies are required beyond the existing tracing crate.
• Structured fields (not format strings) are used for path values and error descriptions where possible.

Raised by

Requested by @leynos as a follow-up to #285.

[leynos]

@coderabbitai looking at the codebase, is this issue still relevant? If the issue has been fixed in the code, does the documentation reflect this?

If it is still relevant, please provide an assessment of the issue criticality (critical, high, medium or low), based on risk of the issue impacting the user and the anticipated blast radius.\n\nOnce a decision has been made, apply the appropriate label.

If the issue is documentation ONLY (i.e., no code changes needed), apply the documentation label in addition to the appropriate criticality label.

Please also provide an impact of the required changes.

[coderabbitai]

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Fetch issue details
gh issue view 292 --repo leynos/netsuke --json title,body,labels,state,comments

echo "---LABELS---"
gh label list --repo leynos/netsuke --json name,description

Length of output: 4012

---

🏁 Script executed:

#!/bin/bash
# Read the relevant source file
fd config_merge.rs --exec cat {}

echo "---TRACING USAGE IN FILE---"
fd config_merge.rs --exec rg -n 'tracing|debug!|trace!|warn!|info!|span!' {}

Length of output: 12894

---

🏁 Script executed:

#!/bin/bash
# Apply medium and enhancement labels to issue `#292`
gh issue edit 292 --repo leynos/netsuke --add-label "medium,enhancement"

Length of output: 226

---

---

@leynos — the issue is still relevant. A full scan of src/cli/config_merge.rs found zero tracing calls. None of resolve_config_path, env_config_path, collect_diag_file_layers, or the error paths inside push_file_layers/load_layers_from_path have been instrumented.

---

Criticality: medium

Dimension	Assessment
User impact	When an explicit --config path fails to load, or when the precedence logic selects an unexpected source (CLI flag vs NETSUKE_CONFIG vs NETSUKE_CONFIG_PATH), there is no structured log output. Users are left to guess why their configuration is not being honoured.
Blast radius	Affects anyone using --config, NETSUKE_CONFIG, or NETSUKE_CONFIG_PATH. Config path issues are common (typos, wrong env var casing, environment not exported). Silent failures are frustrating but the application itself does not behave incorrectly — it falls back or errors out.
Risk	No data loss, no security exposure, no incorrect computation. Pure observability gap.

medium applied ("Could be disruptive, but might not happen"). enhancement also applied as no existing behaviour is broken.

---

Required changes

All changes are confined to src/cli/config_merge.rs. No new dependencies are needed — tracing is already present.

1. env_config_path — add a trace! event recording the variable name and whether a path was produced.
2. resolve_config_path — add a debug! event stating which selector won (CLI flag, NETSUKE_CONFIG, NETSUKE_CONFIG_PATH, or None) and the resolved path.
3. collect_diag_file_layers — wrap in a debug_span!; emit an event on the explicit-path branch and on the discovery branch.
4. Error paths in load_layers_from_path — emit a warn! event on Err, including the path and the error, so explicit config load failures surface in logs.

[leynos]

@CodeRabbit plan

[coderabbitai]

Coding Plan

Summary

• Introduce the first debug_span! in the codebase to wrap collect_diag_file_layers, satisfying the ticket's span requirement.
• Refactor resolve_config_path from an or_else chain to an if/else cascade, enabling a single debug! event that names the winning selector.
• Add trace! breadcrumbs in env_config_path for low-level variable inspection.
• Guard explicit-path error returns in both collect_diag_file_layers and push_file_layers with warn! events, ensuring failures surface in logs before aggregation.

Design Choices

Design Choice 1: Span versus event-only instrumentation for `collect_diag_file_layers`

Options Considered:

1. Use an explicit debug_span!(...).entered() guard at function entry, matching the ticket wording and enabling future correlation of nested events.
2. Emit only standalone debug! events to stay consistent with existing event-only patterns.

Chosen Option: 1

Rationale: The ticket is prescriptive; introducing a span here establishes a clear pattern for future observability work and satisfies the acceptance criteria literally.

Design Choice 2: Refactoring `resolve_config_path` to capture the selector source

Options Considered:

1. Replace the or_else chain with explicit if let / else if let branches that record the selector name before returning.
2. Duplicate calls inside each or_else closure to emit separate events, preserving the chain but repeating log logic.

Chosen Option: 1

Rationale: An if/else cascade is clearer, logs exactly once, and keeps the selector name adjacent to the path assignment.

💡 User Tips

Regenerate the plan with different choices with @coderabbitai <feedback>.

Implementation Steps

Instrument config resolution functions with tracing

Add the tracing import to config_merge.rs and instrument each target function in dependency order, finishing with the error-path warnings.

Task 1: Add tracing imports

Import the required macros at the top of src/cli/config_merge.rs.

• Add use tracing::{debug, debug_span, trace, warn}; alongside the existing imports in src/cli/config_merge.rs.

Task 2: Instrument `env_config_path`

Emit a trace event each time the function is invoked, recording the variable name and whether a path was produced.

• At the end of env_config_path, before returning, emit a trace! event with structured fields var_name and found (boolean indicating Some vs None).
• Optionally include the path value using ?path when present; omit or redact if privacy is a concern.

Task 3: Instrument `resolve_config_path`

Emit a debug event identifying which selector was chosen and the resolved path.

• Replace the or_else chain with an if let / else if let cascade that assigns a selector label ("cli_flag", "NETSUKE_CONFIG", "NETSUKE_CONFIG_PATH", or "none") alongside the Option<PathBuf>.
• Before returning, emit debug!(selector, path = ?resolved_path, "resolved config path");.
• Preserve the existing #[expect(clippy::option_as_ref_cloned)] attribute; logic remains equivalent.

Task 4: Instrument `collect_diag_file_layers`

Wrap the function body in a debug span and emit events distinguishing the explicit-path branch from the discovery branch.

• At function entry, create let _span = debug_span!("collect_diag_file_layers").entered();.
• Inside the explicit-path branch (if let Some(path) = resolve_config_path(...)), emit debug!(path = ?path, "using explicit config path");.
• Inside the discovery branch, emit debug!("using config discovery"); after calling config_discovery.
• When the project-scope file is appended via project_scope_layers, emit debug!("appending project-scope layers");.

Task 5: Instrument error paths with `warn` events

Emit a warn event whenever an explicit config load fails, capturing the path and the error.

• In collect_diag_file_layers, before returning the Err from load_layers_from_path, emit warn!(path = ?path, error = %err, "explicit config load failed");.
  • Restructure the explicit-path branch to bind the result, inspect for Err, warn, then return.
• In push_file_layers, apply the same pattern: capture the result of load_layers_from_path, warn on Err, then pass the result to push_layers_result.
• Use %err (Display) for user-facing messages; ?err (Debug) is acceptable if more detail is preferred.

🤖 Prompt for AI agents

Add structured tracing instrumentation to `src/cli/config_merge.rs`, working
through the functions in dependency order as described below.

**Task 1: Add tracing imports**
- Add `use tracing::{debug, debug_span, trace, warn};` alongside the existing
imports in `src/cli/config_merge.rs`.

**Task 2: Instrument `env_config_path`**
- At the end of `env_config_path`, before the return statement, emit a `trace!`
event with structured fields `var_name` and `found` (a boolean indicating `Some`
vs `None`).
- Optionally include the path value using `?path` when present; omit or redact
if privacy is a concern.

**Task 3: Instrument `resolve_config_path`**
- Replace the existing `or_else` chain with an `if let` / `else if let` cascade.
- Assign a `selector` label at each branch: `"cli_flag"`, `"NETSUKE_CONFIG"`,
`"NETSUKE_CONFIG_PATH"`, or `"none"`.
- Before returning, emit `debug!(selector, path = ?resolved_path, "resolved
config path");`.
- Preserve the existing `#[expect(clippy::option_as_ref_cloned)]` attribute;
keep the logic equivalent.

**Task 4: Instrument `collect_diag_file_layers`**
- At function entry, add `let _span =
debug_span!("collect_diag_file_layers").entered();`.
- Inside the explicit-path branch (`if let Some(path) =
resolve_config_path(...)`), emit `debug!(path = ?path, "using explicit config
path");`.
- Inside the discovery branch, emit `debug!("using config discovery");` after
calling `config_discovery`.
- When appending via `project_scope_layers`, emit `debug!("appending
project-scope layers");`.

**Task 5: Instrument error paths with `warn` events**
- In `collect_diag_file_layers`, restructure the explicit-path branch to bind
the result of `load_layers_from_path`, inspect for `Err`, emit `warn!(path =
?path, error = %err, "explicit config load failed");`, then return the error.
- In `push_file_layers`, apply the same pattern: capture the result of
`load_layers_from_path`, warn on `Err`, then pass the result to
`push_layers_result`.
- Use `%err` (Display) for the error field; `?err` (Debug) is acceptable if more
detail is preferred.

Research

The project uses tracing v0.1 throughout src/ but has no existing tracing calls in src/cli/. Established conventions favour explicit macro calls (tracing::debug! or imported debug!) with structured key=value fields; the message string always appears last with a trailing comma. #[instrument] attributes and span-creation macros (debug_span!, info_span!) are not used anywhere—this change will introduce the first span in the codebase. Error logging follows the pattern ?err (Debug) or %err (Display) for the error field.

---

🚀 Next Steps

🤖 All AI agent prompts combined

Add structured tracing instrumentation to `src/cli/config_merge.rs`, working
through the functions in dependency order as described below.

**Task 1: Add tracing imports**
- Add `use tracing::{debug, debug_span, trace, warn};` alongside the existing
imports in `src/cli/config_merge.rs`.

**Task 2: Instrument `env_config_path`**
- At the end of `env_config_path`, before the return statement, emit a `trace!`
event with structured fields `var_name` and `found` (a boolean indicating `Some`
vs `None`).
- Optionally include the path value using `?path` when present; omit or redact
if privacy is a concern.

**Task 3: Instrument `resolve_config_path`**
- Replace the existing `or_else` chain with an `if let` / `else if let` cascade.
- Assign a `selector` label at each branch: `"cli_flag"`, `"NETSUKE_CONFIG"`,
`"NETSUKE_CONFIG_PATH"`, or `"none"`.
- Before returning, emit `debug!(selector, path = ?resolved_path, "resolved
config path");`.
- Preserve the existing `#[expect(clippy::option_as_ref_cloned)]` attribute;
keep the logic equivalent.

**Task 4: Instrument `collect_diag_file_layers`**
- At function entry, add `let _span =
debug_span!("collect_diag_file_layers").entered();`.
- Inside the explicit-path branch (`if let Some(path) =
resolve_config_path(...)`), emit `debug!(path = ?path, "using explicit config
path");`.
- Inside the discovery branch, emit `debug!("using config discovery");` after
calling `config_discovery`.
- When appending via `project_scope_layers`, emit `debug!("appending
project-scope layers");`.

**Task 5: Instrument error paths with `warn` events**
- In `collect_diag_file_layers`, restructure the explicit-path branch to bind
the result of `load_layers_from_path`, inspect for `Err`, emit `warn!(path =
?path, error = %err, "explicit config load failed");`, then return the error.
- In `push_file_layers`, apply the same pattern: capture the result of
`load_layers_from_path`, warn on `Err`, then pass the result to
`push_layers_result`.
- Use `%err` (Display) for the error field; `?err` (Debug) is acceptable if more
detail is preferred.

💡 Iterate on the plan with: `@coderabbitai` <feedback>

Example Feedback
- `@coderabbitai` You can skip phase 3. Add a simple unit test case for phase 2.
- `@coderabbitai` For design choice 1 go ahead with option 3 and replan.

---

💬 Have feedback or questions? Drop into our discord!