[LFXV2-1701] fix: consolidate duplicate error logging to boundary callers by andrest50 · Pull Request #88 · linuxfoundation/lfx-v2-committee-service

andrest50 · 2026-05-11T19:29:45Z

Summary

Errors returned from the service orchestration layer were being logged 2-3 times per error: once in the internal orchestrator, again in the message handler, and once more at the transport boundary (wrapError for HTTP, committee_handler for NATS, stream_consumer for JetStream)
Removed redundant slog.ErrorContext calls from committee_reader.go and message_handler.go for all errors that are simply returned to a caller
Per-member error logs inside the HandleCommitteeUpdated sync loop are retained since they carry unique member_uid context that the outer caller cannot replicate

Ticket

🤖 Generated with Claude Code

Errors returned from the service orchestration layer were being logged both at the point of origin and again by the boundary caller (wrapError for HTTP, committee_handler for NATS, stream_consumer for JetStream), producing 2-3 redundant log lines per error. Remove the intermediate slog.ErrorContext calls so each error is logged exactly once at the outermost handler. Per-member error logs inside the sync loop are kept because they carry unique member_uid context not available to the caller. Generated with [Claude Code](https://claude.ai/code) Signed-off-by: Andres Tobon <andrest2455@gmail.com>

coderabbitai · 2026-05-11T19:29:57Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 11e2b908-8ef7-430f-8777-ddd0467f289d

📥 Commits

Reviewing files that changed from the base of the PR and between ee10551 and eec5f76.

📒 Files selected for processing (2)

internal/service/committee_reader.go
internal/service/message_handler.go

🚧 Files skipped from review as they are similar to previous changes (2)

internal/service/committee_reader.go
internal/service/message_handler.go

Walkthrough

Removed multiple slog.ErrorContext error logs across committee reader and message-handler methods while adding request metadata (committee_uid, member_uid, attribute) into contexts; control flow and returned errors are unchanged.

Changes

Error-Context Logging Removal

Layer / File(s)	Summary
Committee Reader error paths `internal/service/committee_reader.go`	Error logging removed from `GetBase`, `GetSettings`, `GetMember` (including committee existence, member fetch, and membership validation), and `ListMembers`; `committee_uid`/`member_uid` are appended to context before reads.
GetAttribute handler `internal/service/message_handler.go`	`HandleCommitteeGetAttribute` no longer logs errors for UUID parse, missing base/attribute, or non-string attribute; it appends `committee_uid` and `attribute` to context and returns underlying errors directly.
ListMembers handler `internal/service/message_handler.go`	`HandleCommitteeListMembers` no longer logs errors for UID parse, missing base, member listing, or marshaling; appends `committee_uid` to context and returns errors directly (marshal errors still wrapped).
MailingListChanged handler `internal/service/message_handler.go`	`HandleCommitteeMailingListChanged` removes error logging for JSON unmarshal, indexer-message building, and publish failures; errors are returned directly.
CommitteeUpdated handler `internal/service/message_handler.go`	`HandleCommitteeUpdated` removes error logging for event JSON unmarshal and for member-listing failures during member-sync; errors are returned directly.
TotalMembersSync handler `internal/service/message_handler.go`	`HandleCommitteeTotalMembersSync` appends `committee_uid` to context and removes error logging for event unmarshal, member listing, base retrieval, and counter update failures; errors are returned directly.

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and specifically summarizes the main change: consolidating duplicate error logging to boundary callers, matching the core objective.
Description check	✅ Passed	The description is directly related to the changeset, explaining the rationale and implementation of consolidating duplicate error logging across service layers.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch fix/consolidate-duplicate-error-logging

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

Copilot

Pull request overview

This PR reduces duplicated error logging by removing slog.ErrorContext calls from service/orchestration code paths and relying on boundary-layer logging (HTTP wrapError, NATS message handler, JetStream consumer) to emit a single error log per failure.

Changes:

Removed redundant slog.ErrorContext calls from internal/service/message_handler.go where errors are simply returned to callers.
Removed redundant slog.ErrorContext calls from internal/service/committee_reader.go where errors are simply returned to callers.
Preserved per-member sync error logs that carry unique member_uid context.

Reviewed changes

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

File	Description
internal/service/message_handler.go	Removes per-error logs in NATS/stream handlers, intending to defer logging to boundary callers.
internal/service/committee_reader.go	Removes per-error logs in reader orchestrator methods, intending to defer logging to boundary callers.

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

…red identifiers Address review comments from copilot-pull-request-reviewer: - committee_reader.go GetBase: append committee_uid to ctx via log.AppendCtx so boundary error logs (wrapError) include this field without per-layer ErrorContext - committee_reader.go GetSettings: same committee_uid enrichment - committee_reader.go GetMember: append both committee_uid and member_uid to ctx - committee_reader.go ListMembers: append committee_uid to ctx - message_handler.go HandleCommitteeGetAttribute: append committee_uid and attribute to ctx after UUID validation so NATS boundary log includes these fields - message_handler.go HandleCommitteeListMembers: append committee_uid to ctx after UUID validation - message_handler.go HandleCommitteeTotalMembersSync: append committee_uid to ctx so JetStream boundary log (stream_consumer) includes committee_uid Resolves 6 review threads. Generated with [Claude Code](https://claude.ai/code) Signed-off-by: Andres Tobon <andrest2455@gmail.com>

Now that committee_uid, member_uid, and attribute are appended to ctx via log.AppendCtx, they no longer need to be passed as explicit fields to individual slog calls within the same function. Also moves AppendCtx before the initial debug log in message handlers so all logs in the function benefit from the enriched context. Generated with [Claude Code](https://claude.ai/code) Signed-off-by: Andres Tobon <andrest2455@gmail.com>

Copilot

Pull request overview

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

 	ctx = context.WithValue(ctx, constants.AuthorizationContextID, "Bearer lfx-v2-committee-service")
+	ctx = log.AppendCtx(ctx, slog.String("committee_uid", committeeUID))

-	slog.DebugContext(ctx, "starting total_members sync",
-		"committee_uid", committeeUID,
-		"subject", subject,
-	)
+	slog.DebugContext(ctx, "starting total_members sync", "subject", subject)



-	)
+	ctx = log.AppendCtx(ctx, slog.String("committee_uid", uid))
+	ctx = log.AppendCtx(ctx, slog.String("attribute", attribute))
+	slog.DebugContext(ctx, "committee get name request")


 	var event model.CommitteeMailingListChangedEvent
 	if err := json.Unmarshal(msg.Data(), &event); err != nil {
-		slog.ErrorContext(ctx, "failed to unmarshal CommitteeMailingListChangedEvent", "error", err)
 		return nil, err


 	var event model.CommitteeEvent
 	if err := json.Unmarshal(msg.Data(), &event); err != nil {
-		slog.ErrorContext(ctx, "failed to unmarshal CommitteeEvent", "error", err)
 		return nil, err


 	var event model.CommitteeEvent
 	if err := json.Unmarshal(msg.Data(), &event); err != nil {
-		slog.ErrorContext(ctx, "failed to unmarshal CommitteeEvent for total_members sync", "error", err)
 		return err


+	ctx = log.AppendCtx(ctx, slog.String("committee_uid", uid))
+	ctx = log.AppendCtx(ctx, slog.String("attribute", attribute))
+	slog.DebugContext(ctx, "committee get name request")


+	ctx = log.AppendCtx(ctx, slog.String("committee_uid", uid))
+	slog.DebugContext(ctx, "executing get committee base use case")


Copilot AI review requested due to automatic review settings May 11, 2026 19:29

andrest50 requested a review from a team as a code owner May 11, 2026 19:29

Copilot AI reviewed May 11, 2026

View reviewed changes

andrest50 added 2 commits May 11, 2026 12:40

Copilot AI review requested due to automatic review settings May 11, 2026 19:47

Copilot started reviewing on behalf of andrest50 May 11, 2026 19:48 View session

Copilot AI reviewed May 11, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[LFXV2-1701] fix: consolidate duplicate error logging to boundary callers#88

[LFXV2-1701] fix: consolidate duplicate error logging to boundary callers#88
andrest50 wants to merge 3 commits into
mainfrom
fix/consolidate-duplicate-error-logging

andrest50 commented May 11, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented May 11, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		ctx = log.AppendCtx(ctx, slog.String("committee_uid", uid))
		slog.DebugContext(ctx, "executing get committee base use case")

Conversation

andrest50 commented May 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Ticket

Uh oh!

coderabbitai Bot commented May 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

andrest50 commented May 11, 2026 •

edited

Loading

coderabbitai Bot commented May 11, 2026 •

edited

Loading