-
Notifications
You must be signed in to change notification settings - Fork 2.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Introduce a container_parser operator for container/k8s logs parsing #31959
Comments
Pinging code owners:
See Adding Labels via Comments if you do not have permissions to add labels yourself. |
I am definitely in favor |
I am in favor of the idea. Generally we try not to write in code what can be done with config, but in this case it is a very specific use case which is also very common. This would make it easier to configure while also giving us the opportunity to optimize performance (e.g. by performing a fewer number of more targeted actions) and also add resiliency (e.g. implement a next-best behavior if X assumption fails). |
This is currently a huge pain for everyone using the Collector for k8s parsing, so huge thanks for the idea! 🚀 What do you think about incorporating the
|
Sure @OverOrion, I think it makes sense to include the There are several things within the implementation that we will need to discuss like re-using already existing code and how much of it. |
I have a draft which implements the parser. It's not widely tested but the basic scenarios seem to work. I will deal with tests and improvements once we verify the implementation details decided. The main thing here is that there is a fundamental question to answer regarding the implementation. It's about how much we should re-use from the existent operators' implementations. Some operations like the routing and the base parsing are easier and makes sense to implement them within the new parser. This gives us the flexibility to easily extend, customize and error handle the flow. However the recombine part for the crio logs is already implemented by the Based on this, I implemented the parser following the hybrid approach bellow:
I spent some time thinking of anything that we could do differently but I concluded that the hybrid approach @djaglowski and all what do you think about this? |
I agree with your assessment that we should reuse the recombine operator and rewrite the routing and parsing. However, I differ on a few other points. I think it's probably not necessary to use
This looks really complicated and it's not clear to me that we need to manage a pipeline internally, especially one that requires dynamic injection & removal of operators. Not only does this seem more complicated than necessary, but I suspect we would be leaving easy performance gains on the table. Can we just instantiate an internal instance of the recombine operator, only pass logs to it if the format is crio, and figure out how to consume it's output asynchronously without a pipeline being involved? |
Thank's @djaglowski for checking this! UPDATE: It looks like even the last |
So I tried to remove the pipeline logic and consume messages asynchronously from the internal recombine operator. I have a draft of this version that seems to work for the basic scenarios at 7dc6949. I just want to verify that we are ok with extending the Note: We/I need to ensure that the stopping process happens sequentially to avoid losing any messages like in other issues we are facing with the filelog receiver. |
Rather than adding complexity to the recombine operator's internals, can we just use |
That sounds reasonable. However, in order to reuse the UPDATE: I filed a draft to illustrate this refactoring. Let me know what you think. |
…2629) **Description:** <Describe what has changed.> <!--Ex. Fixing a bug - Describe the bug and how this fixes the issue. Ex. Adding a feature - Explain what this achieves.--> This PR moves `LogEmitter` to a common place in `pkg/stanza/operator/helper` so as to be reusable by operators without hitting circular imports. This is explained at #31959 (comment). **Link to tracking Issue:** <Issue number if applicable> **Testing:** <Describe what testing was performed and which tests were added.> **Documentation:** <Describe the documentation added.> Signed-off-by: ChrsMark <chrismarkou92@gmail.com>
Love the idea, and am happy to help with testing. We use a fairly complicated operator pipeline for our container logging here, this would help simplify it. |
…en-telemetry#32629) **Description:** <Describe what has changed.> <!--Ex. Fixing a bug - Describe the bug and how this fixes the issue. Ex. Adding a feature - Explain what this achieves.--> This PR moves `LogEmitter` to a common place in `pkg/stanza/operator/helper` so as to be reusable by operators without hitting circular imports. This is explained at open-telemetry#31959 (comment). **Link to tracking Issue:** <Issue number if applicable> **Testing:** <Describe what testing was performed and which tests were added.> **Documentation:** <Describe the documentation added.> Signed-off-by: ChrsMark <chrismarkou92@gmail.com>
**Description:** <Describe what has changed.> <!--Ex. Fixing a bug - Describe the bug and how this fixes the issue. Ex. Adding a feature - Explain what this achieves.--> This PR implements the new container logs parser as it was proposed at #31959. **Link to tracking Issue:** <Issue number if applicable> #31959 **Testing:** <Describe what testing was performed and which tests were added.> Added unit tests. Providing manual testing steps as well: ### How to test this manually 1. Using the following config file: ```yaml receivers: filelog: start_at: end include_file_name: false include_file_path: true include: - /var/log/pods/*/*/*.log operators: - id: container-parser type: container output: m1 - type: move id: m1 from: attributes.k8s.pod.name to: attributes.val - id: some type: add field: attributes.key2.key_in value: val2 exporters: debug: verbosity: detailed service: pipelines: logs: receivers: [filelog] exporters: [debug] processors: [] ``` 2. Start the collector: `./bin/otelcontribcol_linux_amd64 --config ~/otelcol/container_parser/config.yaml` 3. Use the following bash script to create some logs: ```bash #! /bin/bash echo '2024-04-13T07:59:37.505201169-05:00 stdout P This is a very very long crio line th' >> /var/log/pods/kube-scheduler-kind-control-plane_49cc7c1fd3702c40b2686ea7486091d3/kube-scheduler43/1.log echo '{"log":"INFO: log line here","stream":"stdout","time":"2029-03-30T08:31:20.545192187Z"}' >> /var/log/pods/kube-controller-kind-control-plane_49cc7c1fd3702c40b2686ea7486091d6/kube-controller/1.log echo '2024-04-13T07:59:37.505201169-05:00 stdout F at is awesome! crio is awesome!' >> /var/log/pods/kube-scheduler-kind-control-plane_49cc7c1fd3702c40b2686ea7486091d3/kube-scheduler43/1.log echo '2021-06-22T10:27:25.813799277Z stdout P some containerd log th' >> /var/log/pods/kube-scheduler-kind-control-plane_49cc7c1fd3702c40b2686ea7486091d3/kube-scheduler44/1.log echo '{"log":"INFO: another log line here","stream":"stdout","time":"2029-03-30T08:31:20.545192187Z"}' >> /var/log/pods/kube-controller-kind-control-plane_49cc7c1fd3702c40b2686ea7486091d6/kube-controller/1.log echo '2021-06-22T10:27:25.813799277Z stdout F at is super awesome! Containerd is awesome' >> /var/log/pods/kube-scheduler-kind-control-plane_49cc7c1fd3702c40b2686ea7486091d3/kube-scheduler44/1.log echo '2024-04-13T07:59:37.505201169-05:00 stdout F standalone crio line which is awesome!' >> /var/log/pods/kube-scheduler-kind-control-plane_49cc7c1fd3702c40b2686ea7486091d3/kube-scheduler43/1.log echo '2021-06-22T10:27:25.813799277Z stdout F standalone containerd line that is super awesome!' >> /var/log/pods/kube-scheduler-kind-control-plane_49cc7c1fd3702c40b2686ea7486091d3/kube-scheduler44/1.log ``` 4. Run the above as a bash script to verify any parallel processing. Verify that the output is correct. ### Test manually on k8s 1. `make docker-otelcontribcol && docker tag otelcontribcol otelcontribcol-dev:0.0.1 && kind load docker-image otelcontribcol-dev:0.0.1` 2. Install using the following helm values file: ```yaml mode: daemonset presets: logsCollection: enabled: true image: repository: otelcontribcol-dev tag: "0.0.1" pullPolicy: IfNotPresent command: name: otelcontribcol config: exporters: debug: verbosity: detailed receivers: filelog: start_at: end include_file_name: false include_file_path: true exclude: - /var/log/pods/default_daemonset-opentelemetry-collector*_*/opentelemetry-collector/*.log include: - /var/log/pods/*/*/*.log operators: - id: container-parser type: container output: some - id: some type: add field: attributes.key2.key_in value: val2 service: pipelines: logs: receivers: [filelog] processors: [batch] exporters: [debug] ``` 3. Check collector's output to verify the logs are parsed properly: ```console 2024-05-10T07:52:02.307Z info LogsExporter {"kind": "exporter", "data_type": "logs", "name": "debug", "resource logs": 1, "log records": 2} 2024-05-10T07:52:02.307Z info ResourceLog #0 Resource SchemaURL: ScopeLogs #0 ScopeLogs SchemaURL: InstrumentationScope LogRecord #0 ObservedTimestamp: 2024-05-10 07:52:02.046236071 +0000 UTC Timestamp: 2024-05-10 07:52:01.92533954 +0000 UTC SeverityText: SeverityNumber: Unspecified(0) Body: Str(otel logs at 07:52:01) Attributes: -> log: Map({"iostream":"stdout"}) -> time: Str(2024-05-10T07:52:01.92533954Z) -> k8s: Map({"container":{"name":"busybox","restart_count":"0"},"namespace":{"name":"default"},"pod":{"name":"daemonset-logs-6f6mn","uid":"1069e46b-03b2-4532-a71f-aaec06c0197b"}}) -> logtag: Str(F) -> key2: Map({"key_in":"val2"}) -> log.file.path: Str(/var/log/pods/default_daemonset-logs-6f6mn_1069e46b-03b2-4532-a71f-aaec06c0197b/busybox/0.log) Trace ID: Span ID: Flags: 0 LogRecord #1 ObservedTimestamp: 2024-05-10 07:52:02.046411602 +0000 UTC Timestamp: 2024-05-10 07:52:02.027386192 +0000 UTC SeverityText: SeverityNumber: Unspecified(0) Body: Str(otel logs at 07:52:02) Attributes: -> log.file.path: Str(/var/log/pods/default_daemonset-logs-6f6mn_1069e46b-03b2-4532-a71f-aaec06c0197b/busybox/0.log) -> time: Str(2024-05-10T07:52:02.027386192Z) -> log: Map({"iostream":"stdout"}) -> logtag: Str(F) -> k8s: Map({"container":{"name":"busybox","restart_count":"0"},"namespace":{"name":"default"},"pod":{"name":"daemonset-logs-6f6mn","uid":"1069e46b-03b2-4532-a71f-aaec06c0197b"}}) -> key2: Map({"key_in":"val2"}) Trace ID: Span ID: Flags: 0 ... ``` **Documentation:** <Describe the documentation added.> Added Signed-off-by: ChrsMark <chrismarkou92@gmail.com>
Completed via #32594 |
Component(s)
receiver/filelog
Is your feature request related to a problem? Please describe.
At the moment the
filelog
receiver is capable to parse container logs from Kubernetes Pods but it requires lot's of specific configuration in order to support multiple container runtime formats.As it is mentioned at #25251 the recommended configuration is the following:
Despite we have #24439 and #23339 for improving the k8s logs parsing experience I wonder if we should already solve the parsing part on
filelog
's receiver level by encoding within the implementation the parsing patterns.That means essentially the following part:
Describe the solution you'd like
With implementing the format's parsing as an operator we can ensure that nothing will be missing from user's configs and that the parsing implementation will also be easily covered by specific unit tests.
The configuration will then be simplified to something like the following:
We can define the format specifically to
docker
,containerd
orcrio
for those users that want to explicitly define it.Additionally we could also support defining the stream (stdout/stderr) to focus parsing on like what Filebeat used to do.
cc: @djaglowski @TylerHelmuth
Describe alternatives you've considered
No response
Additional context
No response
The text was updated successfully, but these errors were encountered: