Skip to content

fix(journal-log-writer): accept only absolute journal directory paths.#22771

Merged
vkalintiris merged 1 commit into
netdata:masterfrom
vkalintiris:fix/otel-journal-relative-path
Jun 18, 2026
Merged

fix(journal-log-writer): accept only absolute journal directory paths.#22771
vkalintiris merged 1 commit into
netdata:masterfrom
vkalintiris:fix/otel-journal-relative-path

Conversation

@vkalintiris

@vkalintiris vkalintiris commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

SSIA.


Summary by cubic

Reject non-absolute journal directories at Log::new in journal-log-writer to fail fast with a clear error and avoid dropped logs on first write.

  • Bug Fixes

    • Validate that journal_dir is absolute before any I/O; return WriterError::InvalidPath naming the value.
    • Clarify file-creation failure message when a generated path can’t be parsed as a journal file.
    • Add tests for rejecting relative paths, accepting absolute paths, and creating missing absolute directories.
    • Add a directory-contract spec and note the absolute-path requirement in the OTEL plugin config; relative-path resolution remains a consumer responsibility.
  • Migration

    • Ensure logs.journal_dir is an absolute path (including NETDATA_OTEL_LOGS_JOURNAL_DIR overrides); relative values now fail at startup.

Written for commit 48900ed. Summary will update on new commits.

Review in cubic

…rtup

A relative journal_dir was accepted through Log::new init (the discarded
canonicalize result left the path relative), then failed at first write with a
cause-less "failed to create journal file in <path>" because
repository::File::from_path only accepts absolute paths. To OTLP clients this
surfaced as a permanent "Failed to write log entry" gRPC Internal error and
dropped log batches.

Validate that the journal directory is absolute as the first step of
create_chain, before any I/O, returning an InvalidPath error that names the
offending value, and enrich the residual create-file error to describe the real
remaining failure mode (unparseable generated file path).

Relative-path resolution stays the consumer's responsibility: netflow resolves
its relative default against the netdata cache dir when one is available, and
the new Log::new check is the safety net for the standalone case.

Add tests and a directory-contract spec; note the requirement in the stock otel
config.
@vkalintiris
vkalintiris requested a review from a team June 18, 2026 16:15
@vkalintiris vkalintiris self-assigned this Jun 18, 2026
@vkalintiris vkalintiris added the opentelemetry Anything related to OpenTelemetry label Jun 18, 2026
@vkalintiris
vkalintiris requested a review from a team as a code owner June 18, 2026 16:15

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 6 files

Confidence score: 5/5

  • Automated review surfaced no issues in the provided summaries.
  • No files require special attention.
Architecture diagram
sequenceDiagram
    participant Config as OTEL Plugin Config
    participant Env as Environment
    participant Plugin as OTEL Plugin
    participant Log as Log::new()
    participant FS as Filesystem
    participant Chain as OwnedChain
    participant Repo as repository::File::from_path()

    Note over Config,Repo: Journal Directory Validation Flow

    Config->>Plugin: journal_dir (may be absolute from @logdir_POST@ or relative override)
    Env->>Plugin: NETDATA_OTEL_LOGS_JOURNAL_DIR override (may be relative)

    Plugin->>Log: new(journal_dir, config)

    alt journal_dir is absolute
        Log->>FS: canonicalize() to verify accessibility
        FS-->>Log: canonical path
        Note over Log: Proceed with journal creation
        Log->>FS: load_machine_id()
        FS-->>Log: machine ID
        Log->>Chain: create_chain(path)
        Chain->>FS: create directory if not exists
        FS-->>Chain: directory created
        Chain->>Repo: from_path() on generated file path
        note over Chain,Repo: CHANGED: clearer error message on parse failure
        alt file path parseable
            Repo-->>Chain: valid File
            Chain-->>Log: OwnedChain
            Log-->>Plugin: Ok(Log)
        else file path not parseable
            Repo-->>Chain: error
            Chain-->>Log: WriterError::FileCreation with detailed message
            Log-->>Plugin: Error
        end
    else journal_dir is relative
        Log-->>Plugin: CHANGED: WriterError::InvalidPath with offending value
        Note over Log,Plugin: Fail-fast before any I/O - no directory created
    end

    Note over Config,Plugin: Consumer Responsibility
    alt NetFlow Plugin path
        Note over Config,Plugin: NetFlow resolves relative path vs NETDATA_CACHE_DIR before calling Log::new
    end
Loading

Re-trigger cubic

@vkalintiris
vkalintiris enabled auto-merge (squash) June 18, 2026 16:24
@vkalintiris
vkalintiris merged commit d47056a into netdata:master Jun 18, 2026
154 of 173 checks passed
@sonarqubecloud

Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/docs opentelemetry Anything related to OpenTelemetry

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants