Conversation
4 tasks
Agent-Logs-Url: https://github.com/github/gh-aw-mcpg/sessions/73f6ab54-aba0-4958-aa14-8eec3780a37e Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
Copilot
AI
changed the title
[WIP] Refactor duplicate logic and utility in Go codebase
Refactor tracer fallback + logger helper layout to remove duplication and tighten logging semantics
Apr 20, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors shared tracer fallback and logger helper plumbing to remove duplication and centralize key behaviors without changing external behavior.
Changes:
- Added
tracing.GetCachedOrGlobal()and replaced duplicated tracer fallback code inproxyHandlerandUnifiedServer. - Moved the shared
LogLevel -> log functiondispatch map (logFuncs) tointernal/logger/common.goand updated references. - Simplified markdown logging flow by removing
logWithMarkdownLevelindirection and updating tests accordingly. - Clarified
marshalAndSanitizeForLogas best-effort logging behavior when JSON marshaling fails.
Show a summary per file
| File | Description |
|---|---|
| internal/tracing/provider.go | Adds GetCachedOrGlobal helper to centralize tracer fallback behavior. |
| internal/tracing/provider_test.go | Adds coverage for GetCachedOrGlobal with cached and nil tracer inputs. |
| internal/server/unified.go | Switches UnifiedServer.getTracer() to use tracing.GetCachedOrGlobal. |
| internal/proxy/handler.go | Switches proxyHandler.getTracer() to use tracing.GetCachedOrGlobal. |
| internal/server/tool_registry.go | Documents marshalAndSanitizeForLog as intentionally best-effort/silent on marshal failure. |
| internal/logger/common.go | Relocates logFuncs into the shared logger conventions/dispatch section. |
| internal/logger/file_logger.go | Removes the old logFuncs definition now hosted in common.go. |
| internal/logger/markdown_logger.go | Removes logWithMarkdownLevel, simplifies markdown logging helper path. |
| internal/logger/server_file_logger.go | Updates comment to reference logFuncs in common.go. |
| internal/logger/helper_functions_test.go | Renames markdown helper test to reflect new helper layout. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 10/10 changed files
- Comments generated: 0
This was referenced Apr 20, 2026
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.
Semantic clustering flagged four targeted refactor opportunities: duplicated tracer fallback logic, misplaced shared logger dispatch state, unnecessary markdown logging indirection, and unclear marshal-error handling in a logging helper. This PR consolidates those seams without changing external behavior.
Tracer fallback deduplicated (
internal/tracing,internal/proxy,internal/server)tracing.GetCachedOrGlobal(cached trace.Tracer)as the single fallback path.getTracer()implementations inproxyHandlerandUnifiedServerwith one-line calls to the shared helper.logFuncsownership moved tocommon.go(internal/logger)LogLevel -> functiondispatch map fromfile_logger.gotocommon.go.Markdown logging helper flow simplified (
internal/logger/markdown_logger.go)logWithMarkdownLevelindirection.logWithMarkdownto resolvelogFuncs[level]internally, reducing one helper layer and tightening call flow forLog*Mdwrappers.Marshal helper intent made explicit (
internal/server/tool_registry.go)marshalAndSanitizeForLogas best-effort logging behavior when JSON marshaling fails (sanitized empty-string fallback by design).Warning
Firewall rules blocked me from connecting to one or more addresses (expand for details)
I tried to connect to the following addresses, but was blocked by firewall rules:
example.com/tmp/go-build1164881766/b513/launcher.test /tmp/go-build1164881766/b513/launcher.test -test.testlogfile=/tmp/go-build1164881766/b513/testlog.txt -test.paniconexit0 -test.timeout=10m0s /tmp/go-build1164881766/b437/vet.cfg _.a go x_amd64/vet --gdwarf-5 --64 -o x_amd64/vet -E mBfaT_lUZ 544961/b287/ x_amd64/vet -I . -imultiarch x_amd64/vet(dns block)Agent-Logs-Url: REDACTED ithub-guard/rust-guard/target/de/home/REDACTED/work/gh-aw-mcpg/gh-aw-mcpg/guards/github-guard/rust/tmp/go-build3536398537/b507/guard.test Ld/symbols.o 437f known-linux-gnu/-bool known-linux-gnu/-buildtags know�� known-linux-gnu/-errorsas known-linux-gnu/-ifaceassert known-linux-gnu/-nilfunc known-linux-gnu/bash known-linux-gnu//usr/bin/runc known-linux-gnu/--version known-linux-gnu/-tests` (dns block)
Agent-Logs-Url: REDACTED ithub-guard/rust-guard/target/de/home/REDACTED/work/gh-aw-mcpg/gh-aw-mcpg/guards/github-guard/rust/tmp/go-build3536398537/b507/guard.test Ld/symbols.o 437f known-linux-gnu/-bool known-linux-gnu/-buildtags know�� known-linux-gnu/-errorsas known-linux-gnu/-ifaceassert known-linux-gnu/-nilfunc known-linux-gnu/bash known-linux-gnu//usr/bin/runc known-linux-gnu/--version known-linux-gnu/-tests` (dns block)
Agent-Logs-Url: REDACTED ithub-guard/rust-guard/target/de/home/REDACTED/work/gh-aw-mcpg/gh-aw-mcpg/guards/github-guard/rust/tmp/go-build3536398537/b507/guard.test Ld/symbols.o 437f known-linux-gnu/-bool known-linux-gnu/-buildtags know�� known-linux-gnu/-errorsas known-linux-gnu/-ifaceassert known-linux-gnu/-nilfunc known-linux-gnu/bash known-linux-gnu//usr/bin/runc known-linux-gnu/--version known-linux-gnu/-tests` (dns block)