From 32d4fd126b9f1c8c36f9465617475363e5366133 Mon Sep 17 00:00:00 2001 From: Enrico Giorio Date: Wed, 23 Oct 2024 08:27:20 +0000 Subject: [PATCH] feat: OB-37910 Improve generation of Ingress rules We used to only return which host(s) each rule would apply to. This PR enhances the formatting of ingress rules, returning them as a JSON object with more information, such as the URL path they are defined on and which serviceName:port the rules are for. --- .../ingressactions.go | 89 +++++++++++++++++-- .../ingressactions_test.go | 52 ++++++++++- .../processor.go | 23 +++-- .../testdata/ingressEvent2.json | 68 ++++++++++++++ .../observek8sattributesprocessor/tmp.json | 22 +++++ 5 files changed, 236 insertions(+), 18 deletions(-) create mode 100644 components/processors/observek8sattributesprocessor/testdata/ingressEvent2.json create mode 100644 components/processors/observek8sattributesprocessor/tmp.json diff --git a/components/processors/observek8sattributesprocessor/ingressactions.go b/components/processors/observek8sattributesprocessor/ingressactions.go index 767d6d16b..49943f02c 100644 --- a/components/processors/observek8sattributesprocessor/ingressactions.go +++ b/components/processors/observek8sattributesprocessor/ingressactions.go @@ -10,18 +10,88 @@ import ( const ( IngressRulesAttributeKey = "rules" IngressLoadBalancerAttributeKey = "loadBalancer" + hostKey = "host" + rulesKey = "rules" + httpRulesKey = "httpRules" + pathKey = "path" + backendKey = "backend" + serviceKey = "service" + resourceKey = "resource" + portKey = "port" + nameKey = "name" ) -// Adapted from https://github.com/kubernetes/kubernetes/blob/0d3b859af81e6a5f869a7766c8d45afd1c600b04/pkg/printers/internalversion/printers.go#L1373 -func formatIngressRules(rules []netv1.IngressRule) string { - list := []string{} +// formatIngressRules converts a slice of IngressRules to a minimal JSON representation. +// The json structure is: +// +// { +// "host": "example.com", (or "*" if no host is specified) +// "rules": [ +// { +// "path": "/app1", +// "backend": { +// "service": { (inside "backend" could be either "service" or "resource") +// "name": "app1-service", +// "port": 8080 (could be either a number or a string) +// } +// } +// }, +// { +// "path": "/app2", +// "backend": { +// "resource": "app2-resource", (alternative to service) +// (no port here, just the name of the resource) +// } +// } +// ] +// } +func formatIngressRules(rules []netv1.IngressRule) []attributes { + var ret []attributes + for _, rule := range rules { - list = append(list, rule.Host) - } - if len(list) == 0 { - return "*" + host := rule.Host + if host == "" { + host = "*" + } + + var httpRules []attributes + if rule.HTTP != nil { + for _, path := range rule.HTTP.Paths { + + ruleInfo := attributes{ + pathKey: path.Path, + } + backend := path.Backend + backendAttrs := attributes{} + if backend.Service != nil { + service := backend.Service + serviceAttrs := attributes{ + nameKey: service.Name, + } + // Remove one level of indentation and use either the port + // name or the number as "port" directly inside "service", + // since we can use values of any type. + if service.Port.Name != "" { + serviceAttrs[portKey] = service.Port.Name + } else { + serviceAttrs[portKey] = service.Port.Number + } + backendAttrs[serviceKey] = serviceAttrs + } else { + backendAttrs[resourceKey] = backend.Resource.Name + } + ruleInfo[backendKey] = backendAttrs + + httpRules = append(httpRules, ruleInfo) + } + } + + ret = append(ret, attributes{ + hostKey: host, + httpRulesKey: httpRules, + }) } - ret := strings.Join(list, ",") + return ret } @@ -35,7 +105,8 @@ func NewIngressRulesAction() IngressRulesAction { // Generates the Ingress "rules" facet. func (IngressRulesAction) ComputeAttributes(ingress netv1.Ingress) (attributes, error) { - return attributes{IngressRulesAttributeKey: formatIngressRules(ingress.Spec.Rules)}, nil + rules := formatIngressRules(ingress.Spec.Rules) + return attributes{IngressRulesAttributeKey: rules}, nil } // ---------------------------------- Ingress "loadBalancer" ---------------------------------- diff --git a/components/processors/observek8sattributesprocessor/ingressactions_test.go b/components/processors/observek8sattributesprocessor/ingressactions_test.go index 224c83d26..5cf9bec99 100644 --- a/components/processors/observek8sattributesprocessor/ingressactions_test.go +++ b/components/processors/observek8sattributesprocessor/ingressactions_test.go @@ -5,14 +5,58 @@ import "testing" func TestIngressActions(t *testing.T) { for _, testCase := range []k8sEventProcessorTest{ { - name: "Ingress rules", + name: "Ingress rules with host", inLogs: resourceLogsFromSingleJsonEvent("./testdata/ingressEvent.json"), expectedResults: []queryWithResult{ - {"observe_transform.facets.rules", "prometheus.observe-eng.com"}, - }, + { + "observe_transform.facets.rules", []any{ + map[string]any{ + "host": "prometheus.observe-eng.com", + "httpRules": []any{ + map[string]any{ + "backend": map[string]any{ + "service": map[string]any{ + "name": "prometheus", + "port": "prometheus", + }, + }, + "path": "/", + }}}}}}, + }, + { + name: "Ingress rules without host", + inLogs: resourceLogsFromSingleJsonEvent("./testdata/ingressEvent2.json"), + expectedResults: []queryWithResult{ + { + "observe_transform.facets.rules", []any{ + map[string]any{ + "host": "*", + "httpRules": []any{ + map[string]any{ + "backend": map[string]any{ + "service": map[string]any{ + "name": "test", + "port": int64(80), + }, + }, + "path": "/testpath", + }}, + }, + // Rule with resource backend + map[string]any{ + "host": "test.com", + "httpRules": []any{ + map[string]any{ + "backend": map[string]any{ + "resource": "testResource", + }, + "path": "/testpath2", + }}, + }, + }}}, }, { - name: "Ingress rules", + name: "Load Balancer", inLogs: resourceLogsFromSingleJsonEvent("./testdata/ingressEvent.json"), expectedResults: []queryWithResult{ {"observe_transform.facets.loadBalancer", "someUniqueElbIdentifier.elb.us-west-2.amazonaws.com"}, diff --git a/components/processors/observek8sattributesprocessor/processor.go b/components/processors/observek8sattributesprocessor/processor.go index de3b5a197..0e6548943 100644 --- a/components/processors/observek8sattributesprocessor/processor.go +++ b/components/processors/observek8sattributesprocessor/processor.go @@ -322,9 +322,6 @@ func (kep *K8sEventsProcessor) processLogs(_ context.Context, logs plog.Logs) (p facetsMap = facets.Map() } else { facetsMap = transformMap.PutEmptyMap("facets") - // Make sure we have capacity for at least as many actions as we have defined - // Actions could generate more than one facet, that's taken care of afterwards. - facetsMap.EnsureCapacity(len(kep.podActions)) } // Compute custom attributes @@ -335,6 +332,8 @@ func (kep *K8sEventsProcessor) processLogs(_ context.Context, logs plog.Logs) (p } // Add them to the facets + // Ensre we have extra capacity for as many attributes as we + // have just compted facetsMap.EnsureCapacity(facetsMap.Len() + len(values)) for key, val := range values { if err := mapPut(facetsMap, key, val); err != nil { @@ -353,14 +352,20 @@ func slicePut(theSlice pcommon.Slice, value any) error { switch typed := value.(type) { case string: elem.SetStr(typed) + case int32: + elem.SetInt(int64(typed)) case int64: elem.SetInt(typed) case bool: elem.SetBool(typed) case float64: elem.SetDouble(typed) - // Let's not complicate things and avoid putting maps/slices into slices. - // There's gotta be an easier way to model the processor's output to avoid it + case attributes: + elem.SetEmptyMap() + elem.Map().EnsureCapacity(len(typed)) + for k, v := range typed { + mapPut(elem.Map(), k, v) + } default: return errors.New("unrecognised type. Cannot be added to a slice") } @@ -383,6 +388,8 @@ func mapPut(theMap pcommon.Map, key string, value any) error { theMap.PutStr(key, typed) case int: theMap.PutInt(key, int64(typed)) + case int32: + theMap.PutInt(key, int64(typed)) case int64: theMap.PutInt(key, typed) case bool: @@ -395,6 +402,12 @@ func mapPut(theMap pcommon.Map, key string, value any) error { for _, elem := range typed { slicePut(slc, elem) } + case []attributes: + slc := theMap.PutEmptySlice(key) + slc.EnsureCapacity(len(typed)) + for _, elem := range typed { + slicePut(slc, elem) + } case attributes: // This is potentially arbitrarily recursive. We don't care about // checking the nesting level since we will never need to define diff --git a/components/processors/observek8sattributesprocessor/testdata/ingressEvent2.json b/components/processors/observek8sattributesprocessor/testdata/ingressEvent2.json new file mode 100644 index 000000000..db79ba63a --- /dev/null +++ b/components/processors/observek8sattributesprocessor/testdata/ingressEvent2.json @@ -0,0 +1,68 @@ +{ + "kind": "Ingress", + "apiVersion": "networking.k8s.io/v1", + "metadata": { + "name": "minimal-ingress", + "namespace": "k8smonitoring", + "uid": "8874fc28-72be-4c88-b15e-6d82af7c4f5d", + "resourceVersion": "862319", + "generation": 1, + "creationTimestamp": "2024-10-22T08:29:52Z", + "annotations": { + "kubectl.kubernetes.io/last-applied-configuration": "{\"apiVersion\":\"networking.k8s.io/v1\",\"kind\":\"Ingress\",\"metadata\":{\"annotations\":{\"nginx.ingress.kubernetes.io/rewrite-target\":\"/\"},\"name\":\"minimal-ingress\",\"namespace\":\"k8smonitoring\"},\"spec\":{\"ingressClassName\":\"nginx-example\",\"rules\":[{\"http\":{\"paths\":[{\"backend\":{\"service\":{\"name\":\"test\",\"port\":{\"number\":80}}},\"path\":\"/testpath\",\"pathType\":\"Prefix\"}]}}]}}\n", + "nginx.ingress.kubernetes.io/rewrite-target": "/" + }, + "managedFields": [ + { + "manager": "kubectl-client-side-apply", + "operation": "Update", + "apiVersion": "networking.k8s.io/v1", + "time": "2024-10-22T08:29:52Z", + "fieldsType": "FieldsV1", + "fieldsV1": { + "f:metadata": { + "f:annotations": { + ".": {}, + "f:kubectl.kubernetes.io/last-applied-configuration": {}, + "f:nginx.ingress.kubernetes.io/rewrite-target": {} + } + }, + "f:spec": { "f:ingressClassName": {}, "f:rules": {} } + } + } + ] + }, + "spec": { + "ingressClassName": "nginx-example", + "rules": [ + { + "http": { + "paths": [ + { + "path": "/testpath", + "pathType": "Prefix", + "backend": { + "service": { "name": "test", "port": { "number": 80 } } + } + } + ] + } + }, + { + "host": "test.com", + "http": { + "paths": [ + { + "path": "/testpath2", + "pathType": "Prefix", + "backend": { + "resource": { "name": "testResource" } + } + } + ] + } + } + ] + }, + "status": { "loadBalancer": {} } +} diff --git a/components/processors/observek8sattributesprocessor/tmp.json b/components/processors/observek8sattributesprocessor/tmp.json new file mode 100644 index 000000000..d5efe7fad --- /dev/null +++ b/components/processors/observek8sattributesprocessor/tmp.json @@ -0,0 +1,22 @@ +{ + "host": "example.com", // (or "*" if no host is specified) + "rules": [ + { + "path": "/app1", + "backend": { + "service": { + "name": "app1-service", + "port": 8080 // (this port could also be the port name of type string) + } + } + }, + { + "path": "/app2", + "backend": { + // (The backend could be either service or resource) + "resource": "app2-resource", + "port": "somePortName" // (alternative to port number) + } + } + ] +}