CROSSLINK-209 lms/ncip event logging

[adamdickmeiss]

https://index-data.atlassian.net/browse/CROSSLINK-209

[Copilot]

Pull request overview

Adds LMS/NCIP request/response event logging to patron-request workflows so outbound/inbound messages can be captured in the event stream.

Changes:

• Introduces a SetLogFunc callback on ncipclient.NcipClient and lms.LmsAdapter to surface raw NCIP XML request/response bytes.
• Emits new patron-request notice events (lms-requester-message, lms-supplier-message) containing outgoing/incoming LMS payloads.
• Updates unit tests/mocks and workspace dependency sums accordingly.

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
go.work.sum	Updates workspace dependency checksum entries.
broker/patron_request/service/message-handler.go	Import cleanup (no functional change).
broker/patron_request/service/action.go	Hooks LMS adapter logging into patron-request actions and creates notice events with payloads.
broker/patron_request/service/action_test.go	Updates mocks to satisfy the expanded LmsAdapter interface.
broker/events/eventmodels.go	Adds new event name constants for LMS message notices.
broker/ncipclient/ncipclient.go	Adds NcipLogFunc and SetLogFunc to the public client interface.
broker/ncipclient/ncipclient_impl.go	Implements log callback plumbing and captures marshaled/unmarshaled XML bytes.
broker/ncipclient/ncipclient_test.go	Adds assertions that the log callback is invoked and receives payloads.
broker/lms/lms_adapter.go	Extends LMS adapter interface with SetLogFunc.
broker/lms/lms_adapter_ncip.go	Forwards logging callback into the underlying NCIP client.
broker/lms/lms_adapter_manual.go	Adds no-op SetLogFunc implementation.
broker/lms/lms_adapter_test.go	Updates NCIP client mock to satisfy the expanded interface.

Comments suppressed due to low confidence (1)

broker/ncipclient/ncipclient_impl.go:223

• logFunc currently receives only the HTTP exchange error (err from RequestResponse). If the response contains an NCIP <Problem> (or other post-exchange validation failures), sendReceiveMessage returns an error but the callback still sees err == nil, which will cause downstream event logging to mark these as SUCCESS. Consider determining the final operation error first (including converting <Problem> to an error) and pass that to logFunc so event status reflects real failures.

	err := httpclient.NewClient().RequestResponse(n.client, http.MethodPost, []string{httpclient.ContentTypeApplicationXml},
		n.address, message, &respMessage, marshalFunc, unmarshalFunc)
	if n.logFunc != nil {
		n.logFunc(outgoing, incoming, err)
	}
	if err != nil {
		return nil, fmt.Errorf("NCIP message exchange failed: %s", err.Error())
	}
	if len(respMessage.Problem) > 0 {
		return nil, &NcipError{
			Message: "NCIP message processing failed",
			Problem: respMessage.Problem[0],
		}
	}
	return &respMessage, nil

[Copilot]

Storing full LMS request/response XML bodies in EventData.CustomData can make event.event_data rows very large (responses are allowed up to 10MB by httpclient.DefaultMaxResponseSize). This can bloat the database and slow down event queries. Consider truncating the stored payloads to a reasonable limit, or storing only key fields plus a reference to the full payload elsewhere.

[jakub-id]

Looks good minus the Copilot comments

[Copilot]

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.