-
-
Notifications
You must be signed in to change notification settings - Fork 21
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
feat: Prometheus Endpoint #123
Conversation
…rVec as well os default Go metrics listening on :2112
WalkthroughThe recent update integrates Prometheus metrics into the system's realtime processing capabilities. This includes initializing and registering metrics, specifically a counter for BirdNET detections by common name, and setting up an HTTP endpoint for metrics exposure. Additionally, the processor's method for handling pending detections now updates these metrics based on specific conditions, and configuration settings have been adjusted to support these changes. Changes
Poem
Recent Review DetailsConfiguration used: CodeRabbit UI Files ignored due to path filters (2)
Files selected for processing (4)
Additional comments not posted (3)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
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.
Actionable comments posted: 1
if settings.Realtime.Prometheus { | ||
settings.Realtime.PrometheusDetectionCounter = prometheus.NewCounterVec( | ||
prometheus.CounterOpts{ | ||
Name: "birdnet_detections", | ||
Help: "How many BirdNET detections partitioned by common name.", | ||
}, | ||
[]string{"name"}, | ||
) | ||
prometheus.MustRegister(settings.Realtime.PrometheusDetectionCounter) | ||
http.Handle("/metrics", promhttp.Handler()) | ||
go http.ListenAndServe(":2112", nil) | ||
} |
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.
Initialization of Prometheus metrics and setup of HTTP endpoint are implemented. Consider handling errors from http.ListenAndServe
and making the port configurable.
- go http.ListenAndServe(":2112", nil)
+ go func() {
+ err := http.ListenAndServe(settings.Realtime.MetricsPort, nil)
+ if err != nil {
+ log.Printf("Failed to start metrics server: %v", err)
+ }
+ }()
Add a MetricsPort
field to the Realtime
struct in the configuration to allow port configuration.
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
if settings.Realtime.Prometheus { | |
settings.Realtime.PrometheusDetectionCounter = prometheus.NewCounterVec( | |
prometheus.CounterOpts{ | |
Name: "birdnet_detections", | |
Help: "How many BirdNET detections partitioned by common name.", | |
}, | |
[]string{"name"}, | |
) | |
prometheus.MustRegister(settings.Realtime.PrometheusDetectionCounter) | |
http.Handle("/metrics", promhttp.Handler()) | |
go http.ListenAndServe(":2112", nil) | |
} | |
if settings.Realtime.Prometheus { | |
settings.Realtime.PrometheusDetectionCounter = prometheus.NewCounterVec( | |
prometheus.CounterOpts{ | |
Name: "birdnet_detections", | |
Help: "How many BirdNET detections partitioned by common name.", | |
}, | |
[]string{"name"}, | |
) | |
prometheus.MustRegister(settings.Realtime.PrometheusDetectionCounter) | |
http.Handle("/metrics", promhttp.Handler()) | |
go func() { | |
err := http.ListenAndServe(settings.Realtime.MetricsPort, nil) | |
if err != nil { | |
log.Printf("Failed to start metrics server: %v", err) | |
} | |
}() | |
} |
Thank you, merged. |
This is very nice! I would like to add some more metrics in the future for observability of the health of the application etc 👍 Just one question, would it be better to offer the /metrics view at the same port used for serving the webpage itself at? In this case 8080. That would make the port forwarding a bit easier. However, I suppose one benefit of having it on a separate port is that it allows more control of who should have access to the metrics? |
I have been thinking of adding Go telemetry for some time , so this is more than welcome. I'd prefer to keep telemetry endpoint separate from main application, for example if people wish to publish their dashboard to Internet it would be at least some level security issue if application telemetry would be exposed as well. |
if settings.Realtime.Prometheus { | ||
settings.Realtime.PrometheusDetectionCounter = prometheus.NewCounterVec( | ||
prometheus.CounterOpts{ | ||
Name: "birdnet_detections", |
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.
As this is a counter it probably should have ended with _total
or _count
. You'll probably see warnings like this when using PromQL.
PromQL info: metric might not be a counter, name does not end in _total/_sum/_count/_bucket
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.
Hey nice catch! I don't know anything about PromQL but I see no reason why this small change shouldn't be applied.
I think my (selfish) goal of this PR was simply to introduce an option to use existing visualization systems instead of building yet another HTML dashboard ( not saying there's anything wrong with the existing one <3 ).
Feel free to make this change if @tphakala's on board.
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.
Thanks - this is just a minor nit. I am using the metric and it works get btw 😄
I understand there is interest in keeping birdnet-go as lightweight as possible so instead of building out a full-blown Web UI, perhaps an alternative option could be to export metrics to Prometheus and visualized with existing tools like Grafana.
This PR include a bool configuration option to enable a very basic Prometheus CounterVec as well as all of the boilerplate Go performance metrics.
To enable the Prometheus endpoint, simply include the following in your configuration file.
The current implementation is very simple, the following is sample output from development / testing :
This could easily be improved upon with :
Summary by CodeRabbit