-
Notifications
You must be signed in to change notification settings - Fork 24
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
NETOBSERV-497 Use direct-prom pipeline #153
Conversation
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/hold |
3576557
to
e3c6de9
Compare
/ok-to-test |
New image: ["quay.io/netobserv/network-observability-operator:677a609"]. It will expire after two weeks. |
encode: | ||
type: prom | ||
prom: | ||
metrics: | ||
- name: bandwidth_per_network_service_per_namespace | ||
type: counter | ||
filter: {key: name, value: bandwidth_network_service_namespace} | ||
valuekey: recent_op_value | ||
valuekey: Bytes | ||
labels: | ||
- Service |
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.
If we no longer do the summation in the extract_aggregate, where is the sum performed? In prometheus counter? So we have to call Prometheus.Add() on each record instead of on the aggregates. OK.
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.
exactly, it's built-in prom client
|
||
// obtain extract_aggregate and encode_prometheus stages from metrics_definitions | ||
aggregates, promMetrics, err := b.obtainMetricsConfiguration() | ||
promMetrics, err := b.obtainMetricsConfiguration() | ||
if err != nil { | ||
return err |
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.
Fix comment that we add only encode_prometheus stage but not extract_aggregate.
@jotak @jpinsonneau @OlivierCazade We have these metrics now in prometheus. How do we expose them to a user? Do we send them to our own prometheus instance or to the existing Openshift prometheus? Then, how do we show the graphs? |
It depends on the operator flavour, if it's upstream or downstream. Upstream, the user needs to install prometheus (or configure cluster monitoring to scrape user workload targets). It's not documented in the operator yet because it's still kinda WIP (cf epic https://issues.redhat.com/browse/NETOBSERV-391 ), but there's some documentation there: https://github.com/netobserv/documents/blob/main/openshift.md#metrics In the productized/downstream version, metrics will be available in cluster monitoring, hence visible from the openshift console. In both case that's not something we're actively working on right now as it's not in 4.12 roadmap, it will certainly be in 4.13. But that doesn't mean we can't make progress on it if we have time for it. And at last, we will create new dashboards in our console plugin (epic https://issues.redhat.com/browse/NETOBSERV-139 ) |
- Remove aggregation stage - Update FLP / vendor
e3c6de9
to
1cc6a63
Compare
New changes are detected. LGTM label has been removed. |
1cc6a63
to
d498034
Compare
Requires netobserv/flowlogs-pipeline#266