NETOBSERV-1284: implement metrics white-listing #447
Conversation
|
@jotak: This pull request references NETOBSERV-1284 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.15.0" version, but no target version was set. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
| // `node_ingress_bytes_total`, `node_ingress_packets_total`, `node_flows_total`, `workload_egress_bytes_total`, | ||
| // `workload_egress_packets_total`, `workload_ingress_bytes_total`, `workload_ingress_packets_total`, `workload_flows_total`. | ||
| // +optional | ||
| IncludeList *[]string `json:"includeList,omitempty"` |
There was a problem hiding this comment.
cc @OlivierCazade can you tell me again why we didn't use pointers on slices? That would be an easy way to tell: this setting was/wasn't explicitly configured.
Here, I'm not providing any default in CRD, but defaults are provided in code
There was a problem hiding this comment.
Using pointer in CRD to enable/disable a feature does not work, if at some point the pointer is not nil, it is not possible to set the pointer to nil again, applying an yaml without the slice section does not modify it.
There was a problem hiding this comment.
hmm I just tired that and it worked.
ie. I've set a includeList => saved => checked results, then removed includeList => saved => check results , this worked as intended
pkg/metrics/predefined_metrics.go
Outdated
| "reflect" | ||
| "strings" | ||
|
|
||
| "github.com/netobserv/flowlogs-pipeline/pkg/api" |
There was a problem hiding this comment.
Nitpicking: may be rename the import flpapi to avoid confusion between operator api and flp api
|
@jotak: This pull request references NETOBSERV-1284 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.15.0" version, but no target version was set. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
/ok-to-test |
openshift-ci
bot
added
the
ok-to-test
|
@jotak: This pull request references NETOBSERV-1284 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.15.0" version, but no target version was set. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
New images:
They will expire after two weeks. To deploy this build: # Direct deployment, from operator repo
IMAGE=quay.io/netobserv/network-observability-operator:82bafc6 make deploy
# Or using operator-sdk
operator-sdk run bundle quay.io/netobserv/network-observability-operator-bundle:v0.0.0-82bafc6Or as a Catalog Source: apiVersion: operators.coreos.com/v1alpha1
kind: CatalogSource
metadata:
name: netobserv-dev
namespace: openshift-marketplace
spec:
sourceType: grpc
image: quay.io/netobserv/network-observability-operator-catalog:v0.0.0-82bafc6
displayName: NetObserv development catalog
publisher: Me
updateStrategy:
registryPoll:
interval: 1m |
I don't have a problem to wait for v1beta2 first, but still, I think this change should also be done on beta1 too, so that users can ACK deprecation and modify their config without having the hassle to migrate entirely to beta2. It's smoother for them to have it in both places |
|
@jotak: This pull request references NETOBSERV-1284 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.15.0" version, but it targets "netobserv-1.5" instead. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
@jotak: This pull request references NETOBSERV-1284 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.15.0" version, but it targets "netobserv-1.5" instead. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
@jotak: This pull request references NETOBSERV-1284 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.15.0" version, but it targets "netobserv-1.5" instead. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
github-actions
bot
removed
the
ok-to-test
|
@jotak: This pull request references NETOBSERV-1284 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.15.0" version, but it targets "netobserv-1.5" instead. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #447 +/- ##
==========================================
+ Coverage 54.90% 61.80% +6.90%
==========================================
Files 49 52 +3
Lines 6453 6530 +77
==========================================
+ Hits 3543 4036 +493
+ Misses 2666 2194 -472
- Partials 244 300 +56
Flags with carried forward coverage won't be shown. Click here to find out more.
☔ View full report in Codecov by Sentry. |
|
/ok-to-test |
openshift-ci
bot
added
the
ok-to-test
|
@jotak: This pull request references NETOBSERV-1284 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.15.0" version, but it targets "netobserv-1.5" instead. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
/hold for premerge testing |
jotak
force-pushed
the
metrics-whitelist
branch
from
c1ea9ed
to
c80d9a7
Compare
jotak
added
the
ok-to-test
|
New images:
They will expire after two weeks. To deploy this build: # Direct deployment, from operator repo
IMAGE=quay.io/netobserv/network-observability-operator:4859b57 make deploy
# Or using operator-sdk
operator-sdk run bundle quay.io/netobserv/network-observability-operator-bundle:v0.0.0-4859b57Or as a Catalog Source: apiVersion: operators.coreos.com/v1alpha1
kind: CatalogSource
metadata:
name: netobserv-dev
namespace: openshift-marketplace
spec:
sourceType: grpc
image: quay.io/netobserv/network-observability-operator-catalog:v0.0.0-4859b57
displayName: NetObserv development catalog
publisher: Me
updateStrategy:
registryPoll:
interval: 1m |
|
@jotak can we rebase so we can premerge test? |
jotak
force-pushed
the
metrics-whitelist
branch
from
c80d9a7
to
a581ce4
Compare
github-actions
bot
removed
the
ok-to-test
|
@nathan-weinberg rebased |
- Introduce CRD field processor.metrics.includeList - Deprecate CRD field processor.metrics.ignoreTags - Convert ignoreTags to includeList approach when includeList isn't set - If includeList isn't set and ignoreTags == default tags in 1.4, default includeList will be used. This should allow a smooth transitioning if more metrics are added in 1.5 - Some code refactoring to move away from embedded metrics defs - this will help to prepare exposing the metrics creation API, and avoid having to define every metric permutation one by one (egress/ingress, bytes/packets, node/ns/workload) - Fixing an issue with the Health dashboard not showing some metrics (previously tagged as "flows") despite they are always present disambiguate package name Fix include inner direction in metrics Rebased / adapt to v1beta2 - In Conversion webhooks, use the per-field dedicated functions to integrate conversion logic - Add tests on conversion webhooks - Automate generation of hack CRD - Modify Health dashboard tagging: just a single tag "dynamic" is sufficient to tell whether we need to check for metric included
jotak
force-pushed
the
metrics-whitelist
branch
from
a581ce4
to
3c065b9
Compare
jotak
added
the
ok-to-test
|
New images:
They will expire after two weeks. To deploy this build: # Direct deployment, from operator repo
IMAGE=quay.io/netobserv/network-observability-operator:d1850d7 make deploy
# Or using operator-sdk
operator-sdk run bundle quay.io/netobserv/network-observability-operator-bundle:v0.0.0-d1850d7Or as a Catalog Source: apiVersion: operators.coreos.com/v1alpha1
kind: CatalogSource
metadata:
name: netobserv-dev
namespace: openshift-marketplace
spec:
sourceType: grpc
image: quay.io/netobserv/network-observability-operator-catalog:v0.0.0-d1850d7
displayName: NetObserv development catalog
publisher: Me
updateStrategy:
registryPoll:
interval: 1m |
|
/unhold |
|
/label qe-approved |
|
@jotak: This pull request references NETOBSERV-1284 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target either version "4.15." or "openshift-4.15.", but it targets "netobserv-1.5" instead. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
thanks @nathan-weinberg ! |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jotak The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Description
Dependencies
FLP: netobserv/flowlogs-pipeline#478
Checklist
If you are not familiar with our processes or don't know what to answer in the list below, let us know in a comment: the maintainers will take care of that.