Skip to content

cmd, helper, packaging, x/bor: granular logs#619

Merged
marcello33 merged 2 commits into
developfrom
mardizzone/module-logs
Jul 4, 2026
Merged

cmd, helper, packaging, x/bor: granular logs#619
marcello33 merged 2 commits into
developfrom
mardizzone/module-logs

Conversation

@marcello33

@marcello33 marcello33 commented Jul 3, 2026

Copy link
Copy Markdown
Collaborator

Summary

Adds per-module log-level filtering to Heimdall. log_level now accepts either a single level applied globally (info, debug, … — unchanged behavior) or a comma-separated list of module:level pairs with an optional *:level default, e.g. *:info,mempool:debug. This lets an operator scope debug to one subsystem on a busy node without the firehose a global debug produces.

The parsing lives in a new helper.LogLevelOption: it tries a plain zerolog.ParseLevel first and falls back to cosmos-sdk's logger.ParseLogLevel / FilterOption for the per-module form, wired into NewRootCmd. The log_level docs in the mainnet and amoy config.toml templates now describe both forms and list the common CometBFT and Heimdall module names.

Executed tests

  • helper/log_test.go — covers the plain-level path, the module:level filter path, the *:default form, and the empty/invalid cases.
  • Standard CI (lint + unit) is the gate; both new tests run there.

Rollout notes

  • Not consensus-affecting. Logging + config-doc only; no state, ABCI, or module-version change.
  • Backward-compatible. An existing log_level = "info" (or any plain level) parses exactly as before — the per-module form is purely additive. No coordinated upgrade needed.
  • Operator-facing. The only visible change is that log_level now accepts the module:level syntax; existing configs keep working untouched.

Copilot AI review requested due to automatic review settings July 3, 2026 12:31

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Claude Code Review

This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.

Tip: disable this comment in your organization's Code Review settings.

@marcello33

Copy link
Copy Markdown
Collaborator Author

@claude review once

Copilot AI 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.

Pull request overview

This PR extends Heimdall’s logging configuration to support per-module log-level filtering (in addition to the existing single global level) and adds a regression test to detect polyproto Header field-population mismatches across the Bor gRPC boundary.

Changes:

  • Add helper.LogLevelOption to parse either a plain log_level or a module:level filter spec (with optional *:default).
  • Wire the new log-level parsing into heimdalld root command logger setup and config initialization.
  • Update mainnet/amoy config.toml templates with syntax docs and common module names; add a Bor gRPC header encode/decode round-trip test.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
helper/log.go Introduces LogLevelOption to support both plain levels and per-module filter specs.
helper/log_test.go Adds unit + end-to-end tests validating global level vs per-module filtering behavior.
helper/config.go Uses LogLevelOption during config init and falls back to info on malformed specs.
cmd/heimdalld/cmd/root.go Wires per-module log-level parsing into runtime logger initialization.
packaging/templates/config/mainnet/config.toml Documents the new log_level syntax and common module names.
packaging/templates/config/amoy/config.toml Mirrors the mainnet template documentation updates.
x/bor/grpc/header_roundtrip_test.go Adds a polyproto Header marshal/unmarshal round-trip test to catch truncated-field population mismatches.

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

Comment thread helper/config.go Outdated
Comment thread x/bor/grpc/header_roundtrip_test.go Outdated
@codecov

codecov Bot commented Jul 3, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 89.47368% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 50.0%. Comparing base (c32976f) to head (3af0225).

Files with missing lines Patch % Lines
cmd/heimdalld/cmd/root.go 0.0% 2 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##           develop    #619   +/-   ##
=======================================
  Coverage     50.0%   50.0%           
=======================================
  Files          180     181    +1     
  Lines        19854   19864   +10     
=======================================
+ Hits          9934    9948   +14     
+ Misses        8646    8643    -3     
+ Partials      1274    1273    -1     
Files with missing lines Coverage Δ
helper/config.go 67.0% <100.0%> (+0.1%) ⬆️
helper/log.go 100.0% <100.0%> (ø)
cmd/heimdalld/cmd/root.go 0.0% <0.0%> (ø)

... and 2 files with indirect coverage changes

Files with missing lines Coverage Δ
helper/config.go 67.0% <100.0%> (+0.1%) ⬆️
helper/log.go 100.0% <100.0%> (ø)
cmd/heimdalld/cmd/root.go 0.0% <0.0%> (ø)

... and 2 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

LGTM — additive logging enhancement plus a polyproto Header round-trip test; no consensus or state impact.

Extended reasoning...

Overview

This PR is scoped to logging ergonomics and test coverage. helper.LogLevelOption accepts either a plain zerolog level (unchanged behavior) or a module:level list backed by cosmos-sdk's ParseLogLevel/FilterOption. It is wired into NewRootCmd and InitHeimdallConfigWith in place of the direct zerolog.ParseLevel call. The config.toml templates for mainnet and amoy get a documentation-only rewrite of the log_level comment. The new x/bor/grpc/header_roundtrip_test.go is a test-only addition that asserts the shipped polyproto Header round-trips to the canonical block hash.

Security risks

None material. Log parsing runs on operator-supplied config only, not external inputs, and a malformed spec falls back to InfoLevel in InitHeimdallConfigWith (matching the old behavior) or returns an error in NewRootCmd. No auth, crypto, or permission surface is touched.

Level of scrutiny

Low. This is operator-facing observability plumbing plus additive tests — none of the files matched by .claude/rules/consensus-critical.md or cross-chain.md are semantically changed, and the diff is not hardfork-shaped. Existing log_level = "info" configs parse exactly as before, so no coordinated upgrade is required.

Other factors

The new behavior is directly exercised by helper/log_test.go (plain, module-spec, wildcard-default, malformed cases plus an end-to-end FiltersRealLogger assertion). The polyproto test file is self-contained and non-functional. The reviewer explicitly requested a review pass. No bugs were flagged by the bug hunting system.

@marcello33 marcello33 changed the title cmd, helper, packaging, x/bor: granular logs, polyproto test cmd, helper, packaging, x/bor: granular logs Jul 3, 2026
@marcello33 marcello33 requested a review from a team July 3, 2026 13:00
@sonarqubecloud

sonarqubecloud Bot commented Jul 3, 2026

Copy link
Copy Markdown

@marcello33 marcello33 merged commit e91f63c into develop Jul 4, 2026
22 of 23 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants