-
Notifications
You must be signed in to change notification settings - Fork 62
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
PHOENIX-6879 Prometheus endpoint implementation #119
base: master
Are you sure you want to change the base?
Conversation
Notes: - added a new servlet (/prometheus) for exposing Prometheus format metrics - metrics description can be enabled by "/prometheus?description=true" query string - metrics can be filtered by "/prometheus?qry=xyz" query string
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.
Some suggestions. You might also want to add a unit test or two.
HandlerList list = (HandlerList) handlers[0]; | ||
|
||
ServletContextHandler ctx = new ServletContextHandler(); | ||
ctx.setContextPath("/prometheus"); |
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.
Prometheus typically expects and endpoint path to be "/metrics".
for (AbstractMetric metrics : metricsRecord.metrics()) { | ||
if (metrics.type() == MetricType.COUNTER || metrics.type() == MetricType.GAUGE) { | ||
|
||
String key = toPrometheusName(metricsRecord.name(), metrics.name()); |
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.
Note that this method and its regexp could be called quite often: per metricRecord, per metric, per metric scrape interval (default scrape interval is 1 minute) could add to a bit of CPU time... if I am right, I suggest memoizing this function
Notes: - Added some unit tests for PrometheusHadoopServlet and PrometheusEndpointServerCustomizer - Changed PrometheusHadoopServlet.toPrometheusName() function's visibility to package-private
In this change:
"/prometheus?description=true"
query string"/prometheus?qry=xyz"
query string