From 7db230757489505ed03baf70f99b6b5dae2c89f9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Charles-Edouard=20Br=C3=A9t=C3=A9ch=C3=A9?= Date: Fri, 9 Dec 2022 10:49:45 +0100 Subject: [PATCH] fix: setup tracing and minor cleanup in tracing and metrics code (#5629) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: setup tracing and minor cleanup in tracing and metrics code Signed-off-by: Charles-Edouard Brétéché * fix Signed-off-by: Charles-Edouard Brétéché Signed-off-by: Charles-Edouard Brétéché --- cmd/internal/metrics.go | 11 ++++- cmd/internal/setup.go | 3 +- cmd/internal/tracing.go | 3 +- pkg/config/config.go | 2 + pkg/config/metricsconfig.go | 66 ++++++++++++------------------ pkg/config/rbac.go | 9 ---- pkg/config/{filter.go => types.go} | 31 ++++++++++++++ pkg/config/webhook.go | 21 ---------- pkg/metrics/metrics.go | 12 ++---- pkg/tracing/tracing.go | 12 ++---- pkg/webhooks/handlers/metrics.go | 5 ++- 11 files changed, 82 insertions(+), 93 deletions(-) delete mode 100644 pkg/config/rbac.go rename pkg/config/{filter.go => types.go} (53%) delete mode 100644 pkg/config/webhook.go diff --git a/cmd/internal/metrics.go b/cmd/internal/metrics.go index d26edb6706..cbfbf226e4 100644 --- a/cmd/internal/metrics.go +++ b/cmd/internal/metrics.go @@ -49,7 +49,16 @@ func SetupMetrics(ctx context.Context, logger logr.Logger, kubeClient kubernetes } if otel == "prometheus" { go func() { - if err := http.ListenAndServe(metricsAddr, metricsServerMux); err != nil { + server := &http.Server{ + Addr: metricsAddr, + Handler: metricsServerMux, + ReadTimeout: 30 * time.Second, + WriteTimeout: 30 * time.Second, + ReadHeaderTimeout: 30 * time.Second, + IdleTimeout: 5 * time.Minute, + ErrorLog: logging.StdLogger(logging.WithName("prometheus-server"), ""), + } + if err := server.ListenAndServe(); err != nil { logger.Error(err, "failed to enable metrics", "address", metricsAddr) } }() diff --git a/cmd/internal/setup.go b/cmd/internal/setup.go index d8b5b33ad1..27536287d5 100644 --- a/cmd/internal/setup.go +++ b/cmd/internal/setup.go @@ -24,5 +24,6 @@ func Setup() (context.Context, logr.Logger, metrics.MetricsConfigManager, contex client := CreateKubernetesClient(logger) ctx, sdownSignals := SetupSignals(logger) metricsManager, sdownMetrics := SetupMetrics(ctx, logger, client) - return ctx, logger, metricsManager, shutdown(logger.WithName("shutdown"), sdownMaxProcs, sdownMetrics, sdownSignals) + sdownTracing := SetupTracing(logger, client) + return ctx, logger, metricsManager, shutdown(logger.WithName("shutdown"), sdownMaxProcs, sdownMetrics, sdownTracing, sdownSignals) } diff --git a/cmd/internal/tracing.go b/cmd/internal/tracing.go index 11c37bbb5b..c99404090f 100644 --- a/cmd/internal/tracing.go +++ b/cmd/internal/tracing.go @@ -9,13 +9,12 @@ import ( "k8s.io/client-go/kubernetes" ) -func SetupTracing(logger logr.Logger, name string, kubeClient kubernetes.Interface) context.CancelFunc { +func SetupTracing(logger logr.Logger, kubeClient kubernetes.Interface) context.CancelFunc { logger = logger.WithName("tracing").WithValues("enabled", tracingEnabled, "address", tracingAddress, "port", tracingPort, "creds", tracingCreds) if tracingEnabled { logger.Info("setup tracing...") shutdown, err := tracing.NewTraceConfig( logger, - name, net.JoinHostPort(tracingAddress, tracingPort), tracingCreds, kubeClient, diff --git a/pkg/config/config.go b/pkg/config/config.go index 8a7f0c400e..883accfefd 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -56,6 +56,8 @@ const ( LivenessServicePath = "/health/liveness" // ReadinessServicePath is the path for check readness health ReadinessServicePath = "/health/readiness" + // MetricsPath is the path for exposing metrics + MetricsPath = "/metrics" ) var ( diff --git a/pkg/config/metricsconfig.go b/pkg/config/metricsconfig.go index ddeea43a1c..2d5681726c 100644 --- a/pkg/config/metricsconfig.go +++ b/pkg/config/metricsconfig.go @@ -2,7 +2,6 @@ package config import ( "context" - "encoding/json" "os" "time" @@ -29,17 +28,39 @@ type MetricsConfiguration interface { CheckNamespace(string) bool } -type namespacesConfig struct { - IncludeNamespaces []string `json:"include,omitempty"` - ExcludeNamespaces []string `json:"exclude,omitempty"` -} - // metricsConfig stores the config for metrics type metricsConfig struct { namespaces namespacesConfig metricsRefreshInterval time.Duration } +// NewDefaultMetricsConfiguration ... +func NewDefaultMetricsConfiguration() *metricsConfig { + return &metricsConfig{ + metricsRefreshInterval: 0, + namespaces: namespacesConfig{ + IncludeNamespaces: []string{}, + ExcludeNamespaces: []string{}, + }, + } +} + +// NewMetricsConfiguration ... +func NewMetricsConfiguration(client kubernetes.Interface) (MetricsConfiguration, error) { + configuration := NewDefaultMetricsConfiguration() + cmName := os.Getenv(metricsConfigEnvVar) + if cmName != "" { + if cm, err := client.CoreV1().ConfigMaps(kyvernoNamespace).Get(context.TODO(), cmName, metav1.GetOptions{}); err != nil { + if !errors.IsNotFound(err) { + return nil, err + } + } else { + configuration.load(cm) + } + } + return configuration, nil +} + // GetExcludeNamespaces returns the namespaces to ignore for metrics exposure func (mcd *metricsConfig) GetExcludeNamespaces() []string { return mcd.namespaces.ExcludeNamespaces @@ -102,36 +123,3 @@ func (cd *metricsConfig) load(cm *corev1.ConfigMap) { } } } - -// NewDefaultMetricsConfiguration ... -func NewDefaultMetricsConfiguration() *metricsConfig { - return &metricsConfig{ - metricsRefreshInterval: 0, - namespaces: namespacesConfig{ - IncludeNamespaces: []string{}, - ExcludeNamespaces: []string{}, - }, - } -} - -// NewMetricsConfiguration ... -func NewMetricsConfiguration(client kubernetes.Interface) (MetricsConfiguration, error) { - configuration := NewDefaultMetricsConfiguration() - cmName := os.Getenv(metricsConfigEnvVar) - if cmName != "" { - if cm, err := client.CoreV1().ConfigMaps(kyvernoNamespace).Get(context.TODO(), cmName, metav1.GetOptions{}); err != nil { - if !errors.IsNotFound(err) { - return nil, err - } - } else { - configuration.load(cm) - } - } - return configuration, nil -} - -func parseIncludeExcludeNamespacesFromNamespacesConfig(jsonStr string) (namespacesConfig, error) { - var namespacesConfigObject namespacesConfig - err := json.Unmarshal([]byte(jsonStr), &namespacesConfigObject) - return namespacesConfigObject, err -} diff --git a/pkg/config/rbac.go b/pkg/config/rbac.go deleted file mode 100644 index ab1582b699..0000000000 --- a/pkg/config/rbac.go +++ /dev/null @@ -1,9 +0,0 @@ -package config - -import ( - "strings" -) - -func parseRbac(list string) []string { - return strings.Split(list, ",") -} diff --git a/pkg/config/filter.go b/pkg/config/types.go similarity index 53% rename from pkg/config/filter.go rename to pkg/config/types.go index a3126cdc61..f9c1e6bc8d 100644 --- a/pkg/config/filter.go +++ b/pkg/config/types.go @@ -1,10 +1,41 @@ package config import ( + "encoding/json" "regexp" "strings" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) +type WebhookConfig struct { + NamespaceSelector *metav1.LabelSelector `json:"namespaceSelector,omitempty"` + ObjectSelector *metav1.LabelSelector `json:"objectSelector,omitempty"` +} + +func parseWebhooks(webhooks string) ([]WebhookConfig, error) { + webhookCfgs := make([]WebhookConfig, 0, 10) + if err := json.Unmarshal([]byte(webhooks), &webhookCfgs); err != nil { + return nil, err + } + return webhookCfgs, nil +} + +func parseRbac(list string) []string { + return strings.Split(list, ",") +} + +type namespacesConfig struct { + IncludeNamespaces []string `json:"include,omitempty"` + ExcludeNamespaces []string `json:"exclude,omitempty"` +} + +func parseIncludeExcludeNamespacesFromNamespacesConfig(jsonStr string) (namespacesConfig, error) { + var namespacesConfigObject namespacesConfig + err := json.Unmarshal([]byte(jsonStr), &namespacesConfigObject) + return namespacesConfigObject, err +} + type filter struct { Kind string // TODO: as we currently only support one GVK version, we use the kind only. But if we support multiple GVK, then GV need to be added Namespace string diff --git a/pkg/config/webhook.go b/pkg/config/webhook.go deleted file mode 100644 index 317ea17592..0000000000 --- a/pkg/config/webhook.go +++ /dev/null @@ -1,21 +0,0 @@ -package config - -import ( - "encoding/json" - - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" -) - -type WebhookConfig struct { - NamespaceSelector *metav1.LabelSelector `json:"namespaceSelector,omitempty" protobuf:"bytes,5,opt,name=namespaceSelector"` - ObjectSelector *metav1.LabelSelector `json:"objectSelector,omitempty" protobuf:"bytes,11,opt,name=objectSelector"` -} - -func parseWebhooks(webhooks string) ([]WebhookConfig, error) { - webhookCfgs := make([]WebhookConfig, 0, 10) - if err := json.Unmarshal([]byte(webhooks), &webhookCfgs); err != nil { - return nil, err - } - - return webhookCfgs, nil -} diff --git a/pkg/metrics/metrics.go b/pkg/metrics/metrics.go index ffc9df69b3..28beb87900 100644 --- a/pkg/metrics/metrics.go +++ b/pkg/metrics/metrics.go @@ -6,6 +6,7 @@ import ( "time" "github.com/go-logr/logr" + "github.com/kyverno/kyverno/pkg/config" kconfig "github.com/kyverno/kyverno/pkg/config" "github.com/kyverno/kyverno/pkg/utils/kube" "github.com/kyverno/kyverno/pkg/version" @@ -95,7 +96,6 @@ func NewOTLPGRPCConfig( log logr.Logger, ) (metric.MeterProvider, error) { options := []otlpmetricgrpc.Option{otlpmetricgrpc.WithEndpoint(endpoint)} - if certs != "" { // here the certificates are stored as configmaps transportCreds, err := kube.FetchCert(ctx, certs, kubeClient) @@ -107,7 +107,6 @@ func NewOTLPGRPCConfig( } else { options = append(options, otlpmetricgrpc.WithInsecure()) } - // create new exporter for exporting metrics exporter, err := otlpmetricgrpc.New(ctx, options...) if err != nil { @@ -118,7 +117,7 @@ func NewOTLPGRPCConfig( resource.Default(), resource.NewWithAttributes( semconv.SchemaURL, - semconv.ServiceNameKey.String("kyverno"), + semconv.ServiceNameKey.String(MeterName), semconv.ServiceVersionKey.String(version.BuildVersion), ), ) @@ -135,7 +134,6 @@ func NewOTLPGRPCConfig( sdkmetric.WithReader(reader), sdkmetric.WithResource(res), ) - return provider, nil } @@ -156,7 +154,6 @@ func NewPrometheusConfig( log.Error(err, "failed creating resource") return nil, nil, err } - exporter, err := prometheus.New( prometheus.WithoutUnits(), prometheus.WithoutTargetInfo(), @@ -165,15 +162,12 @@ func NewPrometheusConfig( log.Error(err, "failed to initialize prometheus exporter") return nil, nil, err } - provider := sdkmetric.NewMeterProvider( sdkmetric.WithReader(exporter), sdkmetric.WithResource(res), ) - metricsServerMux := http.NewServeMux() - metricsServerMux.Handle("/metrics", promhttp.Handler()) - + metricsServerMux.Handle(config.MetricsPath, promhttp.Handler()) return provider, metricsServerMux, nil } diff --git a/pkg/tracing/tracing.go b/pkg/tracing/tracing.go index 7cfaa82b60..84ede9c5f8 100644 --- a/pkg/tracing/tracing.go +++ b/pkg/tracing/tracing.go @@ -22,6 +22,7 @@ import ( ) const ( + TracerName = "kyverno" // engine attributes PolicyGroupKey = attribute.Key("kyverno.policy.group") PolicyVersionKey = attribute.Key("kyverno.policy.version") @@ -76,18 +77,15 @@ const ( ) // NewTraceConfig generates the initial tracing configuration with 'address' as the endpoint to connect to the Opentelemetry Collector -func NewTraceConfig(log logr.Logger, name, address, certs string, kubeClient kubernetes.Interface) (func(), error) { +func NewTraceConfig(log logr.Logger, address, certs string, kubeClient kubernetes.Interface) (func(), error) { ctx := context.Background() - var client otlptrace.Client - if certs != "" { // here the certificates are stored as configmaps transportCreds, err := kube.FetchCert(ctx, certs, kubeClient) if err != nil { log.Error(err, "Error fetching certificate from secret") } - client = otlptracegrpc.NewClient( otlptracegrpc.WithEndpoint(address), otlptracegrpc.WithTLSCredentials(transportCreds), @@ -98,19 +96,17 @@ func NewTraceConfig(log logr.Logger, name, address, certs string, kubeClient kub otlptracegrpc.WithInsecure(), ) } - // create New Exporter for exporting metrics traceExp, err := otlptrace.New(ctx, client) if err != nil { log.Error(err, "Failed to create the collector exporter") return nil, err } - res, err := resource.Merge( resource.Default(), resource.NewWithAttributes( semconv.SchemaURL, - semconv.ServiceNameKey.String("kyverno"), + semconv.ServiceNameKey.String(TracerName), semconv.ServiceVersionKey.String(version.BuildVersion), ), ) @@ -118,13 +114,11 @@ func NewTraceConfig(log logr.Logger, name, address, certs string, kubeClient kub log.Error(err, "failed creating resource") return nil, err } - // create controller and bind the exporter with it tp := sdktrace.NewTracerProvider( sdktrace.WithBatcher(traceExp), sdktrace.WithResource(res), ) - // set global propagator to tracecontext (the default is no-op). otel.SetTextMapPropagator(propagation.TraceContext{}) otel.SetTracerProvider(tp) diff --git a/pkg/webhooks/handlers/metrics.go b/pkg/webhooks/handlers/metrics.go index 2d373d0fb5..aff30cfdf1 100644 --- a/pkg/webhooks/handlers/metrics.go +++ b/pkg/webhooks/handlers/metrics.go @@ -7,6 +7,7 @@ import ( "github.com/go-logr/logr" "github.com/kyverno/kyverno/pkg/config" + "github.com/kyverno/kyverno/pkg/metrics" "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/metric/global" "go.opentelemetry.io/otel/metric/instrument" @@ -14,11 +15,11 @@ import ( ) func (inner AdmissionHandler) WithMetrics(logger logr.Logger, metricsConfig config.MetricsConfiguration, attrs ...attribute.KeyValue) AdmissionHandler { - return inner.withMetrics(logger, metricsConfig).WithTrace("METRICS") + return inner.withMetrics(logger, metricsConfig, attrs...).WithTrace("METRICS") } func (inner AdmissionHandler) withMetrics(logger logr.Logger, metricsConfig config.MetricsConfiguration, attrs ...attribute.KeyValue) AdmissionHandler { - meter := global.MeterProvider().Meter("kyverno") + meter := global.MeterProvider().Meter(metrics.MeterName) admissionRequestsMetric, err := meter.SyncInt64().Counter( "kyverno_admission_requests_total", instrument.WithDescription("can be used to track the number of admission requests encountered by Kyverno in the cluster"),