Skip to content

Conversation

@tac0turtle
Copy link
Contributor

@tac0turtle tac0turtle commented Nov 20, 2025

Overview

this pr proposes reducing log noise when syncing from da. In our case we recommend p2p is set so now the node can be far ahead of da sync height and this causes log pollution

PM INF syncing block component=syncer height=12380514
2025-11-20T12:12:46.853Z	DEBUG	rpc	[email protected]/client.go:686	rpc result	{"type": "*da.GetIDsResult"}
12:12PM INF optimistically marked header as DA included component=da_retriever da_height=8854157 header_hash=0D2D3E53BE88E96F41578E446A1D3C4A693AEF10BC19121FF4453AE1903BDD03 height=11911920
12:12PM INF optimistically marked header as DA included component=da_retriever da_height=8854157 header_hash=493FFE6C3A5B3CADC81EEAE5D2B3576AA0B2A9A2716559AC748933558A673CE5 height=11911921
12:12PM INF optimistically marked header as DA included component=da_retriever da_height=8854157 header_hash=1BA321C9080D7128EAAEE82767AF71B1A83CEAD258A74C7FC413BCD80FAC3618 height=11911922
12:12PM INF optimistically marked header as DA included component=da_retriever da_height=8854157 header_hash=FFF1F67F1E4E436B60351284418E693044D142883BA2E224EE6A9669C06073B6 height=11911923
12:12PM INF optimistically marked header as DA included component=da_retriever da_height=8854157 header_hash=BE1975489E9AD910E1BF7E182861BF129AD8B7DFF78E9C776461D7883711A5BA height=11911924
12:12PM INF optimistically marked header as DA included component=da_retriever da_height=8854157 header_hash=7BEEA104648CFB68A3CF601788B164AC1597B12D0E39694B7A4E132296E87BC5 height=11911925
12:12PM INF optimistically marked header as DA included component=da_retriever da_height=8854157 header_hash=19CAF2D4E3B2F74B1C02815BBECED277DDD70584F093EA15B196D2D9C60C1A00 height=11911926
12:12PM INF optimistically marked header as DA included component=da_retriever da_height=8854157 header_hash=319F177811412603CDD0212C7DFE5B8592DC1B0E44CC58B904D10F95FCEB1E2F height=11911927
12:12PM INF optimistically marked header as DA included component=da_retriever da_height=8854157 header_hash=6300007D19E5F66CA4885E5EC1E6CDB0D6B5940AFED36C66EF622DDC3F69FB65 height=11911928
12:12PM INF optimistically marked header as DA included component=da_retriever da_height=8854157 header_hash=AB2FA1541398A7CEEADDA33E0CAB2187F46AF1726E10570E81C64075F9EBD7A6 height=11911929
12:12PM INF optimistically marked header as DA included component=da_retriever da_height=8854157 header_hash=49DAF21622E10CF12A0471BA6FECEA0E3CBF798E4EF817A6C309B78E09E27948 height=11911930
12:12PM INF optimistically marked header as DA included component=da_retriever da_height=8854157 header_hash=34750BF409D2D59C39212184436FE0E389E8EB4F168E08BD20665BCCDA3D7402 height=11911931
12:12PM INF optimistically marked header as DA included component=da_retriever da_height=8854157 header_hash=D347C371B6FC55213F2A5A84A0E532433752BCED81F40CF5CB51A6982678F60A height=11911932
12:12PM INF optimistically marked header as DA included component=da_retriever da_height=8854157 header_hash=90139B80A6E653A218A763237951FC74EE5D5DB85E1970A1EEE6BBD24E1A5030 height=11911933
12:12PM INF optimistically marked header as DA included component=da_retriever da_height=8854157 header_hash=7398F875392E9619C7C0232EF3445A7EDAF774A0C180453ECCC2AAA241357F67 height=11911934
12:12PM INF optimistically marked header as DA included component=da_retriever da_height=8854157 header_hash=152B48472B3F8C51EFE07C977469F668EBA2B294DC45A01F47B5A824CD85DF91 height=11911935
12:12PM INF optimistically marked header as DA included component=da_retriever da_height=8854157 header_hash=86E1E3FB3D33DF40B2A86750DE91348705150CD377B97462C0A52502BCB0BDC1 height=11911936
12:12PM INF optimistically marked header as DA included component=da_retriever da_height=8854157 header_hash=F8492A623800AF8C83E925387F98B0D53E89215F3DD4744EE81E69C5A7741E96 height=11911937
12:12PM INF optimistically marked header as DA included component=da_retriever da_height=8854157 header_hash=A3BEF0997B705B643237C196882E55E024ABD335DD5A42D37F57D2286FF1797B height=11911938
12:12PM INF optimistically marked header as DA included component=da_retriever da_height=8854157 header_hash=D068BD46F59895126C2E68DD1459335E18146702285D391A2670A9D9C37386EA height=11911939
12:12PM INF optimistically marked header as DA included component=da_retriever da_height=8854157 header_hash=AA54758DE29046F2C1C06688EBEFED24FFAEFD9ED3F221252F91DEED2EB363AD height=11911940
12:12PM INF optimistically marked header as DA included component=da_retriever da_height=8854157 header_hash=AD62B5EA1F1A845F3E12071BE8214F38DDFCFFCEE4900F010A4873F32B1F8055 height=11911941
12:12PM INF optimistically marked header as DA included component=da_retriever da_height=8854157 header_hash=712A42AD336CA101FBF58946469AD42F58383174451F1BC52FB2CCF7EF095A9D height=11911942
12:12PM INF optimistically marked header as DA included component=da_retriever da_height=8854157 header_hash=D58E3B656F9D874EF5E4F04F262EFB946D095B1EACB43F0176EC146972C133F1 height=11911943
12:12PM INF processed block from DA component=da_retriever da_height=8854157 height=11911927
12:12PM INF processed block from DA component=da_retriever da_height=8854157 height=11911941
12:12PM INF processed block from DA component=da_retriever da_height=8854157 height=11911942
12:12PM INF processed block from DA component=da_retriever da_height=8854157 height=11911925
12:12PM INF processed block from DA component=da_retriever da_height=8854157 height=11911933
12:12PM INF processed block from DA component=da_retriever da_height=8854157 height=11911940
12:12PM INF processed block from DA component=da_retriever da_height=8854157 height=11911923
12:12PM INF processed block from DA component=da_retriever da_height=8854157 height=11911930
12:12PM INF processed block from DA component=da_retriever da_height=8854157 height=11911932
12:12PM INF processed block from DA component=da_retriever da_height=8854157 height=11911935
12:12PM INF processed block from DA component=da_retriever da_height=8854157 height=11911937
12:12PM INF processed block from DA component=da_retriever da_height=8854157 height=11911939
12:12PM INF processed block from DA component=da_retriever da_height=8854157 height=11911920
12:12PM INF processed block from DA component=da_retriever da_height=8854157 height=11911924
12:12PM INF processed block from DA component=da_retriever da_height=8854157 height=11911938
12:12PM INF processed block from DA component=da_retriever da_height=8854157 height=11911922
12:12PM INF processed block from DA component=da_retriever da_height=8854157 height=11911926
12:12PM INF processed block from DA component=da_retriever da_height=8854157 height=11911929
12:12PM INF processed block from DA component=da_retriever da_height=8854157 height=11911934
12:12PM INF processed block from DA component=da_retriever da_height=8854157 height=11911936
12:12PM INF processed block from DA component=da_retriever da_height=8854157 height=11911943
12:12PM INF processed block from DA component=da_retriever da_height=8854157 height=11911921
12:12PM INF processed block from DA component=da_retriever da_height=8854157 height=11911928
12:12PM INF processed block from DA component=da_retriever da_height=8854157 height=11911931

