Skip to content

Conversation

@wolfeidau
Copy link
Member

@wolfeidau wolfeidau commented Sep 14, 2025

This is based on the work in open-telemetry/semantic-conventions#2083 with the goal to be standardised monitoring of this MCP server.

  • improved the logging for local interactive users
  • added a new outer tool span which holds the semantic fields
  • sorted out error reporting for tools and resources
  • fixed the annoying error printed for tracing as I wasn't configuring it correctly

This is based on the work in open-telemetry/semantic-conventions#2083 with the goal to be standardised monitoring of this MCP server.
@wolfeidau wolfeidau marked this pull request as ready for review September 16, 2025 03:56
@Copilot Copilot AI review requested due to automatic review settings September 16, 2025 03:56
Copy link

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

This PR adds standardized OpenTelemetry (OTEL) monitoring to the MCP server by implementing configurable trace exporters and comprehensive instrumentation hooks. The changes support different exporter types (OTLP, gRPC, noop) and add tracing middleware for tool and resource handlers.

  • Added configurable OTEL exporter support with OTLP, gRPC, and noop options
  • Implemented tracing middleware for tool and resource handlers with structured logging
  • Updated test coverage to validate different exporter configurations

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
pkg/trace/trace.go Added configurable exporter factory, tracing middleware functions, and updated hooks
pkg/trace/trace_test.go Enhanced tests to validate different exporter types and improved assertions
pkg/server/mcp.go Integrated tracing middleware for tool and resource handlers
cmd/buildkite-mcp-server/main.go Added OTEL exporter configuration and improved console logging for interactive terminals
internal/commands/stdio.go Added startup logging message
go.mod Moved go-isatty dependency from indirect to direct

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@wolfeidau wolfeidau changed the title feat: adding new hooks to ensure otel attributes are set feat: adding handler middleware to ensure otel attributes are set Sep 16, 2025
@wolfeidau wolfeidau force-pushed the feat_otel_hooks branch 2 times, most recently from 904afef to d508fb3 Compare September 16, 2025 04:15
* improved the logging for local interactive users
* added a new outer tool span which holds the semantic fields
* sorted out error reporting for tools and resources
* fixed the annoying error printed for tracing as I wasn't configuring it correctly
Copy link
Contributor

@catkins catkins left a comment

Choose a reason for hiding this comment

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

One little comment on the protocol name but looks good otherwise.

@catkins
Copy link
Contributor

catkins commented Sep 16, 2025

🚀

So by default the logger is info, json when non intractive, pretty when interactive. If you enable debug it level is changed to reflect this.

To simplify things the global logger is added to the MCP context used by the STDIO server.
@wolfeidau wolfeidau requested a review from catkins September 17, 2025 03:42
@wolfeidau wolfeidau merged commit 752873f into main Sep 17, 2025
1 check passed
@wolfeidau wolfeidau deleted the feat_otel_hooks branch September 17, 2025 03:46
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