From 6f61284b139c429499a950f5b7b692846a1fc8cd Mon Sep 17 00:00:00 2001 From: Andres Tobon Date: Mon, 11 May 2026 12:15:37 -0700 Subject: [PATCH 1/3] fix: consolidate duplicate error logging to boundary callers 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 --- internal/service/committee_reader.go | 26 ---------------- internal/service/message_handler.go | 44 ---------------------------- 2 files changed, 70 deletions(-) diff --git a/internal/service/committee_reader.go b/internal/service/committee_reader.go index e9f68794..72a4eca4 100644 --- a/internal/service/committee_reader.go +++ b/internal/service/committee_reader.go @@ -65,10 +65,6 @@ func (rc *committeeReaderOrchestrator) GetBase(ctx context.Context, uid string) // Get committee base from storage committeeBase, revision, err := rc.committeeReader.GetBase(ctx, uid) if err != nil { - slog.ErrorContext(ctx, "failed to get committee base", - "error", err, - "committee_uid", uid, - ) return nil, 0, err } @@ -90,10 +86,6 @@ func (rc *committeeReaderOrchestrator) GetSettings(ctx context.Context, uid stri // Get committee settings from storage committeeSettings, revision, err := rc.committeeReader.GetSettings(ctx, uid) if err != nil { - slog.ErrorContext(ctx, "failed to get committee settings", - "error", err, - "committee_uid", uid, - ) return nil, 0, err } @@ -137,31 +129,17 @@ func (rc *committeeReaderOrchestrator) GetMember(ctx context.Context, committeeU // First, verify that the committee exists _, _, err := rc.committeeReader.GetBase(ctx, committeeUID) if err != nil { - slog.ErrorContext(ctx, "failed to get committee base - committee does not exist", - "error", err, - "committee_uid", committeeUID, - ) return nil, 0, err } // Get committee member from storage committeeMember, revision, err := rc.committeeReader.GetMember(ctx, memberUID) if err != nil { - slog.ErrorContext(ctx, "failed to get committee member", - "error", err, - "committee_uid", committeeUID, - "member_uid", memberUID, - ) return nil, 0, err } // Verify that the member belongs to the requested committee if committeeMember.CommitteeUID != committeeUID { - slog.ErrorContext(ctx, "committee member does not belong to the requested committee", - "committee_uid", committeeUID, - "member_uid", memberUID, - "member_committee_uid", committeeMember.CommitteeUID, - ) return nil, 0, errs.NewValidation("committee member does not belong to the requested committee") } @@ -184,10 +162,6 @@ func (rc *committeeReaderOrchestrator) ListMembers(ctx context.Context, committe // Get all committee members from storage members, err := rc.committeeReader.ListMembers(ctx, committeeUID) if err != nil { - slog.ErrorContext(ctx, "failed to list committee members", - "error", err, - "committee_uid", committeeUID, - ) return nil, err } diff --git a/internal/service/message_handler.go b/internal/service/message_handler.go index b80c832b..df997922 100644 --- a/internal/service/message_handler.go +++ b/internal/service/message_handler.go @@ -71,36 +71,22 @@ func (m *messageHandlerOrchestrator) HandleCommitteeGetAttribute(ctx context.Con // Validate that the committee ID is a valid UUID. _, err := uuid.Parse(uid) if err != nil { - slog.ErrorContext(ctx, "error parsing committee ID", "error", err) return nil, err } // Use the committee reader to get the committee base information committee, _, err := m.committeeReader.GetBase(ctx, uid) if err != nil { - slog.ErrorContext(ctx, "failed to get committee base", - "error", err, - "committee_uid", uid, - ) return nil, err } value, ok := fields.LookupByTag(committee, "json", attribute) if !ok { - slog.ErrorContext(ctx, "attribute not found in committee", - "attribute", attribute, - "committee_uid", uid, - ) return nil, errors.NewNotFound(fmt.Sprintf("attribute %s not found in committee %s", attribute, uid)) } strValue, ok := value.(string) if !ok { - slog.ErrorContext(ctx, "attribute value is not a string", - "attribute", attribute, - "committee_uid", uid, - "value_type", fmt.Sprintf("%T", value), - ) return nil, errors.NewValidation(fmt.Sprintf("attribute %s value is not a string", attribute)) } @@ -120,37 +106,24 @@ func (m *messageHandlerOrchestrator) HandleCommitteeListMembers(ctx context.Cont // Validate that the committee ID is a valid UUID. _, err := uuid.Parse(uid) if err != nil { - slog.ErrorContext(ctx, "error parsing committee ID", "error", err) return nil, err } // Check if the committee exists first _, _, err = m.committeeReader.GetBase(ctx, uid) if err != nil { - slog.ErrorContext(ctx, "failed to get committee base", - "error", err, - "committee_uid", uid, - ) return nil, err } // Get all members for the committee members, err := m.committeeReader.ListMembers(ctx, uid) if err != nil { - slog.ErrorContext(ctx, "failed to list committee members", - "error", err, - "committee_uid", uid, - ) return nil, err } // Marshal the members to JSON membersJSON, err := json.Marshal(members) if err != nil { - slog.ErrorContext(ctx, "failed to marshal committee members", - "error", err, - "committee_uid", uid, - ) return nil, errors.NewUnexpected("failed to marshal committee members", err) } @@ -167,7 +140,6 @@ func (m *messageHandlerOrchestrator) HandleCommitteeListMembers(ctx context.Cont func (m *messageHandlerOrchestrator) HandleCommitteeMailingListChanged(ctx context.Context, msg port.TransportMessenger) ([]byte, error) { 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 } @@ -183,8 +155,6 @@ func (m *messageHandlerOrchestrator) HandleCommitteeMailingListChanged(ctx conte committee, changed, err := m.committeeWriter.UpdateHasMailingList(ctx, event.CommitteeUID, event.HasMailingList) if err != nil { - slog.ErrorContext(ctx, "failed to update has_mailing_list", - "committee_uid", event.CommitteeUID, "error", err) return nil, err } if !changed { @@ -202,15 +172,11 @@ func (m *messageHandlerOrchestrator) HandleCommitteeMailingListChanged(ctx conte indexerMsg, err := buildIndexerMessage(ctx, model.ActionUpdated, committee, fullCommittee.Tags()) if err != nil { - slog.ErrorContext(ctx, "failed to build indexer message", - "committee_uid", event.CommitteeUID, "error", err) return nil, err } indexerMsg.IndexingConfig = buildCommitteeIndexingConfig(fullCommittee) if err := m.committeePublisher.Indexer(ctx, constants.IndexCommitteeSubject, indexerMsg, false); err != nil { - slog.ErrorContext(ctx, "failed to publish committee indexer update", - "committee_uid", event.CommitteeUID, "error", err) return nil, err } @@ -229,7 +195,6 @@ func (m *messageHandlerOrchestrator) HandleCommitteeUpdated(ctx context.Context, 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 } @@ -267,8 +232,6 @@ func (m *messageHandlerOrchestrator) HandleCommitteeUpdated(ctx context.Context, members, err := m.committeeReader.ListMembers(ctx, data.CommitteeUID) if err != nil { - slog.ErrorContext(ctx, "failed to list members for sync", - "committee_uid", data.CommitteeUID, "error", err) return nil, err } @@ -333,7 +296,6 @@ func (m *messageHandlerOrchestrator) HandleCommitteeTotalMembersSync(ctx context 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 } @@ -365,16 +327,12 @@ func (m *messageHandlerOrchestrator) HandleCommitteeTotalMembersSync(ctx context members, err := m.committeeReader.ListMembers(ctx, committeeUID) if err != nil { - slog.ErrorContext(ctx, "failed to list members for total_members sync", - "committee_uid", committeeUID, "error", err) return err } actualCount := len(members) committee, revision, err := m.committeeReader.GetBase(ctx, committeeUID) if err != nil { - slog.ErrorContext(ctx, "failed to get committee base for total_members sync", - "committee_uid", committeeUID, "error", err) return err } @@ -395,8 +353,6 @@ func (m *messageHandlerOrchestrator) HandleCommitteeTotalMembersSync(ctx context committee.TotalMembers = actualCount if _, err := m.committeeWriterOrchestrator.Update(ctx, &model.Committee{CommitteeBase: *committee}, revision, false); err != nil { - slog.ErrorContext(ctx, "failed to update committee total_members", - "committee_uid", committeeUID, "error", err) return err } From ee10551d50a72453b73167b27a8764ad6ca257f0 Mon Sep 17 00:00:00 2001 From: Andres Tobon Date: Mon, 11 May 2026 12:40:55 -0700 Subject: [PATCH 2/3] =?UTF-8?q?fix(review):=20address=20PR=20#88=20review?= =?UTF-8?q?=20feedback=20=E2=80=94=20enrich=20ctx=20with=20structured=20id?= =?UTF-8?q?entifiers?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- internal/service/committee_reader.go | 6 ++++++ internal/service/message_handler.go | 7 +++++++ 2 files changed, 13 insertions(+) diff --git a/internal/service/committee_reader.go b/internal/service/committee_reader.go index 72a4eca4..86d612f6 100644 --- a/internal/service/committee_reader.go +++ b/internal/service/committee_reader.go @@ -12,6 +12,7 @@ import ( "github.com/linuxfoundation/lfx-v2-committee-service/internal/domain/port" errs "github.com/linuxfoundation/lfx-v2-committee-service/pkg/errors" "github.com/linuxfoundation/lfx-v2-committee-service/pkg/fields" + "github.com/linuxfoundation/lfx-v2-committee-service/pkg/log" ) // CommitteeReader defines the interface for committee read operations @@ -58,6 +59,7 @@ type committeeReaderOrchestrator struct { // GetBase retrieves committee base information by UID func (rc *committeeReaderOrchestrator) GetBase(ctx context.Context, uid string) (*model.CommitteeBase, uint64, error) { + ctx = log.AppendCtx(ctx, slog.String("committee_uid", uid)) slog.DebugContext(ctx, "executing get committee base use case", "committee_uid", uid, ) @@ -79,6 +81,7 @@ func (rc *committeeReaderOrchestrator) GetBase(ctx context.Context, uid string) // GetSettings retrieves committee settings by UID func (rc *committeeReaderOrchestrator) GetSettings(ctx context.Context, uid string) (*model.CommitteeSettings, uint64, error) { + ctx = log.AppendCtx(ctx, slog.String("committee_uid", uid)) slog.DebugContext(ctx, "executing get committee settings use case", "committee_uid", uid, ) @@ -121,6 +124,8 @@ func (rc *committeeReaderOrchestrator) GetMemberRevision(ctx context.Context, me // GetMember retrieves a committee member by committee UID and member UID func (rc *committeeReaderOrchestrator) GetMember(ctx context.Context, committeeUID, memberUID string) (*model.CommitteeMember, uint64, error) { + ctx = log.AppendCtx(ctx, slog.String("committee_uid", committeeUID)) + ctx = log.AppendCtx(ctx, slog.String("member_uid", memberUID)) slog.DebugContext(ctx, "executing get committee member use case", "committee_uid", committeeUID, "member_uid", memberUID, @@ -155,6 +160,7 @@ func (rc *committeeReaderOrchestrator) GetMember(ctx context.Context, committeeU // ListMembers retrieves all members for a given committee UID func (rc *committeeReaderOrchestrator) ListMembers(ctx context.Context, committeeUID string) ([]*model.CommitteeMember, error) { + ctx = log.AppendCtx(ctx, slog.String("committee_uid", committeeUID)) slog.DebugContext(ctx, "executing list committee members use case", "committee_uid", committeeUID, ) diff --git a/internal/service/message_handler.go b/internal/service/message_handler.go index df997922..aa09a566 100644 --- a/internal/service/message_handler.go +++ b/internal/service/message_handler.go @@ -16,6 +16,7 @@ import ( "github.com/linuxfoundation/lfx-v2-committee-service/pkg/constants" "github.com/linuxfoundation/lfx-v2-committee-service/pkg/errors" "github.com/linuxfoundation/lfx-v2-committee-service/pkg/fields" + "github.com/linuxfoundation/lfx-v2-committee-service/pkg/log" ) // messageHandlerOrchestrator orchestrates the message handling process @@ -74,6 +75,9 @@ func (m *messageHandlerOrchestrator) HandleCommitteeGetAttribute(ctx context.Con return nil, err } + ctx = log.AppendCtx(ctx, slog.String("committee_uid", uid)) + ctx = log.AppendCtx(ctx, slog.String("attribute", attribute)) + // Use the committee reader to get the committee base information committee, _, err := m.committeeReader.GetBase(ctx, uid) if err != nil { @@ -109,6 +113,8 @@ func (m *messageHandlerOrchestrator) HandleCommitteeListMembers(ctx context.Cont return nil, err } + ctx = log.AppendCtx(ctx, slog.String("committee_uid", uid)) + // Check if the committee exists first _, _, err = m.committeeReader.GetBase(ctx, uid) if err != nil { @@ -319,6 +325,7 @@ func (m *messageHandlerOrchestrator) HandleCommitteeTotalMembersSync(ctx context committeeUID := member.CommitteeUID 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, From eec5f7663b46105f5e00e75eb865f4d6e8aa6ebd Mon Sep 17 00:00:00 2001 From: Andres Tobon Date: Mon, 11 May 2026 12:47:13 -0700 Subject: [PATCH 3/3] fix: remove redundant explicit log fields already present in context 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 --- internal/service/committee_reader.go | 38 ++++++---------------------- internal/service/message_handler.go | 33 ++++++------------------ 2 files changed, 16 insertions(+), 55 deletions(-) diff --git a/internal/service/committee_reader.go b/internal/service/committee_reader.go index 86d612f6..649520a8 100644 --- a/internal/service/committee_reader.go +++ b/internal/service/committee_reader.go @@ -60,9 +60,7 @@ type committeeReaderOrchestrator struct { func (rc *committeeReaderOrchestrator) GetBase(ctx context.Context, uid string) (*model.CommitteeBase, uint64, error) { ctx = log.AppendCtx(ctx, slog.String("committee_uid", uid)) - slog.DebugContext(ctx, "executing get committee base use case", - "committee_uid", uid, - ) + slog.DebugContext(ctx, "executing get committee base use case") // Get committee base from storage committeeBase, revision, err := rc.committeeReader.GetBase(ctx, uid) @@ -70,10 +68,7 @@ func (rc *committeeReaderOrchestrator) GetBase(ctx context.Context, uid string) return nil, 0, err } - slog.DebugContext(ctx, "committee base retrieved successfully", - "committee_uid", uid, - "revision", revision, - ) + slog.DebugContext(ctx, "committee base retrieved successfully", "revision", revision) return committeeBase, revision, nil } @@ -82,9 +77,7 @@ func (rc *committeeReaderOrchestrator) GetBase(ctx context.Context, uid string) func (rc *committeeReaderOrchestrator) GetSettings(ctx context.Context, uid string) (*model.CommitteeSettings, uint64, error) { ctx = log.AppendCtx(ctx, slog.String("committee_uid", uid)) - slog.DebugContext(ctx, "executing get committee settings use case", - "committee_uid", uid, - ) + slog.DebugContext(ctx, "executing get committee settings use case") // Get committee settings from storage committeeSettings, revision, err := rc.committeeReader.GetSettings(ctx, uid) @@ -92,10 +85,7 @@ func (rc *committeeReaderOrchestrator) GetSettings(ctx context.Context, uid stri return nil, 0, err } - slog.DebugContext(ctx, "committee settings retrieved successfully", - "committee_uid", uid, - "revision", revision, - ) + slog.DebugContext(ctx, "committee settings retrieved successfully", "revision", revision) return committeeSettings, revision, nil } @@ -126,10 +116,7 @@ func (rc *committeeReaderOrchestrator) GetMember(ctx context.Context, committeeU ctx = log.AppendCtx(ctx, slog.String("committee_uid", committeeUID)) ctx = log.AppendCtx(ctx, slog.String("member_uid", memberUID)) - slog.DebugContext(ctx, "executing get committee member use case", - "committee_uid", committeeUID, - "member_uid", memberUID, - ) + slog.DebugContext(ctx, "executing get committee member use case") // First, verify that the committee exists _, _, err := rc.committeeReader.GetBase(ctx, committeeUID) @@ -148,11 +135,7 @@ func (rc *committeeReaderOrchestrator) GetMember(ctx context.Context, committeeU return nil, 0, errs.NewValidation("committee member does not belong to the requested committee") } - slog.DebugContext(ctx, "committee member retrieved successfully", - "committee_uid", committeeUID, - "member_uid", memberUID, - "revision", revision, - ) + slog.DebugContext(ctx, "committee member retrieved successfully", "revision", revision) return committeeMember, revision, nil } @@ -161,9 +144,7 @@ func (rc *committeeReaderOrchestrator) GetMember(ctx context.Context, committeeU func (rc *committeeReaderOrchestrator) ListMembers(ctx context.Context, committeeUID string) ([]*model.CommitteeMember, error) { ctx = log.AppendCtx(ctx, slog.String("committee_uid", committeeUID)) - slog.DebugContext(ctx, "executing list committee members use case", - "committee_uid", committeeUID, - ) + slog.DebugContext(ctx, "executing list committee members use case") // Get all committee members from storage members, err := rc.committeeReader.ListMembers(ctx, committeeUID) @@ -171,10 +152,7 @@ func (rc *committeeReaderOrchestrator) ListMembers(ctx context.Context, committe return nil, err } - slog.DebugContext(ctx, "committee members retrieved successfully", - "committee_uid", committeeUID, - "member_count", len(members), - ) + slog.DebugContext(ctx, "committee members retrieved successfully", "member_count", len(members)) return members, nil } diff --git a/internal/service/message_handler.go b/internal/service/message_handler.go index aa09a566..794ae1cf 100644 --- a/internal/service/message_handler.go +++ b/internal/service/message_handler.go @@ -64,10 +64,9 @@ func (m *messageHandlerOrchestrator) HandleCommitteeGetAttribute(ctx context.Con // Parse message data to extract committee UID uid := string(msg.Data()) - slog.DebugContext(ctx, "committee get name request", - "committee_uid", uid, - "attribute", attribute, - ) + ctx = log.AppendCtx(ctx, slog.String("committee_uid", uid)) + ctx = log.AppendCtx(ctx, slog.String("attribute", attribute)) + slog.DebugContext(ctx, "committee get name request") // Validate that the committee ID is a valid UUID. _, err := uuid.Parse(uid) @@ -75,9 +74,6 @@ func (m *messageHandlerOrchestrator) HandleCommitteeGetAttribute(ctx context.Con return nil, err } - ctx = log.AppendCtx(ctx, slog.String("committee_uid", uid)) - ctx = log.AppendCtx(ctx, slog.String("attribute", attribute)) - // Use the committee reader to get the committee base information committee, _, err := m.committeeReader.GetBase(ctx, uid) if err != nil { @@ -103,9 +99,8 @@ func (m *messageHandlerOrchestrator) HandleCommitteeListMembers(ctx context.Cont // Parse message data to extract committee UID uid := string(msg.Data()) - slog.DebugContext(ctx, "committee list members request", - "committee_uid", uid, - ) + ctx = log.AppendCtx(ctx, slog.String("committee_uid", uid)) + slog.DebugContext(ctx, "committee list members request") // Validate that the committee ID is a valid UUID. _, err := uuid.Parse(uid) @@ -113,8 +108,6 @@ func (m *messageHandlerOrchestrator) HandleCommitteeListMembers(ctx context.Cont return nil, err } - ctx = log.AppendCtx(ctx, slog.String("committee_uid", uid)) - // Check if the committee exists first _, _, err = m.committeeReader.GetBase(ctx, uid) if err != nil { @@ -133,10 +126,7 @@ func (m *messageHandlerOrchestrator) HandleCommitteeListMembers(ctx context.Cont return nil, errors.NewUnexpected("failed to marshal committee members", err) } - slog.DebugContext(ctx, "committee list members response", - "committee_uid", uid, - "member_count", len(members), - ) + slog.DebugContext(ctx, "committee list members response", "member_count", len(members)) return membersJSON, nil } @@ -327,10 +317,7 @@ func (m *messageHandlerOrchestrator) HandleCommitteeTotalMembersSync(ctx context 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) members, err := m.committeeReader.ListMembers(ctx, committeeUID) if err != nil { @@ -344,15 +331,11 @@ func (m *messageHandlerOrchestrator) HandleCommitteeTotalMembersSync(ctx context } if committee.TotalMembers == actualCount { - slog.DebugContext(ctx, "total_members already correct — skipping update", - "committee_uid", committeeUID, - "total_members", actualCount, - ) + slog.DebugContext(ctx, "total_members already correct — skipping update", "total_members", actualCount) return nil } slog.DebugContext(ctx, "updating total_members counter", - "committee_uid", committeeUID, "previous", committee.TotalMembers, "actual", actualCount, )