Draft
Conversation
Adds a sensible logging integration on top of the (currently unused)
observability.Exporters scaffolding:
* pkg/github/logging_middleware.go
ToolLoggingMiddleware times every tools/call and logs at Debug on
success / Error on failure (Go error or IsError result). It enriches
a *slog.Logger with mcp.method and mcp.tool, then stores it on the
context via observability.ContextWithLogger so tool handlers pick up
the request-scoped logger automatically from deps.Logger(ctx).
* pkg/observability/logger_context.go
ContextWithLogger / LoggerFromContext helpers.
* pkg/observability/log_level.go
ParseLogLevel for 'debug|info|warn|error'.
* --log-level flag / GITHUB_LOG_LEVEL env var
Fills an obvious gap: previously levels were hard-coded (stderr=Info,
file=Debug). The default behaviour is preserved when the flag is empty.
* Fix fmt.Fprintf(os.Stderr, ...) feature-flag check reporting in
BaseDeps.IsFeatureEnabled / RequestDeps.IsFeatureEnabled — these
previously bypassed the structured logger entirely.
Tool handler code is intentionally untouched. The middleware gives every
tool a duration/outcome log line uniformly; tools that want richer
structured logs can opt in via deps.Logger(ctx).
Tests cover the middleware (success / error / IsError / non-tool
pass-through / missing deps), the context helpers, level parsing, and
the BaseDeps.Logger(ctx) context fallback.
Co-authored-by: Copilot <[email protected]>
1df340e to
9d3e8b7
Compare
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
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.
Adds a sensible logging integration on top of the (currently unused) observability.Exporters scaffolding:
pkg/github/logging_middleware.go ToolLoggingMiddleware times every tools/call and logs at Debug on success / Error on failure (Go error or IsError result). It enriches a *slog.Logger with mcp.method and mcp.tool, then stores it on the context via observability.ContextWithLogger so tool handlers pick up the request-scoped logger automatically from deps.Logger(ctx).
pkg/observability/logger_context.go ContextWithLogger / LoggerFromContext helpers.
pkg/observability/log_level.go ParseLogLevel for 'debug|info|warn|error'.
--log-level flag / GITHUB_LOG_LEVEL env var Fills an obvious gap: previously levels were hard-coded (stderr=Info, file=Debug). The default behaviour is preserved when the flag is empty.
Fix fmt.Fprintf(os.Stderr, ...) feature-flag check reporting in BaseDeps.IsFeatureEnabled / RequestDeps.IsFeatureEnabled — these previously bypassed the structured logger entirely.
Tool handler code is intentionally untouched. The middleware gives every tool a duration/outcome log line uniformly; tools that want richer structured logs can opt in via deps.Logger(ctx).
Tests cover the middleware (success / error / IsError / non-tool pass-through / missing deps), the context helpers, level parsing, and the BaseDeps.Logger(ctx) context fallback.
Co-authored-by: Copilot [email protected]
Summary
Why
Fixes #
What changed
MCP impact
Prompts tested (tool changes only)
Security / limits
Tool renaming
deprecated_tool_aliases.goNote: if you're renaming tools, you must add the tool aliases. For more information on how to do so, please refer to the official docs.
Lint & tests
./script/lint./script/testDocs