Skip to content

Commit

Permalink
NETOBSERV-1284 implement metrics white-listing (#447)
Browse files Browse the repository at this point in the history
- 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
  • Loading branch information
jotak authored Nov 8, 2023
1 parent 6c1b0b7 commit 74a4814
Show file tree
Hide file tree
Showing 45 changed files with 780 additions and 752 deletions.
16 changes: 16 additions & 0 deletions api/v1alpha1/flowcollector_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"github.com/netobserv/network-observability-operator/api/v1beta2"
utilconversion "github.com/netobserv/network-observability-operator/pkg/conversion"
"github.com/netobserv/network-observability-operator/pkg/helper"
"github.com/netobserv/network-observability-operator/pkg/metrics"
v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
apiconversion "k8s.io/apimachinery/pkg/conversion"
"sigs.k8s.io/controller-runtime/pkg/conversion"
Expand Down Expand Up @@ -73,6 +74,12 @@ func (r *FlowCollector) ConvertTo(dstRaw conversion.Hub) error {
// Loki
dst.Spec.Loki.Enable = restored.Spec.Loki.Enable

if restored.Spec.Processor.Metrics.IncludeList != nil {
list := make([]string, len(*restored.Spec.Processor.Metrics.IncludeList))
copy(list, *restored.Spec.Processor.Metrics.IncludeList)
dst.Spec.Processor.Metrics.IncludeList = &list
}

return nil
}

Expand Down Expand Up @@ -172,3 +179,12 @@ func Convert_v1beta2_FlowCollectorEBPF_To_v1alpha1_FlowCollectorEBPF(in *v1beta2
func Convert_v1beta2_ServerTLS_To_v1alpha1_ServerTLS(in *v1beta2.ServerTLS, out *ServerTLS, s apiconversion.Scope) error {
return autoConvert_v1beta2_ServerTLS_To_v1alpha1_ServerTLS(in, out, s)
}

// This function need to be manually created because conversion-gen not able to create it intentionally because
// we have new defined fields in v1beta2 not in v1beta1
// nolint:golint,stylecheck,revive
func Convert_v1alpha1_FLPMetrics_To_v1beta2_FLPMetrics(in *FLPMetrics, out *v1beta2.FLPMetrics, s apiconversion.Scope) error {
includeList := metrics.GetEnabledNames(in.IgnoreTags, nil)
out.IncludeList = &includeList
return autoConvert_v1alpha1_FLPMetrics_To_v1beta2_FLPMetrics(in, out, s)
}
19 changes: 7 additions & 12 deletions api/v1alpha1/zz_generated.conversion.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

17 changes: 14 additions & 3 deletions api/v1beta1/flowcollector_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -344,13 +344,24 @@ type FLPMetrics struct {
// +optional
Server MetricsServerConfig `json:"server,omitempty"`

// `ignoreTags` is a list of tags to specify which metrics to ignore. Each metric is associated with a list of tags. More details in https://github.com/netobserv/network-observability-operator/tree/main/controllers/flowlogspipeline/metrics_definitions .
// `ignoreTags` [deprecated (*)] is a list of tags to specify which metrics to ignore. Each metric is associated with a list of tags. More details in https://github.com/netobserv/network-observability-operator/tree/main/controllers/flowlogspipeline/metrics_definitions .
// Available tags are: `egress`, `ingress`, `flows`, `bytes`, `packets`, `namespaces`, `nodes`, `workloads`, `nodes-flows`, `namespaces-flows`, `workloads-flows`.
// Namespace-based metrics are covered by both `workloads` and `namespaces` tags, hence it is recommended to always ignore one of them (`workloads` offering a finer granularity).
//+kubebuilder:default:={"egress","packets","nodes-flows","namespaces-flows","workloads-flows","namespaces"}
// Namespace-based metrics are covered by both `workloads` and `namespaces` tags, hence it is recommended to always ignore one of them (`workloads` offering a finer granularity).<br>
// Deprecation notice: use `includeList` instead.
// +kubebuilder:default:={"egress","packets","nodes-flows","namespaces-flows","workloads-flows","namespaces"}
// +optional
IgnoreTags []string `json:"ignoreTags"`

// `includeList` is a list of metric names to specify which metrics to generate.
// The names correspond to the name in Prometheus, without the prefix. For example,
// `namespace_egress_packets_total` will show up as `netobserv_namespace_egress_packets_total` in Prometheus.
// Available names are: `namespace_egress_bytes_total`, `namespace_egress_packets_total`, `namespace_ingress_bytes_total`,
// `namespace_ingress_packets_total`, `namespace_flows_total`, `node_egress_bytes_total`, `node_egress_packets_total`,
// `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"`

// `disableAlerts` is a list of alerts that should be disabled.
// Possible values are:<br>
// `NetObservNoFlows`, which is triggered when no flows are being observed for a certain period.<br>
Expand Down
28 changes: 21 additions & 7 deletions api/v1beta1/flowcollector_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"github.com/netobserv/network-observability-operator/api/v1beta2"
utilconversion "github.com/netobserv/network-observability-operator/pkg/conversion"
"github.com/netobserv/network-observability-operator/pkg/helper"
"github.com/netobserv/network-observability-operator/pkg/metrics"
v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
apiconversion "k8s.io/apimachinery/pkg/conversion"
"sigs.k8s.io/controller-runtime/pkg/conversion"
Expand Down Expand Up @@ -79,13 +80,6 @@ func Convert_v1beta2_FlowCollectorFLP_To_v1beta1_FlowCollectorFLP(in *v1beta2.Fl
return autoConvert_v1beta2_FlowCollectorFLP_To_v1beta1_FlowCollectorFLP(in, out, s)
}

// This function need to be manually created because conversion-gen not able to create it intentionally because
// we have new defined fields in v1beta2 not in v1beta1
// nolint:golint,stylecheck,revive
func Convert_v1beta2_FLPMetrics_To_v1beta1_FLPMetrics(in *v1beta2.FLPMetrics, out *FLPMetrics, s apiconversion.Scope) error {
return autoConvert_v1beta2_FLPMetrics_To_v1beta1_FLPMetrics(in, out, s)
}

// This function need to be manually created because conversion-gen not able to create it intentionally because
// we have new defined fields in v1beta2 not in v1beta1
// nolint:golint,stylecheck,revive
Expand Down Expand Up @@ -129,3 +123,23 @@ func Convert_v1beta1_FlowCollectorLoki_To_v1beta2_FlowCollectorLoki(in *FlowColl
}
return autoConvert_v1beta1_FlowCollectorLoki_To_v1beta2_FlowCollectorLoki(in, out, s)
}

// This function need to be manually created because conversion-gen not able to create it intentionally because
// we have new defined fields in v1beta2 not in v1beta1
// nolint:golint,stylecheck,revive
func Convert_v1beta2_FLPMetrics_To_v1beta1_FLPMetrics(in *v1beta2.FLPMetrics, out *FLPMetrics, s apiconversion.Scope) error {
return autoConvert_v1beta2_FLPMetrics_To_v1beta1_FLPMetrics(in, out, s)
}

// This function need to be manually created because conversion-gen not able to create it intentionally because
// we have new defined fields in v1beta2 not in v1beta1
// nolint:golint,stylecheck,revive
func Convert_v1beta1_FLPMetrics_To_v1beta2_FLPMetrics(in *FLPMetrics, out *v1beta2.FLPMetrics, s apiconversion.Scope) error {
err := autoConvert_v1beta1_FLPMetrics_To_v1beta2_FLPMetrics(in, out, s)
if err != nil {
return err
}
includeList := metrics.GetEnabledNames(in.IgnoreTags, in.IncludeList)
out.IncludeList = &includeList
return nil
}
160 changes: 160 additions & 0 deletions api/v1beta1/flowcollector_webhook_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,160 @@
package v1beta1

import (
"testing"

"github.com/netobserv/network-observability-operator/api/v1beta2"
"github.com/stretchr/testify/assert"
"k8s.io/utils/ptr"
)

func TestBeta1ConversionRoundtrip_Loki(t *testing.T) {
// Testing beta1 -> beta2 -> beta1
assert := assert.New(t)

initial := FlowCollector{
Spec: FlowCollectorSpec{
Loki: FlowCollectorLoki{
Enable: ptr.To(true),
URL: "http://loki",
StatusURL: "http://loki/status",
QuerierURL: "http://loki/querier",
TenantID: "tenant",
AuthToken: LokiAuthForwardUserToken,
TLS: ClientTLS{
Enable: true,
InsecureSkipVerify: true,
},
StatusTLS: ClientTLS{
Enable: true,
InsecureSkipVerify: true,
},
BatchSize: 1000,
},
},
}

var converted v1beta2.FlowCollector
err := initial.ConvertTo(&converted)
assert.NoError(err)

assert.Equal(v1beta2.LokiModeManual, converted.Spec.Loki.Mode)
assert.True(*converted.Spec.Loki.Enable)
assert.Equal("http://loki", converted.Spec.Loki.Manual.IngesterURL)
assert.Equal("http://loki/status", converted.Spec.Loki.Manual.StatusURL)
assert.Equal("http://loki/querier", converted.Spec.Loki.Manual.QuerierURL)
assert.Equal("tenant", converted.Spec.Loki.Manual.TenantID)
assert.Equal(LokiAuthForwardUserToken, converted.Spec.Loki.Manual.AuthToken)
assert.True(converted.Spec.Loki.Manual.TLS.Enable)
assert.True(converted.Spec.Loki.Manual.TLS.InsecureSkipVerify)
assert.True(converted.Spec.Loki.Manual.StatusTLS.Enable)
assert.True(converted.Spec.Loki.Manual.StatusTLS.InsecureSkipVerify)

// Other way
var back FlowCollector
err = back.ConvertFrom(&converted)
assert.NoError(err)
assert.Equal(initial.Spec.Loki, back.Spec.Loki)
}

func TestBeta2ConversionRoundtrip_Loki(t *testing.T) {
// Testing beta2 -> beta1 -> beta2
assert := assert.New(t)

initial := v1beta2.FlowCollector{
Spec: v1beta2.FlowCollectorSpec{
Loki: v1beta2.FlowCollectorLoki{
Enable: ptr.To(true),
Mode: v1beta2.LokiModeLokiStack,
LokiStack: v1beta2.LokiStackRef{
Name: "lokiii",
Namespace: "lokins",
},
BatchSize: 1000,
},
},
}

var converted FlowCollector
err := converted.ConvertFrom(&initial)
assert.NoError(err)

assert.True(*converted.Spec.Loki.Enable)
assert.Equal("https://lokiii-gateway-http.lokins.svc:8080/api/logs/v1/network/", converted.Spec.Loki.URL)
assert.Equal("https://lokiii-query-frontend-http.lokins.svc:3100/", converted.Spec.Loki.StatusURL)
assert.Equal("https://lokiii-gateway-http.lokins.svc:8080/api/logs/v1/network/", converted.Spec.Loki.QuerierURL)
assert.Equal("network", converted.Spec.Loki.TenantID)
assert.Equal(LokiAuthForwardUserToken, converted.Spec.Loki.AuthToken)
assert.True(converted.Spec.Loki.TLS.Enable)
assert.False(converted.Spec.Loki.TLS.InsecureSkipVerify)
assert.True(converted.Spec.Loki.StatusTLS.Enable)
assert.False(converted.Spec.Loki.StatusTLS.InsecureSkipVerify)

// Other way
var back v1beta2.FlowCollector
err = converted.ConvertTo(&back)
assert.NoError(err)
assert.Equal(initial.Spec.Loki, back.Spec.Loki)
}

func TestBeta1ConversionRoundtrip_Metrics(t *testing.T) {
// Testing beta1 -> beta2 -> beta1
assert := assert.New(t)

initial := FlowCollector{
Spec: FlowCollectorSpec{
Processor: FlowCollectorFLP{
Metrics: FLPMetrics{
DisableAlerts: []FLPAlert{AlertLokiError},
IgnoreTags: []string{"nodes", "workloads", "bytes", "ingress"},
},
},
},
}

var converted v1beta2.FlowCollector
err := initial.ConvertTo(&converted)
assert.NoError(err)

assert.Equal([]v1beta2.FLPAlert{v1beta2.AlertLokiError}, converted.Spec.Processor.Metrics.DisableAlerts)
assert.NotNil(converted.Spec.Processor.Metrics.IncludeList)
assert.Equal([]string{"namespace_egress_packets_total", "namespace_flows_total"}, *converted.Spec.Processor.Metrics.IncludeList)

// Other way
var back FlowCollector
err = back.ConvertFrom(&converted)
assert.NoError(err)
// Here, includeList is preserved; it takes precedence over ignoreTags
assert.Equal([]string{"namespace_egress_packets_total", "namespace_flows_total"}, *back.Spec.Processor.Metrics.IncludeList)
assert.Equal(initial.Spec.Processor.Metrics.DisableAlerts, back.Spec.Processor.Metrics.DisableAlerts)
assert.Equal(initial.Spec.Processor.Metrics.Server, back.Spec.Processor.Metrics.Server)
}

func TestBeta2ConversionRoundtrip_Metrics(t *testing.T) {
// Testing beta2 -> beta1 -> beta2
assert := assert.New(t)

initial := v1beta2.FlowCollector{
Spec: v1beta2.FlowCollectorSpec{
Processor: v1beta2.FlowCollectorFLP{
Metrics: v1beta2.FLPMetrics{
DisableAlerts: []v1beta2.FLPAlert{v1beta2.AlertLokiError},
IncludeList: &[]string{"namespace_egress_packets_total", "namespace_flows_total"},
},
},
},
}

var converted FlowCollector
err := converted.ConvertFrom(&initial)
assert.NoError(err)

assert.Equal([]FLPAlert{AlertLokiError}, converted.Spec.Processor.Metrics.DisableAlerts)
assert.NotNil(converted.Spec.Processor.Metrics.IncludeList)
assert.Equal([]string{"namespace_egress_packets_total", "namespace_flows_total"}, *converted.Spec.Processor.Metrics.IncludeList)

var back v1beta2.FlowCollector
err = converted.ConvertTo(&back)
assert.NoError(err)
assert.Equal(initial.Spec.Processor.Metrics, back.Spec.Processor.Metrics)
}
20 changes: 8 additions & 12 deletions api/v1beta1/zz_generated.conversion.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

9 changes: 9 additions & 0 deletions api/v1beta1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit 74a4814

Please sign in to comment.