here are the logs today

@github-actions
Copy link
Contributor

The latest Buf updates on your PR. Results from workflow CI / buf-check (pull_request).

BuildFormatLintBreakingUpdated (UTC)
✅ passed⏩ skipped✅ passed✅ passedNov 20, 2025, 12:18 PM

@gemini-code-assist
Copy link
Contributor

Summary of Changes

Hello @tac0turtle, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request aims to significantly reduce log noise, particularly in scenarios where a node is syncing from the Data Availability layer and might be far ahead of the DA sync height. By consolidating block processing logs and lowering the verbosity of header and data decoding messages, the changes provide a cleaner and more focused logging output, enhancing the clarity of operational logs without sacrificing essential information for debugging.

Highlights

  • Reduced Log Verbosity for Block Processing: The processBlobs function now logs a single summary message indicating the count of processed blocks from the Data Availability (DA) layer, instead of logging each block individually. This log message is also conditional, only appearing if blocks were actually processed.
  • Lowered Log Level for Header and Data Decoding: Log messages generated upon successful decoding of headers (tryDecodeHeader) and data (tryDecodeData) from the DA layer have been demoted from Info level to Debug level, reducing noise during normal operation.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

@claude
Copy link
Contributor

claude bot commented Nov 20, 2025

Claude finished @tac0turtle's task —— View job


Code Review Complete ✅

I've reviewed the logging changes in this PR. Here's my comprehensive feedback:

Summary of Changes

This PR makes three targeted changes to reduce log noise in block/internal/syncing/da_retriever.go:

  1. Consolidated block processing logs (processBlobs:216-218) - Changed from per-block logging to a single summary log
  2. Demoted header decoding log (tryDecodeHeader:254) - Changed from Info to Debug level
  3. Demoted data decoding log (tryDecodeData:285) - Changed from Info to Debug level

Review Feedback

