-
Notifications
You must be signed in to change notification settings - Fork 3
feat(prometheus): Add duration metrics for SPOA message processing #135
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
- Add MessageDuration histogram metric to track processing time for crowdsec-http and crowdsec-ip messages - Instrument handlerWrapper to measure duration of each message type - Use prometheus.NewTimer() with WithLabelValues() to avoid allocations in hot path - Register new metric in Prometheus registry
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 PR adds Prometheus histogram metrics to track the processing duration of SPOA message types in the HAProxy SPOA bouncer. The implementation instruments the message handler wrapper to measure how long it takes to process "crowdsec-http" and "crowdsec-ip" messages, providing valuable observability into request processing performance.
Key Changes:
- Adds
MessageDurationhistogram metric with exponential buckets (0.1ms to ~3.3s range) to track SPOA message processing time - Instruments
handlerWrapperto create timers for each message type usingprometheus.NewTimer() - Registers the new metric in the Prometheus registry alongside existing metrics
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| pkg/metrics/metrics.go | Defines new MessageDuration histogram metric with "message_type" label and exponential buckets |
| pkg/spoa/root.go | Adds timer creation and duration observation in the message handler wrapper |
| cmd/root.go | Registers the new MessageDuration metric with Prometheus |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Update histogram buckets for better granularity around 500ms timeout threshold - Use defer for timer observation to ensure durations are recorded even on errors/panics - Address Copilot review feedback
- Remove defer and closure to eliminate allocations in hot path - Call ObserveDuration() directly after handler execution - Passes golangci-lint with zero allocations
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
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Remove message-level duration metrics (not granular enough) - Add IPCheckDuration metric with lookup_type label (ip_or_range, country) - Add CaptchaValidationDuration metric for captcha operations - Add GeoLookupDuration metric for geo database lookups - Instrument getIPRemediation to track IP/country checks separately - Instrument captcha validation in handleCaptchaRemediation - All metrics use 10 buckets optimized for 0-500ms timeout threshold - Zero allocations in hot path using prometheus.NewTimer()
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
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Move IP/range lookup timing into dataset.CheckIP for better encapsulation - Distinguish between 'ip' and 'range' lookup types after dataset redesign - Update IPCheckDuration buckets to start at 100μs for microsecond-level granularity - IP/range lookups are typically ~100μs, buckets now cover 100μs to 1s (10 buckets) - Update all test files to match new CheckIP signature - Zero allocations in hot path using prometheus.NewTimer()
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
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Uh oh!
There was an error while loading. Please reload this page.