Initial commit for setting up a new component: k8slog receiver #24439

h0cheung · 2023-07-24T08:05:18Z

Description:

k8slog receiver provides full functionality to collect logs in k8s containers. It is an all-in-one solution, which supports to:

collect logs from files inside k8s containers via daemonset
support docker and cri-containerd
support many docker graph drivers / snapshotters
collect logs from stdout of k8s containers via daemonset
filter containers by metadata
extract metadata of containers
collect file logs in k8s containers via sidecar

If we want to collect logs from k8s, we can use this component.

Link to tracking Issue:

#23339

Testing:

Tests for unmarshaling configuration have been added.

Documentation:

Docs for configurations and overall design has been added.

github-actions · 2023-08-08T05:20:07Z

This PR was marked stale due to lack of activity. It will be closed in 14 days.

h0cheung · 2023-08-08T13:34:58Z

This PR was marked stale due to lack of activity. It will be closed in 14 days.

remove stale

dmitryax · 2023-08-14T20:55:50Z

receiver/k8slogreceiver/config.go

+)
+
+// Config is the configuration of a k8s input operator
+type Config struct {


Are all of the configuration options required for the initial implementation? Can you remove them for now and add them along with the functionality using them going forward?

I agree, starting small will make things easier to review and get started and we can always add them in the future.

TylerHelmuth

@h0cheung I am very excited about this component, sorry I didn't see your proposal sooner. @dmitryax thanks for pointing it out.

TylerHelmuth · 2023-08-14T21:22:59Z

receiver/k8slogreceiver/config.go

+
+	// Expr represents the rules to filter containers based on a custom expression.
+	// TODO: define the values passed to expr.
+	Expr string `mapstructure:"expr"`


Lets not introduce expr filtering to start. The other filter options you've provided are quite extensive. For expression language filtering I'd prefer we use OTTL.

receiver/k8slogreceiver/config.go

TylerHelmuth · 2023-08-14T21:28:38Z

receiver/k8slogreceiver/metadata.yaml

+  stability:
+    development: [logs]
+  codeowners:
+    active: [h0cheung]


You can add me here as well

OK, I will do it soon.

Hey @TylerHelmuth @h0cheung , I will be more than happy to help reviewing/testing and maintaining this component. In this, feel free to add me into the list as well if that's ok already.

TylerHelmuth · 2023-08-14T21:29:42Z

receiver/k8slogreceiver/config.go

+)
+
+// Config is the configuration of a k8s input operator
+type Config struct {


I agree, starting small will make things easier to review and get started and we can always add them in the future.

receiver/k8slogreceiver/config.go

h0cheung · 2023-09-04T02:42:50Z

@TylerHelmuth @dmitryax I've simplified the configs and added Tyler Helmuth to the code owner. Could you please have a look at this?

TylerHelmuth · 2023-09-06T22:35:27Z

receiver/k8slogreceiver/config.go

+	// Env represents the rules to extract from container environment variables.
+	Env []FieldExtractConfig `mapstructure:"env"`
+
+	// OtelEnv represents the rules to extract from environment variables of otel itself.
+	OtelEnv []FieldExtractConfig `mapstructure:"otel_env"`


I don't think we should allow extracting from env variables, that feels like a security risk.

OtelEnv has been removed.
However, Env is needed in some cases. For example, if there are some environment variables in the docker image, there is no other way to get it.
Is this really risky? resourcedetection supports it, and this component gets k8s node name from environment variables, too.

Using an env var to filter feels ok. But this section is for taking fields from the pod and adding them as resource attributes to the log right?

Getting attributes from environment variables is useful, and many components support it such as resoucedetection

Are there any reports of security risks associated with reading environment variables as strings and adding them to data?

The risk is exposing apikey/password information, but that risk is lowered if the config only allows static strings. If we do allow this I think we can do all env vars with 1 config option, no need to split out OTel env vars.

By default, the env config is empty, and no variable will be exposed. When a user set this, he should know what he is doing.

It seems that otel_env is necessary, so I've removed it in the latest commit.

@dmitryax do you have an opinion?

Any updates for this?

Lets move forward with this allowed as we've discussed.

I also find it useful to have the option to collect ENV_VAR information from the Pods.
That would give us important information about OTEL specific info like OTEL_SERVICE_NAME.
Having service.name populated in log records collected by the k8slog receiver is quite important since it will allow us to correlate logs with traces using the service.name value.

receiver/k8slogreceiver/config.go

github-actions · 2024-03-20T05:19:36Z

This PR was marked stale due to lack of activity. It will be closed in 14 days.

atoulme · 2024-03-20T07:26:00Z

@TylerHelmuth fyi is it a good time to review? I'll try my hand at it if possible.

atoulme · 2024-03-20T07:27:34Z

receiver/k8slogreceiver/README.md

+Two modes of discovery are planned to be supported in the future:
+
+- `daemonset-file`: Deployed as a DaemonSet, the receiver will read logs from files inside pods in the same node.
+- `sidecar`: Deployed as a sidecar container, the receiver will read logs from files.


not blocking, but it'd be nice to have issues to track those?

atoulme · 2024-03-20T07:28:27Z

receiver/k8slogreceiver/README.md

+|---------|-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
+| `op`    | The operation to perform. Options are: <br> - "equals": (default) the value must be equal to the specified value. <br>- "not-equals": the value must not be equal to the specified value. <br> - "exists": the value must exist. <br> - "not-exists": the value must not exist. <br> - "matches": the value must match the specified regular expression. <br> - "not-matches": the value must not match the specified regular expression. |
+| `key`   | The key of the map.                                                                                                                                                                                                                                                                                                                                                                                                                       |
+| `value` | The value to match. Only used for "equals", "not-equals", "matches", and "not-matches" operations.                                                                                                                                                                                                                                                                                                                                        |


Do we need this right away?

Note other components also have built their own filters, such as k8sobjects.

This filter is similar to the k8sattributes processor. Many users who want to use this component may be familiar with k8sattributes since it was the previous solution for correlating k8s metadata to logs. This may reduce their migration costs.

atoulme

As a first PR it looks like this can land. You could cut down a bit on the filters to bring them back later with more testing, but it's ok to mature them in as well.

ChrsMark · 2024-03-29T07:35:22Z

receiver/k8slogreceiver/README.md

+| `extract`        |                    | The rules to extract metadata from pods and containers. TODO default values.                                     |
+| TODO: add fields for reading files similar to filelogreceiver |
+
+When `discovery.mode` is not `sidecar`, there are additional configuration options:


It would be nice to mention what functionalities users lose by not being able set these options.

receiver/k8slogreceiver/design.md

ChrsMark · 2024-03-29T07:50:26Z

receiver/k8slogreceiver/design.md

+
+The first one is caculated by the graph driver, and the second one is from the mount points.
+
+Also, the host root is mounted under /host-root on the otel container.


Will the otel container need any special permissions in order to be able to share mounts with the incoming target Pods on the fly?

No. It connect to the runtime API.

ChrsMark · 2024-03-29T07:53:50Z

receiver/k8slogreceiver/config.go

+	// NodeFromEnv represents the environment variable which contains the node name.
+	NodeFromEnv string `mapstructure:"node_from_env"`
+
+	// HostRoot represents the path which is used to mount the host's root filesystem.


I'm not sure why we need the whole host's root directory to be mounted. Can't we just focus on /var/log/pods by default for the stdout mode? I guess many users would have concerns if their whole host directory is mounted by default.

Yes. But the files under /var/log/pods may be symbol links to paths defined by runtimes, which can also be configured by ops. If they don't mount the whole host's root directory, they must have knowledge about will the files actually live. This may be difficult for users, especially with k8s clusters that mix nodes with different runtimes.

Wouldn't that be better if we chose a reasonable list of directories like /var/log/pods/*, /var/lib/docker/* etc in order to specifically define the possible directories the known runtimes use?
We can even make this list configurable so as advanced users could set their own extra directories if the want.

Mounting the whole host's root directory looks problematic to me and maybe shouldn't be the default.
In GKE Autopilot for example this would not be supported: grafana/loki#9100.

So my suggestion would be to start with reasonable defaults, while also giving the option for more advanced users to extend these defaults.

ChrsMark · 2024-03-29T08:00:30Z

receiver/k8slogreceiver/design.md

+```
+
+The `Source` will get pod list and metadata from k8s api (maybe there will also be an implementation using kubelet).
+Then, it will get the logPath and env from docker/cri-containerd api. And they will be associated with the pod metadata


Can't we just by-pass the extra API call to the runtime and construct the log path based on a default pattern which is more or less known? i.e. /var/log/pods/'<namespace>_<pod_name>_<pod_uid>'/<container_name>/*.log, where namespace, pod_name, pod_uid and container_name are already known from the k8s API.

This also works. Datadog is using this way. But:

The path is different across k8s versions. Maybe it will also change in future. see https://github.com/DataDog/datadog-agent/blob/0d3dcb9fdb96e8a9d1810bb041d2dae6250372f8/pkg/logs/launchers/container/tailerfactory/file.go#L239

Some runtime API will directly return the actual path, which reduces resolving symbol links.

If we bypass extra API, it will be complex to get environment variables defined by downward api.

Co-authored-by: Chris Mark <chrismarkou92@gmail.com>

github-actions · 2024-04-17T05:19:03Z

This PR was marked stale due to lack of activity. It will be closed in 14 days.

h0cheung · 2024-04-24T02:33:36Z

This PR was marked stale due to lack of activity. It will be closed in 14 days.

remove stale

h0cheung requested a review from a team as a code owner July 24, 2023 08:05

h0cheung requested a review from kovrus July 24, 2023 08:05

github-actions bot assigned bogdandrutu Jul 24, 2023

h0cheung force-pushed the feat/k8slog-setup branch from 4858200 to 3e05fa1 Compare July 24, 2023 10:02

github-actions bot added the Stale label Aug 8, 2023

h0cheung force-pushed the feat/k8slog-setup branch from 3e05fa1 to ea3a841 Compare August 8, 2023 13:42

github-actions bot removed the Stale label Aug 9, 2023

h0cheung force-pushed the feat/k8slog-setup branch 6 times, most recently from 951c26c to 04d4b40 Compare August 10, 2023 06:39

dmitryax mentioned this pull request Aug 14, 2023

Improve Kubernetes Logs Collection Experience #25251

Closed

dmitryax reviewed Aug 14, 2023

View reviewed changes

TylerHelmuth reviewed Aug 14, 2023

View reviewed changes

TylerHelmuth removed the request for review from kovrus August 14, 2023 21:32

dmitryax mentioned this pull request Aug 28, 2023

New component: Kubernetes api logs receiver #24641

Closed

h0cheung force-pushed the feat/k8slog-setup branch from 04d4b40 to 6eb08af Compare September 1, 2023 12:39

h0cheung requested a review from atoulme as a code owner September 1, 2023 12:39

github-actions bot added the cmd/githubgen label Sep 1, 2023

h0cheung force-pushed the feat/k8slog-setup branch 2 times, most recently from 82afa36 to 5fde154 Compare September 1, 2023 13:06

TylerHelmuth reviewed Sep 6, 2023

View reviewed changes

h0cheung force-pushed the feat/k8slog-setup branch 3 times, most recently from 132b260 to 8256f32 Compare September 8, 2023 12:35

h0cheung force-pushed the feat/k8slog-setup branch 6 times, most recently from f7a8980 to a912135 Compare March 5, 2024 11:23

github-actions bot added the Stale label Mar 20, 2024

atoulme reviewed Mar 20, 2024

View reviewed changes

atoulme approved these changes Mar 20, 2024

View reviewed changes

h0cheung added 3 commits March 20, 2024 22:29

feat: initial commit for k8slog receiver

8e06cf6

mod: only try to connect to cri by default

842071a

mod: update README to respect the code changes

41ebcc6

h0cheung force-pushed the feat/k8slog-setup branch 3 times, most recently from 954c61e to 60fc7e4 Compare March 20, 2024 15:00

h0cheung added 2 commits March 20, 2024 23:07

docs (receiver/k8slog): describe detailed design for each mode.

117ac98

feat (receiver/k8slog): update dependencies.

b85c330

h0cheung force-pushed the feat/k8slog-setup branch from 60fc7e4 to b85c330 Compare March 20, 2024 15:08

github-actions bot removed the Stale label Mar 21, 2024

ChrsMark mentioned this pull request Mar 26, 2024

Introduce a container_parser operator for container/k8s logs parsing #31959

Open

ChrsMark reviewed Mar 29, 2024

View reviewed changes

docs(receiver/k8slog): fix typo

74863f9

Co-authored-by: Chris Mark <chrismarkou92@gmail.com>

OverOrion mentioned this pull request Apr 12, 2024

PodLogMonitor resource for better logging self service in kubernetes deployments open-telemetry/opentelemetry-operator#2879

Open

github-actions bot added the Stale label Apr 17, 2024

github-actions bot removed the Stale label Apr 24, 2024


		The first one is caculated by the graph driver, and the second one is from the mount points.

		Also, the host root is mounted under /host-root on the otel container.

Initial commit for setting up a new component: k8slog receiver #24439

Are you sure you want to change the base?

Initial commit for setting up a new component: k8slog receiver #24439

Conversation

h0cheung commented Jul 24, 2023

github-actions bot commented Aug 8, 2023

h0cheung commented Aug 8, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TylerHelmuth left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

h0cheung Aug 20, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

h0cheung commented Sep 4, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Mar 20, 2024

atoulme commented Mar 20, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

h0cheung Mar 21, 2024 • edited

Choose a reason for hiding this comment

atoulme left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Apr 17, 2024

h0cheung commented Apr 24, 2024

h0cheung Aug 20, 2023 •

edited

h0cheung Mar 21, 2024 •

edited