✅ Strengths

  1. Well-targeted changes - The modifications directly address the stated problem of log pollution when the node is ahead of DA sync height
  2. Improved signal-to-noise ratio - Aggregating per-block logs into a summary is a good pattern for high-frequency operations
  3. Appropriate log levels - Moving optimistic DA inclusion markers to Debug is correct; these are implementation details useful for troubleshooting but not operational events
  4. Maintains observability - The summary log still provides the essential information (count + DA height)

🔍 Observations & Suggestions

1. Log Consistency Pattern (processBlobs:216-218)

if len(events) > 0 {
    r.logger.Info().Int("count", len(events)).Uint64("da_height", daHeight).Msg("processed blocks from DA")
}

Good: Conditional logging prevents "processed 0 blocks" noise
💡 Consider: For very high event counts (e.g., bulk sync), you might still get frequent logs. If this becomes an issue later, consider:

  • Rate-limiting the log (e.g., every 10th DA height)
  • Adding a threshold (only log if count > 10)
  • This is not needed now, but worth keeping in mind

2. Debug-level Messages Still Valuable (tryDecodeHeader:254-258 and tryDecodeData:285-289)
The messages "optimistically marked header as DA included" and "data marked as DA included" are now at Debug level, which is appropriate. However, verify that:

  • These are still accessible when troubleshooting DA sync issues (ensure Debug logs are available in production when needed)
  • The cache operations (SetHeaderDAIncluded, SetDataDAIncluded) don't have their own logging that might create redundancy

3. Semantic Improvement in Message
The message changed from singular "processed block from DA" to plural "processed blocks from DA" - good attention to detail! 👍

4. No Breaking Changes
The changes are internal logging only - no API changes, no behavioral changes, no test updates needed. Appropriate for a chore: commit.


Code Quality Assessment

Focus Area Rating Notes
Code Quality & Architecture ✅ Excellent Clean, focused changes; maintains existing patterns
Error Handling & Resilience ✅ N/A No error handling changes; existing patterns are sound
Security ✅ N/A No security implications
Performance ✅ Improved Reduces logging overhead in hot path
Testing ✅ Acceptable No tests needed for log-level changes
Documentation ✅ Acceptable PR description clearly explains motivation

Verification Questions

To ensure completeness, consider these scenarios:

  1. DA sync lag scenario: When p2p sync is far ahead and DA retrieval returns many empty results, will the reduced logging provide enough visibility into sync health?

    • Answer: Yes, because the retrieved blob data log at line 75 remains, and the summary log shows actual processed blocks
  2. Debugging DA issues: If there's a DA submission/retrieval problem, are Debug logs easily accessible?

    • Recommendation: Ensure production logging configuration allows dynamic log level changes (e.g., via signals or config reload)
  3. Metrics/observability: Are these events tracked in metrics separate from logs?

    • Context: If critical for monitoring, ensure prometheus/metrics track DA sync progress independently of logs

Final Recommendation

Approve ✅ - This PR accomplishes its goal cleanly and correctly. The changes are:

  • Minimal and focused
  • Appropriate for the problem described
  • Consistent with Go logging best practices
  • Low-risk (internal logging only)

The log noise reduction will improve operational clarity, especially in the described scenario where p2p sync is ahead of DA sync.


Todo List

  • Read and analyze the changed file
  • Review code quality and architecture
  • Check error handling and logging patterns
  • Verify the changes achieve the stated goal
  • Provide detailed feedback

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request effectively reduces log noise during DA sync. The changes involve lowering the severity of frequent log messages from Info to Debug and consolidating individual block processing logs into a single summary message per DA height. These modifications are well-aligned with the goal of reducing log pollution and have been implemented correctly. The overall code quality is improved by making the logs less verbose during normal operation while retaining details at the debug level.

@codecov
Copy link

codecov bot commented Nov 20, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 64.80%. Comparing base (9d4c64c) to head (a352859).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2864      +/-   ##
==========================================
- Coverage   64.89%   64.80%   -0.10%     
==========================================
  Files          81       81              
  Lines        7242     7243       +1     
==========================================
- Hits         4700     4694       -6     
- Misses       1998     2005       +7     
  Partials      544      544              
Flag Coverage Δ
combined 64.80% <100.00%> (-0.10%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

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

Copy link
Member

@julienrbrt julienrbrt left a comment

Choose a reason for hiding this comment

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

ACK

@tac0turtle tac0turtle merged commit e5aa2c3 into main Nov 20, 2025
33 checks passed
@tac0turtle tac0turtle deleted the marko/logs branch November 20, 2025 13:04
alpe added a commit that referenced this pull request Nov 20, 2025
* main:
  chore: reduce log noise (#2864)
  fix: sync service for non zero height starts with empty store (#2834)
  build(deps): Bump golang.org/x/crypto from 0.43.0 to 0.45.0 in /execution/evm (#2861)
  chore: minor improvement for docs (#2862)
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.

3 participants