Claude/query log collection mloy38#126
Merged
Merged
Conversation
Query logs only reach the response when something calls save_logs( response_id, logger) with a logger carrying the query_log_handler. Two producers never did, so their logs were dropped: - The server callback handler logged the result count and callback/query lookups but never persisted them. Add a save_logs call before returning. - The merge_message worker: (a) the async parent hand-rolls its lifecycle and never calls wrap_up_task, so its "obtained lock"/"drained N"/error lines were lost -- add save_logs in the finally; (b) the actual merge runs in a ProcessPoolExecutor child whose logger has no query handler and whose records can't cross the process boundary, so every merge/filter/kgraph log vanished. Attach a call-scoped QueryLogHandler in the child, return its formatted entries alongside the merged ids, and fold them into the parent's query logger (via a new QueryLogHandler.ingest) so the single save_logs flushes them. Keeping all Redis log writes in the async parent avoids a second sync writer racing on the same response_id key. Add tests for the child log return, handler-leak cleanup, and update the batched-merge tests for the new (merged, log_entries) return contract. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TQwM6cHEH55jYYXRf7wtvB
…errors
Addresses review feedback on the log-collection change:
- Ordering: the stored log list reflected flush order, not event order, so
merge/callback logs surfaced before the lookup logs that preceded them.
get_logs now sorts by each entry's ISO8601 UTC timestamp (stable sort), so
both the async splice in finish_query and the /response endpoint return logs
in chronological order regardless of which worker flushed when.
- Noise: demote per-task plumbing lines that flooded the list to DEBUG --
"Doing task" (dumped the whole payload), "Sending task to", "Finished task",
and the merge worker's "Obtained lock" / "Drained N callback(s)". Meaningful
lines ("Got back N results", "Sent response back to ...") stay at INFO.
- Callback errors weren't saved: the 413 (oversized) and 422 (unparseable)
paths reject before the body's response_id is known, and their logger had no
query handler. Attach the query handler up front (keyed on callback_id) and
add _save_callback_error_logs, which resolves the response_id from the
callback->query mapping and flushes -- run before remove_callback_id, which
deletes that mapping. The two unresolvable 500 paths now at least warn.
Add tests for the timestamp sort (incl. missing-timestamp stability) and for
log persistence on the 413/422 paths.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TQwM6cHEH55jYYXRf7wtvB
Codecov Report❌ Patch coverage is
... and 1 file with indirect coverage changes Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.