-
Notifications
You must be signed in to change notification settings - Fork 11
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
RFC for gateway api metrics exporter #101
Conversation
cc @maleck13 - RFC related to what we chatted about earlier |
Existing [example dashboards](https://github.com/Kuadrant/gateway-api-state-metrics/tree/main/src/dashboards) in the gateway-api-state-metrics project will be copied over to the exporter project and continue to work as before. | ||
However, initially it will just be the Gateway, GatewayClass and HTTPRoute dashboards as those will be the metrics that are implemented first. | ||
|
||
# Reference-level explanation |
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.
So why are we not doing this work directly in gateway api state metrics. I am missing this piece . It seems we are starting with a new library?
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.
I see it is covered below
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.
individual listener and httproute status metrics seems highly valuable. Shame it is considered out of reach to add to the existing implementation.
@jasonmadigan @R-Lawton |
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.
few small comments
Signed-off-by: David Martin <[email protected]>
fc69e09
to
f58087d
Compare
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.
LGTM
👀 |
Second, implement new metrics, as per the examples below, to capture the additional status information: | ||
|
||
```promql | ||
gatewayapi_gateway_status_listeners_conditions{namespace="<NAMESPACE>",name="<GATEWAY>",listener_name="<LISTENER_NAME>",type="<ResolvedRefs|Ready|Other>} 1 |
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.
Wdyt about using a gateway API acronym so something like gwapi for the metric name? im thinking long run having to type these out might get tedious.
gatewayapi_gateway_status_listeners_conditions{namespace="<NAMESPACE>",name="<GATEWAY>",listener_name="<LISTENER_NAME>",type="<ResolvedRefs|Ready|Other>} 1 | |
gwapi_gateway_status_listeners_conditions{namespace="<NAMESPACE>",name="<GATEWAY>",listener_name="<LISTENER_NAME>",type="<ResolvedRefs|Ready|Other>} 1 |
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.
I do like the shorter version.
My concern with changing the name of the metrics is that they lose backwards compatibility with gateway-api-state-metrics, and any example alerts and dashboards provided as part of that. Same for the kuadrant example alerts and dashboards.
We would need to allow for migration of versions.
Do you think it's worth the change?
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.
yes good point, i think it would be nice to have but the efforts to change it arent worth it
Thanks @maleck13 @jasonmadigan @R-Lawton |
Related to Kuadrant/kuadrant-operator#675 and Kuadrant/gateway-api-state-metrics#1