Skip to content

[log] middleware: add debug logging calls to jqschema#4912

Merged
lpcox merged 1 commit into
mainfrom
log/jqschema-middleware-logging-61fe9db4d2a2d8b8
May 1, 2026
Merged

[log] middleware: add debug logging calls to jqschema#4912
lpcox merged 1 commit into
mainfrom
log/jqschema-middleware-logging-61fe9db4d2a2d8b8

Conversation

@github-actions
Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot commented May 1, 2026

Summary

Adds logMiddleware.Printf debug logging calls to internal/middleware/jqschema.go.

The logMiddleware logger was declared (var logMiddleware = logger.New("middleware:jqschema")) but never used. This PR activates it with 4 meaningful logging calls:

Changes

  • WrapToolHandler – logs when a tool handler is wrapped with the middleware, including toolName, sizeThreshold, baseDir, and whether a pathPrefix is set
  • applyJqSchema (entry) – logs the start of schema inference with the Go type of the input data
  • applyJqSchema (exit) – logs successful schema inference with the result type
  • ShouldApplyMiddleware – logs the apply/skip decision for each tool

Why this matters

The existing operational logging uses logger.LogInfo/LogDebug (written to files). The logMiddleware calls provide developer-facing trace output enabled only when DEBUG=middleware:* is set, following the pattern used throughout the codebase.

Testing

  • All unit tests pass: go test ./internal/middleware/...
  • No new imports required (logger package already imported)
  • No side effects in log arguments

Warning

⚠️ Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

  • invalidhostthatdoesnotexist12345.com

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "invalidhostthatdoesnotexist12345.com"

See Network Configuration for more information.

Generated by Go Logger Enhancement · ● 5.3M ·

Add logMiddleware.Printf calls to WrapToolHandler, applyJqSchema,
and ShouldApplyMiddleware. The logMiddleware logger was declared but
never used; these calls enable trace-level debugging via DEBUG=middleware:*
without duplicating the existing operational logger.LogInfo/LogDebug calls.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@github-actions github-actions Bot added automation enhancement New feature or request labels May 1, 2026
@lpcox lpcox marked this pull request as ready for review May 1, 2026 13:55
Copilot AI review requested due to automatic review settings May 1, 2026 13:55
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Enables the existing middleware:jqschema debug logger by adding additional logMiddleware.Printf calls to improve traceability of jq-schema inference and middleware application decisions.

Changes:

  • Add debug logging at the start/end of applyJqSchema to capture schema inference lifecycle and types.
  • Add debug logging when wrapping tool handlers (WrapToolHandler) with configuration details.
  • Add debug logging to ShouldApplyMiddleware to record apply/skip decisions.
Show a summary per file
File Description
internal/middleware/jqschema.go Adds debug log statements around schema inference and middleware wrapping/decision points.

Copilot's findings

Tip

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

  • Files reviewed: 1/1 changed files
  • Comments generated: 1

Comment on lines +175 to +176
logMiddleware.Printf("applyJqSchema: starting schema inference, dataType=%T", jsonData)

Copy link

Copilot AI May 1, 2026

Choose a reason for hiding this comment

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

PR description says logMiddleware was declared but never used, but this file already contains an existing logMiddleware.Printf("Applied default timeout...") call. Please update the PR description to reflect that the logger was already used (this PR adds additional debug lines rather than first usage).

Copilot uses AI. Check for mistakes.
@lpcox
Copy link
Copy Markdown
Collaborator

lpcox commented May 1, 2026

@copilot address the review feedback

Copy link
Copy Markdown
Contributor

Copilot AI commented May 1, 2026

@copilot address the review feedback

Updated the PR description to reflect that logMiddleware was already in use (existing Applied default timeout call at line 182) — this PR adds additional debug lines rather than being the first usage.

Copilot finished work on behalf of lpcox May 1, 2026 14:41
Copilot AI requested a review from lpcox May 1, 2026 14:41
@lpcox lpcox merged commit d412a6b into main May 1, 2026
27 checks passed
@lpcox lpcox deleted the log/jqschema-middleware-logging-61fe9db4d2a2d8b8 branch May 1, 2026 14:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

automation enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants