-
-
Notifications
You must be signed in to change notification settings - Fork 2.4k
feat: Add comprehensive Prometheus metrics support #6202
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: main
Are you sure you want to change the base?
Conversation
Implements optional Prometheus metrics collection with secure endpoint for monitoring and observability. Features: - Disabled by default, enabled via ENABLE_METRICS environment variable - Secure token-based authentication with Argon2 hashing support - Comprehensive metrics collection across all system components - Conditional compilation with enable_metrics feature flag - HTTP request instrumentation with automatic path normalization - Database connection pool and query performance monitoring - Authentication attempt tracking and session management - Business metrics for users, organizations, and vault items - System uptime and build information tracking Security: - Token authentication required (METRICS_TOKEN configuration) - Support for both plain text and Argon2 hashed tokens - Path normalization prevents high cardinality metric explosion - No-op implementations when metrics disabled for zero overhead - Network access controls recommended for production deployment Implementation: - Added prometheus dependency with conditional compilation - Created secure /metrics endpoint with request guard authentication - Implemented HTTP middleware fairing for automatic instrumentation - Added database metrics utilities with timing macros - Comprehensive unit and integration test coverage - Complete documentation with Prometheus, Grafana, and alerting examples Files added: - src/metrics.rs - Core metrics collection module - src/api/metrics.rs - Secure metrics endpoint implementation - src/api/middleware.rs - HTTP request instrumentation - src/db/metrics.rs - Database timing utilities - METRICS.md - Configuration and usage guide - MONITORING.md - Complete monitoring setup documentation - examples/metrics-config.env - Configuration examples - scripts/test-metrics.sh - Automated testing script - Comprehensive test suites for both enabled/disabled scenarios This implementation follows security best practices with disabled-by-default configuration and provides production-ready monitoring capabilities for Vaultwarden deployments.
- Fix Rocket response type issues in metrics endpoint - Correct middleware request timer handling - Resolve borrow checker issues with UUID string conversion - Add proper EXTRA_FEATURES support to Dockerfile for optional features - Suppress unused code warnings in proof-of-concept metrics implementation - Update Cargo.lock with resolved dependencies These fixes enable successful compilation of the metrics feature and production-ready Docker image building with: docker build --build-arg EXTRA_FEATURES="enable_metrics" The metrics-enabled image has been successfully built and tested.
|
You sort of beat me to it. I was thinking of adding OTel: also metrics, but mostly traces. Even though the vw service is not highly distributed (maybe a db backend on a different machine), traces are still useful just to verify where exactly the time is spent. Also helps a lot with debugging. I despise logs unless they are audit logs. ;-) Great job adding the Prometheus metrics! |
fix(config): also correct typo in config.rs
…e/vaultwarden into feature/prometheus-metrics
The DbConn type was missing an import for the non-feature version of update_business_metrics function, causing compilation failure during Docker build.
|
Is there any chance to see this comming ? |
Doesn't look like it. If there's no sign of interest from the maintainer within a reasonable time, I'll close the PR. |
|
Maybe some patients would be nice. The maintainers do have a life outside of this project. And they maintain this in there spare free time. I also do not see closing it will help anything. And keeping it open should be that hard either. |
|
@BlackDex - i appreciate that maintainers have lives outside their project etc. Contributors do too. I am both to various projects. I was not trying to be impatient or rude. It's just the way I work. There is another side to the maintainer/contributor coin that people often don't consider. Sometimes a contributor has many, many PRs outstanding against many, many third-party projects. Some are against actively maintained projects, others not so actively maintained. I'm getting too old now to be maintaining an ever-increasing backlog of PRs against other people's projects in my spare time that just aren't likely to get merged, so every couple of weekends or so I check up on the older PRs, maybe chase one or two. If it's still outstanding, and none of the maintainers hasn't acknowledged, commented on, reviewed or even given it a 'thumbs up' emoji or any kind, I think it's safe to assume they don't consider whatever I'm contributing to be very important to them. So it gets closed. If I do end up closing this PR, it's not a problem. I'm more than happy if others want to fork my fork and create their own PR to persist it, and maybe they'll have better luck pushing the maintainers on it. The contribution is still there for anyone who wants it, I just ain't sitting on PRs forever as they go stale. In this case, I knew before I did the work that it was unlikely that this would get merged. Because, before I did it I looked for prior work and found this... You said...
So, really, I just pushed this as a PR on the off chance that minds might have changed, or that this patch was a little more acceptable than mpatterson's. And to give it a public airing, so that others that do want this feature can have it. I haven't closed it yet. |
|
I believe this PR has a better chance to get merged than the one you mentioned, since it provides token access and thus does not make the access to the endpoint available to the public. IMO securing a |
|
Indeed, usually prometheus metrics are secured by firewalls / proxies so that's pretty standard. I still hope this gets into vaultwarden, it's a highly useful feature. Whenever maintainers have time, no hurry. |
Summary
This PR adds comprehensive Prometheus metrics support to Vaultwarden, providing observability into application performance, database health, authentication patterns, and business metrics.
Key Features
/metrics(disabled by default)--features enable_metricsMetrics Categories
Security Considerations
ENABLE_METRICS=trueDocumentation
METRICS.mdMONITORING.mdTest Plan
Breaking Changes
None - this is an optional feature that's disabled by default.
Files Added/Modified
src/api/metrics.rs- Metrics endpoint implementationMETRICS.md- Configuration and usage guideMONITORING.md- Complete monitoring setup guideexamples/metrics-config.env- Configuration examplesscripts/test-metrics.sh- Testing scriptCargo.toml- Added optional prometheus dependencyREADME.md- Added metrics feature documentationdocker/Dockerfile.debian- Support for extra features in build