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

Add spec resource container attributes #3468

Conversation

marcsanmi
Copy link

Changes

Update the container semantic conventions to add a couple of non-default resource attributes introduced in open-telemetry/opentelemetry-collector-contrib#21185.

@marcsanmi marcsanmi force-pushed the add-spec-resource-container-attributes branch from 23c6eb2 to d2a1134 Compare May 4, 2023 09:04
@reyang reyang added the area:semantic-conventions Related to semantic conventions label May 9, 2023
@reyang
Copy link
Member

reyang commented May 9, 2023

@marcsanmi heads up - most likely this PR will be closed, and we'll ask you to resubmit the PR in a new repo, please refer to #3474 (comment).

@arminru arminru added the spec:resource Related to the specification/resource directory label May 9, 2023
@@ -11,7 +11,9 @@
|---|---|---|---|---|
| `container.name` | string | Container name used by container runtime. | `opentelemetry-autoconf` | Recommended |
| `container.id` | string | Container ID. Usually a UUID, as for example used to [identify Docker containers](https://docs.docker.com/engine/reference/run/#container-identification). The UUID might be abbreviated. | `a3bf90e006b2` | Recommended |
| `container.command_line` | string | The full command executed by the container. | `nginx -g daemon off;` | Recommended |
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could this contain any credentials that would need to be redacted?

Copy link
Author

@marcsanmi marcsanmi May 11, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. Yes, this would contain any command (+args) the container runs. By need to be redacted do you mean writing some advice in the Description column?

It's worth mentioning that this resource attribute will be disabled by default. Let me know your thoughts.

Note: considering the first comment, I'll make the changes in the new repository.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Anything added regarding that here should probably added also to the process.* semantic conventions. And I wonder if these could somehow be shared. At least they should be aligned, so consider also adding a container.command_args string array. https://github.com/open-telemetry/opentelemetry-specification/blob/b0a22680de561c2a7d45cf78aa65c65583f42ed2/specification/resource/semantic_conventions/process.md#process

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You'll need to add these changes in the YAML definition from which these MD files are then generated.
See https://github.com/open-telemetry/opentelemetry-specification/tree/main/semantic_conventions for the YAMLs and the README there for instructions.

@joaopgrassi
Copy link
Member

@marcsanmi the move of semconv to a new repo happened. Could you please rebase and submit the PR there? https://github.com/open-telemetry/semantic-conventions

Copy link
Contributor

@jsuereth jsuereth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@github-actions
Copy link

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

@github-actions github-actions bot added the Stale label May 20, 2023
@github-actions
Copy link

Closed as inactive. Feel free to reopen if this PR is still being worked on.

@github-actions github-actions bot closed this May 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:semantic-conventions Related to semantic conventions spec:resource Related to the specification/resource directory Stale
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants