-
Notifications
You must be signed in to change notification settings - Fork 966
Define an Entity Merge algorithm (from OTEP #264) #4768
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
Open
jsuereth
wants to merge
14
commits into
open-telemetry:main
Choose a base branch
from
jsuereth:wip-entity-merge
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
14 commits
Select commit
Hold shift + click to select a range
d56d488
Initial entity merge specification.
jsuereth ee98376
markdown toc.
jsuereth 1c6b4bb
Fix lint.
jsuereth 287ce5b
Fix spelling mistake and update note on merge algorithm.
jsuereth 538a697
Fix algorithm to not use d' from teh OTEP.
jsuereth e0af885
Add handling for flattening resources.
jsuereth 0ccf603
Fix lint issues.
jsuereth de9dd47
Add changelog.
jsuereth 7d43b4b
Merge branch 'main' into wip-entity-merge
jsuereth 778e1a6
Merge remote-tracking branch 'origin/main' into wip-entity-merge
jsuereth ec93cd1
Fixes from review.
jsuereth 8fccbeb
Merge remote-tracking branch 'origin/main' into wip-entity-merge
jsuereth db998c8
Update specification/entities/data-model.md
jsuereth 3a82999
Fixes from review. Add examples, clarify some sections.
jsuereth File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,6 +13,8 @@ weight: 2 | |
| <!-- toc --> | ||
|
|
||
| - [Identity](#identity) | ||
| - [Merging Resources](#merging-resources) | ||
| * [Merging Entities into a Resource](#merging-entities-into-a-resource) | ||
|
|
||
| <!-- tocstop --> | ||
|
|
||
|
|
@@ -44,6 +46,149 @@ Entity includes its own notion of identity. The identity of a resource is | |
| the set of entities contained within it. Two resources are considered | ||
| different if one contains an entity not found in the other. | ||
|
|
||
| Some resources include raw attributes in additon to Entities. Raw attributes are | ||
| considered identifying on a resource. That is, if the key-value pairs of | ||
| Some resources include raw attributes in addition to Entities. Raw attributes | ||
| are considered identifying on a resource. That is, if the key-value pairs of | ||
| raw attributes are different, then you can assume the resource is different. | ||
|
|
||
| ## Merging Resources | ||
|
|
||
| Note: The current SDK specification outlines a [merge algorithm](sdk.md#merge). | ||
| This specification updates the algorithm to be compliant with entities. This | ||
| section will replace that section upon stabilization of entities. SDKs SHOULD | ||
| NOT update their merge algorithm until full Entity SDK support is provided. | ||
|
|
||
| Merging resources is an action of joining together the context of observation. | ||
| That is, we can look at the resource context for a signal and *expand* that | ||
| context to include more details (see | ||
| [telescoping identity](README.md#telescoping)). As such, a merge SHOULD preserve | ||
| any identity that already existed on a Resource while adding in new identifying | ||
| information or descriptive attributes. | ||
|
|
||
| ### Merging Entities into a Resource | ||
|
|
||
| We define the following algorithm for merging entities into an existing | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a fairly complicated algorithm. Can you please add a few examples to demonstrate what it is doing?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added |
||
| resource. | ||
|
|
||
| - Construct a set of existing entities on the resource, `E`. | ||
| - For each entity, `new_entity`, in priority order (highest first), | ||
dashpole marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| do one of the following: | ||
| - If an entity `e` exists in `E` with the same entity type as `new_entity`: | ||
| - Perform an [Entity DataModel Merge](../entities/data-model.md#merging-of-entities) with `e` and `new_entity` | ||
| - Note: If unable to merge `e` and `new_entity`, then no change is made. | ||
| - Otherwise, add the entity `new_entity` to set `E` | ||
| - Update the Resource to use the set of entities `E`. | ||
| - If all entities within `E` have the same `schema_url`, set the | ||
| resources `schema_url` to match. | ||
| - Otherwise set the Resource `schema_url` blank. | ||
| - Remove any attribute from `Attributes` which exists in either the | ||
| description or identity of an entity in `E`. | ||
dmitryax marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| - Solve for resource flattening issues (See | ||
| [Attribute Referencing Model](../entities/data-model.md#attribute-referencing-model)). | ||
| - If, for all entities, there are now overlapping attribute keys, then nothing | ||
| is needed. | ||
| - If there is a conflict where two entities use the same attribute key then | ||
| remove the lower priority entity from the Resource. | ||
|
|
||
| #### Examples | ||
|
|
||
| _These examples demonstrate how conflicts are resolved during a merge._ | ||
|
|
||
| ##### Example 1: Entity replaces loose attribute | ||
|
|
||
| The conflict between loose attributes and those belonging to an entity. Here when entity is added it removes previous attributes. | ||
|
|
||
| **Initial Resource:** | ||
| - Entities: _None_ | ||
| - Attributes: | ||
| - `host.name`: `"old-name"` | ||
| - `env`: `"prod"` | ||
|
|
||
| **Entities to Merge (by priority):** | ||
| 1. `host` | ||
| - type: `"host"` | ||
| - identity: | ||
| - `host.id`: `"H1"` | ||
| - description: | ||
| - `host.name`: `"new-name"` | ||
| 2. `service` | ||
| - type: `"service"` | ||
| - identity: | ||
| - `service.name`: `"my-svc"` | ||
|
|
||
| **Resulting Resource:** | ||
| - Entities: | ||
| - `host` | ||
| - type: `"host"` | ||
| - identity: | ||
| - `host.id`: `"H1"` | ||
| - description: | ||
| - `host.name`: `"new-name"` | ||
| - `service` | ||
| - type: `"service"` | ||
| - identity: | ||
| - `service.name`: `"my-svc"` | ||
| - Attributes: | ||
| - `env`: `"prod"` | ||
|
|
||
| ##### Example 2: Loose attribute replaces entity attribute | ||
|
|
||
| The conflict between loose attributes and those belonging to an entity. Here when the loose attribute is added, the entity must be removed due to conflict. | ||
|
|
||
| **Initial Resource:** | ||
| - Entities: | ||
| - `host` | ||
| - type: `"host"` | ||
| - identity: | ||
| - `host.id`: `"H1"` | ||
| - description: | ||
| - `host.name`: `"detected-name"` | ||
| - Attributes: _None_ | ||
|
|
||
| **Resource to Merge:** | ||
| - Entities: _None_ | ||
| - Attributes: | ||
| - `host.id`: `"h2"` | ||
| - `env`: `"prod"` | ||
|
|
||
| **Resulting Resource:** | ||
| - Entities: _None_ | ||
| - Attributes: | ||
| - `host.id`: `"h2"` | ||
| - `env`: `"prod"` | ||
|
|
||
| ##### Example 3: Identity & Attribute Conflicts | ||
|
|
||
| Reject an entity with a different identity of the same type, and drop a lower priority entity due to an attribute key conflict. | ||
|
|
||
| **Initial Resource:** | ||
| - Entities: | ||
| - `host` | ||
| - type: `"host"` | ||
| - identity: | ||
| - `host.id`: `"H1"` | ||
| - description: | ||
| - `env`: `"prod"` | ||
| - Attributes: _None_ | ||
|
|
||
| **Entities to Merge (by priority):** | ||
| 1. `host` | ||
| - type: `"host"` | ||
| - identity: | ||
| - `host.id`: `"H2"` | ||
| 2. `service` | ||
| - type: `"service"` | ||
| - identity: | ||
| - `service.name`: `"S1"` | ||
| - description: | ||
| - `env`: `"dev"` | ||
|
|
||
| **Resulting Resource:** | ||
| - Entities: | ||
| - `host` | ||
| - type: `"host"` | ||
| - identity: | ||
| - `host.id`: `"H1"` | ||
| - description: | ||
| - `env`: `"prod"` | ||
| - Attributes: _None_ | ||
|
|
||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you see a possibility to relax the last requirement a bit? For example can we aim for: "schema_url is the same or schemas are compatible", where "schemas are compatible" if attributes used by Entities are unchanged between schema versions? I understand it is more work to check compatibility but it makes merging much more widely possible.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So in that event, we expect the following:
So while we do want to allow schema_url based conversions, the merge algorithm for Entities can be defined much more simply and we leave room for version-conversion prior to this.