feat(api): new storage server#64
Conversation
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## devbox-daemon-v2 #64 +/- ##
===================================================
Coverage ? 61.97%
===================================================
Files ? 8
Lines ? 647
Branches ? 0
===================================================
Hits ? 401
Misses ? 200
Partials ? 46 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Zllinc
force-pushed
the
newThinpool
branch
4 times, most recently
from
59522d9 to
df135e7
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR introduces a new storage server component that provides gRPC-based storage monitoring and statistics collection for the Sealos devbox system. The implementation adds comprehensive LVM storage metrics collection, VictoriaMetrics integration, and replaces the existing storage stats provider with a client-server architecture.
Key Changes
- Storage server architecture: Implements a gRPC server for storage statistics collection with health checking capabilities
- LVM metrics collection: Adds detailed thin pool monitoring with VictoriaMetrics export functionality
- Client-server integration: Replaces direct storage stats collection with gRPC client communication in the devbox controller
Reviewed Changes
Copilot reviewed 22 out of 23 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| controllers/devbox/stat/storage_server/storage_server.go | New gRPC server implementation for storage statistics with health checking |
| controllers/devbox/stat/storage_client/storage_client.go | New gRPC client for communicating with storage server |
| controllers/devbox/stat/stat.go | Core storage statistics collection with LVM provider and VictoriaMetrics export |
| controllers/devbox/stat/proto/*.go | Generated protobuf files for gRPC communication |
| controllers/devbox/stat/exporter/export.go | VictoriaMetrics exporter for storage metrics |
| controllers/devbox/stat/cmd/main.go | Main entry point for storage server with monitoring capabilities |
| controllers/devbox/internal/controller/devbox_controller.go | Updated to use storage client instead of direct stats provider |
| controllers/devbox/cmd/main.go | Integration of storage client initialization |
Comments suppressed due to low confidence (1)
controllers/devbox/stat/storage_server/storage_server.go:1
- The comment line 71 in the diff appears to be malformed or contains placeholder text that should be removed.
package storageserver
|
|
||
| package storage; | ||
|
|
||
| option go_package = "github.com/labring/sealos/controllers/devbox/internal/stat/proto"; |
There was a problem hiding this comment.
The go_package path is incorrect. It references 'internal/stat/proto' but the actual path is 'stat/proto'. This will cause import issues when the protobuf files are used.
| option go_package = "github.com/labring/sealos/controllers/devbox/internal/stat/proto"; | |
| option go_package = "github.com/labring/sealos/controllers/devbox/stat/proto"; |
| // join data into string | ||
| data := strings.Join(allPrometheusData, "\n") | ||
| // todo : delete data and save length | ||
| logger.Info("[exportToVictoriaMetrics] Sending %d metrics to VM:\n%s\n", len(allPrometheusData), data) |
There was a problem hiding this comment.
Logging the full metrics data at Info level could impact performance and log storage, especially with large metric datasets. Consider using Debug level or removing the data content from the log message.
| logger.Info("[exportToVictoriaMetrics] Sending %d metrics to VM:\n%s\n", len(allPrometheusData), data) | |
| logger.Info("[exportToVictoriaMetrics] Sending %d metrics to VM", len(allPrometheusData)) |
| // TODO: In production, use proper TLS credentials | ||
| // For development, we use insecure connection | ||
| // In production, uncomment the following lines and provide proper certificates: | ||
| // creds := credentials.NewTLS(&tls.Config{ | ||
| // MinVersion: tls.VersionTLS12, | ||
| // }) | ||
| // server := grpc.NewServer(grpc.Creds(creds)) | ||
|
|
||
| // Development mode - insecure connection | ||
| server := grpc.NewServer() |
There was a problem hiding this comment.
The gRPC server is configured without TLS encryption in production code. This could expose sensitive storage metrics data. Consider implementing proper TLS configuration or adding a configuration option to enable secure connections.
Zllinc
force-pushed
the
newThinpool
branch
2 times, most recently
from
ee1739e to
d01739d
Compare
No description provided.