[chore] update wording of ecs recommendations #1272
Conversation
| - We should aim to avoid using an existing ECS name as a namespace whenever | ||
| possible and feasible. However, in critical situations where no other good | ||
| option exists, such changes might be necessary. If the name must differ, |
There was a problem hiding this comment.
based on the slack discussion, it seems this limitation can be removed altogether. What would be the reason to keep it?
There was a problem hiding this comment.
To make ecs compatibility with otel easier. This whole chapter was added for this goal - to consider existing ecs namespaces and attributes while advancing on otel during donation of ECS, we should aim to avoid this conflict as we do with other breaking changes.
There was a problem hiding this comment.
I'm lost
Felix Barnsteiner 1 day ago
At Elastic, we’re soon moving to a model where we store OTel attributes as-is, and with a flat structure that avoids causing ingestion issues in these situations. There will be optional runtime/alias mappings back to ECS. So I don’t really see the issue here and why a change like this would be a breaking one.
Felix Barnsteiner 1 day ago
Changing file.owner to file.owner.id is a breaking change but it’s not worse than changing it to file.owner_id, for instance.
I may be missing some context here, and maybe there are some nuances like this making an ECS->OTel compatibility layer more challenging. Although these seem like technical challenges that we could overcome.
I'm reading this and the impression I get is that conflict with namespace is not a problem. Breaking change is, but conflict with namespace does not introduce new problems on top of breaking.
So if I'm reading this correctly, there is no reason to keep the limitation and previous points in this doc already advise against breaking changes to ECS. What am I missing?
There was a problem hiding this comment.
If we take the whole paragraph it is about preventing breaking changes in ecs. The first point is about avoiding breaking changes in attribute naming and second is avoiding breaking changes in attribute/namespace collisions. Those are different sorts of breaking changes and we should try to avoid them both if possible.
We also have the same rule (stricter one) for semconv itself:
Names SHOULD NOT coincide with namespaces. For example if service.instance.id is an attribute name then it is no longer valid to have an attribute named service.instance because service.instance is already a namespace. Because of this rule be careful when choosing names: every existing name prohibits existence of an equally named namespace in the future, and vice versa: any existing namespace prohibits existence of an equally named attribute key in the future.
I want to emphasise both cases therefore I have left both options and changed the wording
There was a problem hiding this comment.
second is avoiding breaking changes in attribute/namespace collisions.
but it seems there are no problem with attribute and namespace collisions in ECS, so why do we need this sentence?
There was a problem hiding this comment.
It's true that it won't be a problem in the future but it's mostly a safety net to ensure a smoother transition of ECS users to the new schema. Note the following:
we’re soon moving to a model where we store OTel attributes as-is.
Specially the soon.
As I see it it's mostly a "soft" guidance to ensure that whenever a breaking-change/conflict of this type can be avoided there is this guidance to suggest it.
If we have a case where there is no strong preference between 2 names, then this ECS guidance could be applied.
There was a problem hiding this comment.
but it seems there are no problem with attribute and namespace collisions in ECS
this is still a problem and a breaking change as with attribute name changes. It's just not as severe as it was before
There was a problem hiding this comment.
I still don't understand why it is a problem. Based on the discussion in the chat it's not or it won't be soon.
|
This PR was marked stale due to lack of activity. It will be closed in 7 days. |
|
Closed as inactive. Feel free to reopen if this PR is still being worked on. |
After recent discussion we decided to soften a bit recommendation on not using an existing ECS name as a namespace
Merge requirement checklist
[chore]