-
Notifications
You must be signed in to change notification settings - Fork 1
Metrics #370
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
base: master
Are you sure you want to change the base?
Metrics #370
Conversation
There was a problem hiding this 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 pull request introduces comprehensive metrics collection and monitoring capabilities using Prometheus. The PR implements tracking of database operations, WebSocket connections, storage operations, and various Nostr-specific events to provide observability into the application's performance and usage.
Key changes include:
- Added a new monitoring package with Prometheus metrics collection
- Instrumented WebSocket handlers to track operations and connection states
- Added metrics collection for storage operations and authentication events
- Integrated database query tracing and connection pool monitoring
Reviewed Changes
Copilot reviewed 13 out of 14 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| monitoring/metrics.go | New comprehensive metrics package with Prometheus collectors for all application components |
| storage/global.go | Added storage size monitoring and periodic metrics collection |
| server/ws/ws.go | Instrumented WebSocket operations with duration and success tracking |
| server/ws/subscriptions.go | Added subscription and authentication metrics tracking |
| server/ws/internal/adapters/websocket.go | Added connection lifecycle tracking |
| server/ws/auth.go | Added authenticated users counter |
| server/server.go | Exposed Prometheus metrics endpoint |
| server/http/nip96/storage_nip96.go | Added comprehensive storage request metrics |
| database/query/query.go | Added events count query for database metrics |
| database/query/internal/connector/storage.go | Integrated database connection and query tracing |
| database/query/global.go | Enhanced database info collection with events count |
| cmd/subzero-ion-connect/subzero_ion_connect.go | Added monitoring initialization and event storage tracking |
| go.mod | Added Prometheus client library dependencies |
Comments suppressed due to low confidence (1)
database/query/internal/connector/storage.go:238
- If the URL parsing succeeds but u.Host is empty, the host variable will be set to an empty string, which may not be the intended behavior. Consider adding a check to ensure u.Host is not empty before assignment.
}
|
|
||
| err := filepath.Walk(dirPath, func(path string, info os.FileInfo, err error) error { | ||
| if err != nil { | ||
| return nil |
Copilot
AI
Aug 1, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The error from filepath.Walk is returned as nil on line 465, which means the actual error is lost. This should return the error to properly handle directory traversal failures.
| return nil | |
| return err |
| func IncreaseActiveSubscriptions() { | ||
| count := activeSubscriptionsCounter.Add(1) |
Copilot
AI
Aug 1, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The function name IncreaseActiveSubscriptions() suggests it increases by 1, but DecreaseActiveSubscriptions(delta int64) takes a parameter. For consistency, consider adding a parameter to this function or renaming it to IncreaseActiveSubscriptionsByOne().
| func IncreaseActiveSubscriptions() { | |
| count := activeSubscriptionsCounter.Add(1) | |
| func IncreaseActiveSubscriptions(delta int64) { | |
| count := activeSubscriptionsCounter.Add(delta) |
| subsCount := 0 | ||
| h.Subscriptions.Range(func(_ string, sub subscription) bool { | ||
| if sub.Writer == respWriter { | ||
| h.Subscriptions.Delete(sub.Source.ID) | ||
| subsCount++ | ||
| } | ||
| return true | ||
| }) | ||
| if subsCount > 0 { | ||
| monitoring.DecreaseActiveSubscriptions(int64(subsCount)) | ||
| } |
Copilot
AI
Aug 1, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The DecreaseActiveSubscriptions function is called after counting all subscriptions in a loop. Consider calling it once per subscription inside the loop to avoid potential race conditions where the count becomes temporarily inconsistent.
| subsCount := 0 | |
| h.Subscriptions.Range(func(_ string, sub subscription) bool { | |
| if sub.Writer == respWriter { | |
| h.Subscriptions.Delete(sub.Source.ID) | |
| subsCount++ | |
| } | |
| return true | |
| }) | |
| if subsCount > 0 { | |
| monitoring.DecreaseActiveSubscriptions(int64(subsCount)) | |
| } | |
| h.Subscriptions.Range(func(_ string, sub subscription) bool { | |
| if sub.Writer == respWriter { | |
| h.Subscriptions.Delete(sub.Source.ID) | |
| monitoring.DecreaseActiveSubscriptions(1) | |
| } | |
| return true | |
| }) |
No description provided.