elastic / elastic/cloud-on-k8s
Improve ECS support in operator logs
- 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
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