elastic / elastic/cloud-on-k8s

Improve ECS support in operator logs

Open
#2,467 0 comments 2 reactions 0 assignees View on GitHub
>enhancement
Dominant language
Go
Stars
2.8k
Forks
790
Avg merge
3d 15h
Merged PRs (30d)
37

Description

Following up #2002, #2457 added some support for ECS but some improvements can be done:

1. Should we consider a "reconciliation loop" as an [event](https://www.elastic.co/guide/en/ecs/current/ecs-event.html)?

* `event.sequence` looks like our `iteration` field _(number of times a controller has run its Reconcile method)_
* `event.duration` looks like our current `took`
* `event.type` could be [ `change`| `creation` | `deletion` ]
* `event.dataset` could help to separate the logs from the different controller (elasticsearch-controller, kibana-controller...)

2. The presence of some fields is inconsistent

It is worth to note that it is not easy to be consistent re. the key/values added to the log.
The most problematic case is when an error is managed by the [`controller` itself](https://github.com/kubernetes-sigs/controller-runtime/blob/8c39906c77cdb574482c01fcd74a76b105b06522/pkg/internal/controller/controller.go#L256) _(i.e. not by our code)_ :

```golang
if result, err := c.Do.Reconcile(req); err != nil {
c.Queue.AddRateLimited(req)
log.Error(err, "Reconciler error", "controller", c.Name, "request", req)
[...]
return false
}
```
This would produce:
```json
{
"message": "Reconciler error",
"log.logger": "controller-runtime.controller",
"log.level": "error",
"controller": "kibana-association-controller",
"error.message": "An error message about kibanas.kibana.k8s.elastic.co \"kb-apm-sample\": something bad happened",
"error.stack_trace": "github.com/go-logr/zapr.(*zapLogger).Error\n\t/go/pkg/mod/github.com/go-logr/zapr@v0.1.0/zapr.go:128\nsigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).reconcileHandler\n\t/go/pkg/mod/sigs.k8s.io/controller-runtime@v0.4.0/pkg/internal/controller/controller.go:258\nsigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).processNextWorkItem\n\t/go/pkg/mod/sigs.k8s.io/controller-runtime@v0.4.0/pkg/internal/controller/controller.go:232\nsigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).worker\n\t/go/pkg/mod/sigs.k8s.io/controller-runtime@v0.4.0/pkg/internal/controller/controller.go:211\nk8s.io/apimachinery/pkg/util/wait.JitterUntil.func1\n\t/go/pkg/mod/k8s.io/apimachinery@v0.0.0-20191028221656-72ed19daf4bb/pkg/util/wait/wait.go:152\nk8s.io/apimachinery/pkg/util/wait.JitterUntil\n\t/go/pkg/mod/k8s.io/apimachinery@v0.0.0-20191028221656-72ed19daf4bb/pkg/util/wait/wait.go:153\nk8s.io/apimachinery/pkg/util/wait.Until\n\t/go/pkg/mod/k8s.io/apimachinery@v0.0.0-20191028221656-72ed19daf4bb/pkg/util/wait/wait.go:88",
"request": "namespace1/kb-apm-sample"
}
```

In the above example if a user is filtering on `namespace: "namespace1" and kibana_name: "kb-apm-sample"` then the error will not be displayed.
We could propose to the controller-runtime project to replace `"request": "namespace1/kb-apm-sample"` by something like:

```golang
log.Error(err, "Reconciler error", "controller", c.Name, "namespace", req.Namespace, "name", req.name)
```
_(we would still have a problem with the "name" field but this could be revisited in our code)_
Or add a processor or our own encoder to transform the output.

We also have some inconsistencies in our code, mostly in the "common" packages, for example when a certificate is generated:
https://github.com/elastic/cloud-on-k8s/blob/b1355ceb5b293a6469c3b196d00ffcb070e41437/pkg/controller/common/certificates/http/reconcile.go#L145

I think it would be nice to add a reference to the owner in order to link this log to the parent resource.

3. Once #2462 is merged we should add some correlation fields (see https://github.com/elastic/cloud-on-k8s/pull/2462#pullrequestreview-347354900)

Contributor guide

Open the contributing guide

Research direction

Start by reviewing the ECS event fields and the controller-runtime logging example in the issue, then inspect pkg/controller/common/certificates/http/reconcile.go and the changes proposed in #2462. Define the desired field consistency, owner references, and correlation fields before implementation. Done means operator logs follow an agreed ECS-compatible structure, including errors handled by controller-runtime.

Written by the indexing model from the issue text.

Assessment

Tech stack
go, kubernetes
Domain
observability-sre
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.