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

RFC Stage 1: TSDB Dimensions #2217

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

agithomas
Copy link
Contributor

Have you signed the contributor license agreement? yes
Have you followed the contributor guidelines? yes
For proposing substantial changes or additions to the schema, have you reviewed the [RFC process] (https://github.com/elastic/ecs/blob/main/rfcs/README.md)? yes
If submitting code/script changes, have you verified all tests pass locally using make test?
If submitting schema/fields updates, have you generated new artifacts by running make and committed those changes?
Is your pull request against main? Unless there is a good reason otherwise, we prefer pull requests against main and will backport as needed.
Have you added an entry to the CHANGELOG.next.md?

@agithomas agithomas requested a review from a team as a code owner June 8, 2023 18:04
@agithomas
Copy link
Contributor Author

agithomas commented Jun 8, 2023

From Stage 0 to Stage 1, fields chosen as dimension fields were re-assessed here.

Stage 1 thus has

Three additions : cloud.account.id, cloud.region, cloud.availability_zone and
One deletion : cloud.project.id

in comparison with what is initially estimated in Stage 0.

No similar changes are expected in near future.

@agithomas
Copy link
Contributor Author

Related Stage 0 RFC

Copy link
Member

@ruflin ruflin left a comment

Choose a reason for hiding this comment

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

Overall LGTM. @lalit-satapathy and team, please dive into the details of each field.

One general comment: There is currently a merger ongoing with semconv and in otel everything is a dimension which currently does not work with Elasticsearch. Before a lengthy discussion pops up around should this be in ECS or not, I would like to side step this and keep this PR moving.

There needs to be a more general discussion to have how we store additional info specific to Elasticsearch related to ECS in the future merged schema (or besides it). Lets do it in a separate issue.

@agithomas
Copy link
Contributor Author

agithomas commented Jun 9, 2023

Reasoning behind inclusions of fields as dimensions and changes from Stage 0 to Stage 1 are mentioned below

Field name Explanation/reasoning
host.name It is a host where the agent is running. Mainly used for cases when integration is installed on-prem. Note: we are not using host.id since it might be not unique
service.address This field is present in case we provide a concrete target for scraping, like IP:PORT of some service (like mongodb), in some cases - it might be not needed to use this field
container.id For now it is mainly used for: docker/containerd/kubernetes packages, this field is empty for other integrations. Container.id in this case is an id of the monitored container
cloud.account.id (new) id used to identify different entities in a multi-tenant environment.
cloud.provider To avoid minimal chance that account.id might be the same for different providers
cloud.region (new) For services that are region specific
cloud.availability_zone (new) For services that are zone specific
cloud.instance.id host.name (can be defined manually by customer) is not unique enough. for Azure - instance.id is globally unique, AWS - region, GCP - availability zone. Technically for azure for example it would be enough to define cloud.instance.id only, but since it should be unified we include all fields: cloud.region/zone
agent.id For cases when 2 data shipers are monitoring the same resource
cloud.project.id (deleted)  This value is empty with AWS and Azure cloud providers. cloud.account.id is hence considered

@rameshelastic
Copy link

@agithomas , @lalit-satapathy Has a separate issue been created for this around the merged schema? Is there value in keeping this issue open anymore?

@github-actions
Copy link

This PR is stale because it has been open for 60 days with no activity.

@github-actions github-actions bot added the stale Stale issues and pull requests label Aug 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
RFC:candidate RFC stale Stale issues and pull requests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants