feat: add kubernetes-metadata #8
Conversation
csatib02
force-pushed
the
feat/improvements
branch
from
4bf1467 to
9d6e895
Compare
There was a problem hiding this comment.
To start, thank you for your contribution.
If you can (or next time), it is best for the reviewer if you split the skip_fail and the kubernetes changes in 2 separate MRs. Because it will take more time to review the k8s addition where I could have merged quickly the skip_fail one.
|
@r0mdau Added the changes! |
|
@csatib02 can you add the new I want to ensure default value is Otherwise, looks good to me. |
csatib02
force-pushed
the
feat/improvements
branch
from
493c685 to
b60b874
Compare
|
@r0mdau Done. |
| if f.config.KubernetesMetadata != nil { | ||
| key := f.config.KubernetesMetadata.Key | ||
| if f.config.KubernetesMetadata.Key == "" { | ||
| key = "kubernetes" | ||
| } | ||
| var namespace, container, pod, node string | ||
| var labels map[string]string | ||
| res.Resource().Attributes().Range(func(k string, v pcommon.Value) bool { | ||
| if k == "k8s.namespace.name" { | ||
| namespace = v.AsString() | ||
| return true | ||
| } | ||
| if k == "k8s.container.name" { | ||
| container = v.AsString() | ||
| } | ||
| if k == "k8s.pod.name" { | ||
| pod = v.AsString() | ||
| } | ||
| if k == "k8s.node.name" { | ||
| node = v.AsString() | ||
| } | ||
| if f.config.KubernetesMetadata.IncludePodLabels && strings.HasPrefix(k, "k8s.pod.labels.") { | ||
| if labels == nil { | ||
| labels = make(map[string]string) | ||
| } | ||
| labelKey := strings.TrimPrefix(k, "k8s.pod.labels.") | ||
| labels[labelKey] = v.AsString() | ||
| } | ||
| return true | ||
| }) | ||
|
|
||
| k8sMetadata := map[string]interface{}{ | ||
| "namespace_name": namespace, | ||
| "container_name": container, | ||
| "pod_name": pod, | ||
| "host": node, | ||
| } | ||
|
|
||
| if f.config.KubernetesMetadata.IncludePodLabels { | ||
| k8sMetadata["labels"] = labels | ||
| } | ||
|
|
||
| m[key] = k8sMetadata | ||
| } | ||
|
|
||
| f.settings.Logger.Debug(fmt.Sprintf("message %+v", m)) | ||
|
|
There was a problem hiding this comment.
This code is for use only with processor/k8sattributesprocessor
I see 2 things:
- Put log Resources into a nested object, like from flat to a sub json document
- Rename fields, for example from
k8s.namespace.nametonamespace_namein the parent object key labeled by theKeyproperty.
First idea for me is to not rename, if you want to rename, you must use the processor/attributesprocessor after the k8sattributesprocessor.
Second idea for me is to not be kubernetes specific. Whatever the ResourceLogs come from.
Example, to get the value in the log line as flat (not in a nested json object), you just need to add it to the default_labels_enabled property, example:
exporters:
fluentforward:
endpoint:
tcp_addr: localhost:24224
default_labels_enabled:
k8s.namespace.name: trueI am wondering in which scenario on your side creating a sub json object is needed ?
There was a problem hiding this comment.
First idea for me is to not rename, if you want to rename, you must use the processor/attributesprocessor after the
k8sattributesprocessor.
That is exactly what we are doing!
You can check it out here:
- https://github.com/kube-logging/telemetry-controller/blob/main/internal/controller/telemetry/pipeline/components/processor/k8s_attributes_processor.go
- https://github.com/kube-logging/telemetry-controller/blob/e9335cec53285bbc36f19c244f8a48c7a58d4412/internal/controller/telemetry/pipeline/components/processor/attributes_processor.go#L75
Also sharing with you an example configuration visualized using OtelBin.
- I would also like to emphasize that the
k8sattributesprocessorand theattributesprocessorsin the pipelines generated by our operator are always present, and cannot be turned off. Mainly to be able to provide users with consistent metrics about their tenants.
There was a problem hiding this comment.
For the renaming topic, taking one field for example for the discussion but it applies to the four fields.
Why extracting k8s.container.name and putting its value in the log field labeled: container_name ?
With my short configuration example above, you can keep the attribute labeled k8s.container.name and its value in the log.
And you can do the same with all the others k8s.pod.name etc...
There was a problem hiding this comment.
The project I've sent the code-snippets from, soon will be available to use together, with one of our other operators: https://github.com/kube-logging/logging-operator
This operator historically used fluentbit for log-collection, which sent the logs in that structure. So we opted to use the same approach with otelcol aswell for compatibility reasons.
This also ensures we don't accidentally break components on other levels of the pipeline.
There was a problem hiding this comment.
I have not checked all the log processors, but I think you can get the same log output from fluentforwardexporter using processors and adding a default_labels_enabled like I suggested above.
It is super important to me to not bake strongly kubernetes dependent logic and attributes within the exporter. Have you seen other otelcol exporters doing that (I am not sure) ?
There was a problem hiding this comment.
I totally understand if you don't want to include this because of k8s dependent logic. It was just a proposal from our-side. Feel free to close this. :)
There was a problem hiding this comment.
We could change the exporter logic by using a denylist instead of an allowlist with the default_labels_enabled property. But works good like it is for now.
I am closing the PR.
Signed-off-by: Bence Csati <[email protected]>
Signed-off-by: Bence Csati <[email protected]>
csatib02
force-pushed
the
feat/improvements
branch
from
b60b874 to
cab8e72
Compare
Some tweaks and enhancements we did to
fluentforwardexporter, we thought you might also find useful.