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

Container semantic conventions: follow Open Container Initiative spec #48

Closed
lmolkova opened this issue May 23, 2023 · 5 comments
Closed
Assignees
Labels
enhancement New feature or request

Comments

@lmolkova
Copy link
Contributor

lmolkova commented May 23, 2023

Container resource attributes declare a few image-specific attributes (e.g. container.image.tag). Images and artifacts are not strictly related to containers. OCI defines image (artifact, etc) spec that we should follow.

We should revise container resource attributes and align them with OCI Image manifest.

@lmolkova lmolkova changed the title container semantic conventions: follow Open Container semantic conventions: follow Open Container Initiative spec May 23, 2023
@lmolkova lmolkova added the enhancement New feature or request label May 23, 2023
@ChrsMark
Copy link
Member

ChrsMark commented Jun 21, 2023

Hey @lmolkova! I had a look into this one and I think it will be a good idea to provide the flexibility for adding more than one image tags. Same would apply to the image.hash/id as I mention at https://github.com/open-telemetry/semantic-conventions/pull/39/files#r1236511385. This would mean that image.tag will be an array of strings and would be capable of storing more than one container image tags.

That would align with the OCI spec and would allow storing more than one tags or hashes/digests in case one want to work on image-object level and not on container level.

Example:

~ docker inspect --format='{{.RepoDigests}}' $IMAGE
[prom/prometheus@sha256:e4ca62c0d62f3e886e684806dfe9d4e0cda60d54986898173c1083856cfda0f4]~ docker inspect --format='{{.RepoTags}}' $IMAGE   
[prom/prometheus:v2.16.0 prom/prometheus:v2.16.0-rc42]

On container level the Image field of the Config section looks like:

~ docker inspect --format='{{.Config.Image}}' 1b507aebbd3d 
prom/prometheus:v2.16.0@sha256:e4ca62c0d62f3e886e684806dfe9d4e0cda60d54986898173c1083856cfda0f4

Also for k8s it's pretty much straight forward since we have sth like the following:

    Image:         registry.k8s.io/etcd:3.5.7-0
    Image ID:      sha256:86b6af7dd652c1b38118be1c338e9354b33469e69a218f7e290a0ca5304ad681

In that case the image.tag is 3.5.7-0 and the image.id is sha256:86b6af7dd652c1b38118be1c338e9354b33469e69a218f7e290a0ca5304ad681.

In addition exec-ing into a k8s node and running crictl image --verbose I get:

ID: sha256:431553d4c70b50f59fa9a57a1136e5833d2700210cda6c5f67770c982a8307f2
RepoTags: registry.k8s.io/kube-proxy:v1.27.1
RepoDigests: docker.io/library/import-2023-05-12@sha256:a9d9b674a55b137b1f10bbcd7f0e43b7568d20c8df5bf969d1bb8a37f3d365db
Size: 72702973

This makes me understand that:

  1. tags can be more than one but usually it's only one
  2. hash/digest is a different thing than the image ID
  3. hash/digest can also be more than one.

So to summarize I would suggest converting container.image.tag into an array of strings and follow a similar approach for the hash/digest part at #39. For image.id it seems that we are ok to go for a single value which is actually the local ID of the image. This would give us the flexibility to cover k8s, docker and OCI at the same time.
LMK what you think :).

Proposal:

container.image.tag: string[]
container.image.hash: string[]
container.image.id: string

@ChrsMark
Copy link
Member

ChrsMark commented Jun 30, 2023

Since #39 was merged let's try to move this one forward too and specifically the container.image.* part.

To summarize the situation here I think we should go for something like the following:

container.image.tag: string[]
container.image.digest: string[]
container.image.id: string

Docker ✅

This is aligned with Docker where an inspect of an image would give sth like the following:

{
  "Id": "sha256:ec3f0931a6e6b6855d76b2d7b0be30e81860baccd891b2e243280bf1cd8ad710",
  "RepoTags": [
  "example:1.0",
  "example:latest",
  "example:stable",
  "internal.registry.example.com:5000/example:1.0"
  ],
  "RepoDigests": [
  "example@sha256:afcc7f1ac1b49db317a7196c902e61c6c3c4607d63599ee1a82d702d249a0ccb",
  "internal.registry.example.com:5000/example@sha256:b69959407d21e8a062e0416bf13405bb2b71ed7a84dde4158ebafacfa06f5578"
  ],
  ...
}

OCI manifest ✅

This is also aligned with https://github.com/opencontainers/image-spec/blob/main/manifest.md which indicates that there are multiple digests out of the many layers an image is built.

k8s ✅

At the same time it's also aligned with what k8s report as I mention in my examples at #48 (comment) and according to the k8s docs:

Example:

Containers:
  etcd:
    Container ID:  containerd://6c531cfb94115489f86783cc6b11712f604a7d82c3b4693327ea1106e5455d13
    Image:         registry.k8s.io/etcd:3.5.7-0
    Image ID:      sha256:86b6af7dd652c1b38118be1c338e9354b33469e69a218f7e290a0ca5304ad681

CRI ✅

The the Container Runtime Interface (CRI) of k8s also (as expected :)) follow the OCI spec and hence this proposal is alligned with this one too: https://github.com/kubernetes/cri-api/blob/c75ef5b473bbe2d0a4fc92f82235efd665ea8e9f/pkg/apis/runtime/v1/api.proto#L1234-L1238

Example:

ID: sha256:431553d4c70b50f59fa9a57a1136e5833d2700210cda6c5f67770c982a8307f2
RepoTags: registry.k8s.io/kube-proxy:v1.27.1
RepoDigests: docker.io/library/import-2023-05-12@sha256:a9d9b674a55b137b1f10bbcd7f0e43b7568d20c8df5bf969d1bb8a37f3d365db
Size: 72702973

So @joaopgrassi @lmolkova @marcsanmi let me know your thoughts on this and if we agree I can file a PR with this changes.

Respective issue to tune ECS accordingly: elastic/ecs#2230

@ChrsMark
Copy link
Member

ChrsMark commented Jul 4, 2023

Opened #159 to implement the above proposal.

@ChrsMark
Copy link
Member

@lmolkova @joaopgrassi is there anything pending here or we can consider it as completed?

@joaopgrassi
Copy link
Member

Looking at the current state https://github.com/open-telemetry/semantic-conventions/blob/main/docs/attributes-registry/container.md it seems we have it all defined as initially wanted, so maybe this is completed. We can always re-open a new one if we need.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants