Skip to content
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

Clarify kubernetes logs attributes names #2628

Closed
sumo-drosiek opened this issue Mar 10, 2021 · 15 comments
Closed

Clarify kubernetes logs attributes names #2628

sumo-drosiek opened this issue Mar 10, 2021 · 15 comments

Comments

@sumo-drosiek
Copy link
Member

sumo-drosiek commented Mar 10, 2021

There are some attributes for kubernetes container logs, which can be useful (or not) and doesn't have naming convention so far:

  • stream (either stdout or stderr) which can be used for example for keeping only stderr logs
  • run_id - I don't know proper name of this attribute. This is run number of container (pods? maybe) (starts from 0 and it's increased with every container restart). This is name of a file log (/var/log/pods/<namespace>_<pod-name>_<pod-uid>/<container-name>/<run-id>.log)

Another question is if container-name is more like k8s.container.name or container.name

semantic convention
k8s convention
container convention

Additional context
#2606

@pmm-sumo
Copy link
Contributor

I wonder if stream should be a member of Log Data Model or a dynamic attribute. What do you think @tigrannajaryan @djaglowski ? In case of the latter, perhaps we might want to have log-specific semantic conventions

As for run_id, I think we should just propose it as part of the specification

@djaglowski
Copy link
Member

I slightly prefer not to add it to the data model itself. There are a lot situations where stream isn't applicable. If we did add it to the data model, I think there is a question of what the default value would be. Perhaps stdout is sufficiently neutral, but I can imagine scenarios where this would be confusing. Perhaps a third possible value such as default or unknown would be necessary.

@tigrannajaryan tigrannajaryan added this to the Basic Logs Support milestone Mar 16, 2021
@tigrannajaryan tigrannajaryan changed the title kubernetes logs attributes names Clarify kubernetes logs attributes names Mar 16, 2021
@tigrannajaryan
Copy link
Member

Note that names like stream and run_id are not compliant with Otel spec recommendations: https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/common/attribute-and-label-naming.md

We need to put them in some reasonable namespaces, I don't think they can be top-level fields.

@tigrannajaryan
Copy link
Member

Seems like run_id can belong to container.* Otel semantic conventions namespace.

@pmm-sumo
Copy link
Contributor

As discussed during the SIG, we might also want to put other log-related attributes, such as the path. ECS schema has a log namespace which includes log.file.path among other fields.

Perhaps we should follow that approach and define log namespace, which could include log.file.path and log.stream?

@tigrannajaryan
Copy link
Member

I can't find the concept of run id in K8s docs. Does anyone have a link?

@sumo-drosiek
Copy link
Member Author

sumo-drosiek commented Mar 18, 2021

I did some research around it and see some references in code as instance number link or container instance link

Didn't find anything useful in documentation

Based on that, I would go with container.instance or container.instance_number

@sumo-drosiek
Copy link
Member Author

Also, we won't be able to ensure that container_id taken by enrichment is truly the container id related to container instance (as container can be restarted meantime)

@dmitryax
Copy link
Member

@sumo-drosiek @tigrannajaryan @pmm-sumo Looks like the run_id part of the logs path represents number of container restarts from k8s API. I would like to suggest to call the attribute container.restart_count.

I want to settle a semantic convention for this attribute, so it can be used along with k8s.pod.uid and container.name to fetch a particular container status in this issue.

Please let me know what do you think

@sumo-drosiek
Copy link
Member Author

@dmitryax for me it sounds good. more intuitive than instance/ instanceNumber/run_id

@dmitryax
Copy link
Member

Ok, I submitted a PR to add this as a semantic convention open-telemetry/opentelemetry-specification#1945

@tigrannajaryan
Copy link
Member

open-telemetry/opentelemetry-specification#1945 resolved the run_id attribute.

open-telemetry/opentelemetry-specification#1980 is open to clarify container.name and k8s.container.name. Please review/comment.

The stream attribute is not addressed at all yet.

@djaglowski
Copy link
Member

This issue proposes a place for stream attribute: open-telemetry/opentelemetry-specification#1772

@dmitryax
Copy link
Member

I think we can close this issue since all the attributes now added to the OTel conventions cc @tigrannajaryan @djaglowski

@tigrannajaryan
Copy link
Member

Closing. Feel free to reopen if there is anything else to be done or create a new issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants