Skip to content
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.

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

chore(RFC): custom metric module system #1125

Closed
wants to merge 1 commit into from

Conversation

kaialang
Copy link

@kaialang kaialang commented Aug 27, 2024

Motivation and Context

This RFC proposes a custom metric module system that allows pluggable modules for metrics, tracing, schema usage, logging, and more. The design of the metric module system is directionally aligned with the core module system design. However, it exposes visitor pattern-like hooks for every event point in the router lifecycle, providing full control. Additionally, it offers logically grouped interfaces for simpler integrations.

Ideally, we want the core module system to expose a superset of hooks of the proposed ones and metrics module can be directly plugin to the core system module. Open for discussion.

TODO

  • Tests or benchmark included
  • Documentation is changed or added on https://app.gitbook.com/
  • PR title must follow [conventional-commit-standard]

Copy link

This PR was marked stale due to lack of activity. It will be closed in 14 days.

@github-actions github-actions bot added the Stale label Sep 11, 2024
MetricOperationPostExecuteHook
}

type MetricOperationHook interface {
Copy link
Contributor

Choose a reason for hiding this comment

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

What was your motivation to create a different abstraction of hooks just for metrics? Wouldn't it be easier and more consistent, if a developer can just leverage the standard hooks to implement custom metrics?
In my opinion, creating a second hook system just for metrics isn't needed.

@@ -0,0 +1,196 @@
---
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this RFC still valid? You already addressing custom metrics support in your other RFC.

Copy link
Author

Choose a reason for hiding this comment

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

Let me close this one and we'll just use the other RFC.

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