Skip to content

Add API calling metrics. #844

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
jealous wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Add API calling metrics. #844

jealous wants to merge 1 commit into from
+269 −46

Conversation

@jealous
Copy link
Contributor

@jealous jealous commented Dec 19, 2025 •
edited
Loading

Purpose of the change

Add prometheus metrics to track the API count and response time.

Description

Add a middleware to track the API call count the response time. Use labels to track the number of different
API calls.

Move all the app middleware to a middleware module.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Refactor (does not change functionality, e.g., code style improvements, linting)
  • Documentation update
  • Project Maintenance (updates to build scripts, CI, etc., that do not affect the main project)
  • Security (improves security without changing functionality)

How Has This Been Tested?

  • Unit Test
  • Integration Test
  • End-to-end Test
  • Test Script (please provide)
  • Manual verification (list step-by-step instructions)

Checklist

  • I have signed the commit(s) within this pull request
  • My code follows the style guidelines of this project (See STYLE_GUIDE.md)
  • I have performed a self-review of my own code
  • I have commented my code
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added unit tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules
  • I have checked my code and corrected any misspellings

Maintainer Checklist

  • Confirmed all checks passed
  • Contributor has signed the commit(s)
  • Reviewed the code
  • Run, Tested, and Verified the change(s) work as expected

Screenshots/Gifs

the metrics page after the change:

截屏2025-12-19 14 06 43

@jealous jealous requested a review from Copilot December 19, 2025 22:13
@jealous jealous self-assigned this Dec 19, 2025
@jealous jealous added the priority: medium Important but not critical. Should be tackled after high-priority tasks. label Dec 19, 2025
Copilot AI reviewed Dec 19, 2025
View reviewed changes
Copy link
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

This PR adds Prometheus metrics to track API call counts and response times for the HTTP server. The implementation introduces two new middleware components: one for access logging and another for recording request metrics with labels for method, path, and status code.

Key Changes:

  • Created a new middleware module with AccessLogMiddleware and RequestMetricsMiddleware classes
  • Refactored MetricsFactoryIdMixin to extract a runtime WithMetricsFactory mixin
  • Moved inline access logging middleware from app.py to the new middleware module

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

File Description
src/memmachine/server/middleware.py Implements new middleware classes for access logging and metrics collection
src/memmachine/server/app.py Removes inline middleware and registers the new middleware classes
src/memmachine/common/configuration/mixin_confs.py Extracts WithMetricsFactory runtime mixin from MetricsFactoryIdMixin
tests/memmachine/server/test_middleware.py Adds comprehensive tests for both middleware classes

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

@jealous jealous force-pushed the bugfix/apiMetrics branch 2 times, most recently from 13d48bb to 7291cf1 Compare December 19, 2025 22:17
@jealous jealous requested a review from a team December 19, 2025 22:21
@o-love o-love requested a review from a team December 19, 2025 22:33
malatewang
malatewang reviewed Dec 19, 2025
View reviewed changes
src/memmachine/server/middleware.py
"""Initialize the middleware and metrics."""
self.metrics_factory_id: str | None = None
super().__init__(app)
metrics_factory = self.get_metrics_factory()
Copy link
Contributor

@malatewang malatewang Dec 19, 2025

Choose a reason for hiding this comment

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

I think the metrics factory id should from the same configuration as the memory?

Copy link
Contributor Author

@jealous jealous Jan 5, 2026

Choose a reason for hiding this comment

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

For now, because we only support one kind of the metric factory, we don't have a configuration. Maybe in the future we can add that if we support multiple metric factory?

@jealous
Add API calling metrics.
1ff5c24 
Add prometheus metrics to track the API count and response time.
@jealous jealous force-pushed the bugfix/apiMetrics branch from 7291cf1 to 1ff5c24 Compare January 5, 2026 19:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@malatewang malatewang malatewang left review comments

Copilot code review Copilot Copilot left review comments

@o-love o-love o-love approved these changes

At least 2 approving reviews are required to merge this pull request.

Assignees

@jealous jealous

Labels

priority: medium Important but not critical. Should be tackled after high-priority tasks.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@jealous @malatewang @o-love