Skip to content

object server: resolve log error TODOs for remote node operations - #4142

Merged
roman-khimov merged 3 commits into
masterfrom
copilot/add-logging-to-object-server
Aug 26, 2026
Merged

roman-khimov merged 3 commits into
masterfrom
copilot/add-logging-to-object-server

Conversation

Copilot AI commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

The object server gained internal logic in #3097 but had no logging, leaving four // TODO: log error stubs in remote-forwarding paths where failures were silently swallowed or only surfaced as returned errors.

Changes

  • putStream: Added log *zap.Logger field; plumbed from Server via newIntermediatePutStream. sendToRemoteNode now warns on per-connection failure inside ForAnyGRPCConn.
  • convertHeadPrm: Added log *zap.Logger parameter; transport func warns when getHeaderFromRemoteNode fails on a connection attempt.
  • convertGetPrm: Added log *zap.Logger parameter; transport func warns when continueWithConn fails on a connection attempt.
  • searchOnRemoteNode: Uses existing s.log to warn when searchOnRemoteAddress fails on a connection attempt.

All four sites log at Warn level — ForAnyGRPCConn may retry on another endpoint, so these are per-connection failures, not necessarily fatal outcomes.

Copilot AI linked an issue Aug 25, 2026 that may be closed by this pull request
Co-authored-by: roman-khimov <22092804+roman-khimov@users.noreply.github.com>
Copilot AI changed the title [WIP] Add logging to object server and resolve TODOs object server: resolve log error TODOs for remote node operations Aug 25, 2026
Copilot AI requested a review from roman-khimov August 25, 2026 16:39

@roman-khimov roman-khimov left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think all of these are Debug level. Failure to perform some action on remote node is normal in general for many reasons and we can't spam logs in this case.

Open to other suggestions.

@roman-khimov
roman-khimov marked this pull request as ready for review August 26, 2026 10:20
@codecov

codecov Bot commented Aug 26, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 11.11111% with 16 lines in your changes missing coverage. Please review.
✅ Project coverage is 30.10%. Comparing base (f8cb96f) to head (9292593).
⚠️ Report is 4 commits behind head on master.

Files with missing lines Patch % Lines
pkg/services/object/server.go 11.11% 16 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4142      +/-   ##
==========================================
+ Coverage   29.78%   30.10%   +0.31%     
==========================================
  Files         678      679       +1     
  Lines       40860    40897      +37     
==========================================
+ Hits        12171    12310     +139     
+ Misses      28689    28587     -102     

☔ View full report in Codecov by Harness.
📢 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.

@End-rey

End-rey commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

I tend to agree: Debug seems more appropriate here to avoid excessive warning logs from expected remote-node failures.

@roman-khimov

Copy link
Copy Markdown
Member

@copilot, make these messages debug level as per review comments

Co-authored-by: roman-khimov <22092804+roman-khimov@users.noreply.github.com>

Copilot AI commented Aug 26, 2026

Copy link
Copy Markdown
Contributor Author

@copilot, make these messages debug level as per review comments

Done — all four log sites changed from Warn to Debug in the latest commit.

@roman-khimov
roman-khimov merged commit 5bc13f9 into master Aug 26, 2026
20 of 22 checks passed
@roman-khimov
roman-khimov deleted the copilot/add-logging-to-object-server branch August 26, 2026 15:06
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.

Add logging to object server

3 participants