Skip to content

Document centralized logging strategy#86

Draft
smar-imran-khawaja with Copilot wants to merge 4 commits into
fix-vulnerabilityfrom
copilot/sub-pr-84-again
Draft

Document centralized logging strategy#86
smar-imran-khawaja with Copilot wants to merge 4 commits into
fix-vulnerabilityfrom
copilot/sub-pr-84-again

Conversation

Copilot AI commented Jan 8, 2026

Copy link
Copy Markdown

Pull Request

Description

Addresses logging consistency feedback from PR #84. The codebase already uses a centralized logger utility (introduced in commit 98b7de6) that ensures MCP protocol compliance. This PR adds documentation to make the logging strategy explicit.

Related Issue

Motivation and Context

The original feedback noted inconsistent logging (console.error, console.debug, console.warn). While the code was already fixed to use a centralized logger, the strategy was undocumented, making it unclear to developers why and how to use it.

How Has This Been Tested?

  • Manual Testing
  • Build verification

Screenshots (if appropriate)

N/A

Types of changes

  • Documentation update

Checklist

  • My code follows the code style of this project
  • My change requires a change to the documentation
  • I have updated the documentation accordingly
  • I have added tests to cover my changes
  • All new and existing tests passed
  • I have checked that my changes do not introduce any new warnings

Changes

README.md

  • Added "Logging" section under Development with usage examples
  • Documents the six log levels and when to use logger vs console.*

docs/LOGGING.md (new)

  • Why: MCP protocol uses stdout for JSON-RPC; console.* corrupts it
  • Log levels: debug, info, notice, warning, error, critical
  • Behavior: stderr before connection, sendLoggingMessage after
  • Migration guide from console.* methods
  • Testing instructions for development and Claude Desktop

src/utils/logger.ts

  • Added JSDoc explaining the centralized logging approach

Usage Example

import { logger } from '../utils/logger.js';

// Before server connection: logs to stderr
logger.info('Processing sheet data');
logger.warning('Rate limit approaching');
logger.error('Failed to fetch data');

// After connection: sent via MCP's sendLoggingMessage
// Visible in Claude Desktop's MCP logs panel

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Copilot AI and others added 3 commits January 8, 2026 00:20
- Added logging section to README.md explaining the centralized logger
- Created docs/LOGGING.md with detailed logging guidelines
- Added JSDoc comments to logger utility for better documentation
- Documented log levels, usage patterns, and best practices

Co-authored-by: smar-imran-khawaja <174051013+smar-imran-khawaja@users.noreply.github.com>
Co-authored-by: smar-imran-khawaja <174051013+smar-imran-khawaja@users.noreply.github.com>
- Clarified filtering instructions for Claude Desktop MCP logs
- Better explanation of why alert/emergency levels aren't exposed

Co-authored-by: smar-imran-khawaja <174051013+smar-imran-khawaja@users.noreply.github.com>
Copilot AI changed the title [WIP] Address feedback on vulnerability fix in PR #84 Document centralized logging strategy Jan 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants