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

Resource/Entity merge logic prevents fine-grained detectors #4266

Open
tigrannajaryan opened this issue Oct 18, 2024 · 1 comment
Open

Resource/Entity merge logic prevents fine-grained detectors #4266

tigrannajaryan opened this issue Oct 18, 2024 · 1 comment
Labels
entities sig-issue A specific SIG should look into this before discussing at the spec

Comments

@tigrannajaryan
Copy link
Member

In Go SDK currently Resource detectors are fine grained with multiple detectors detecting portions of what in fact constitutes a single entity. For example there are separate host and hostid detectors. host detects only the host.name attribute while hostid detects the host.id attribute.

Attempting to refactor these detectors in a straightforward way, such that each existing Resource detector becomes an Entity detector is unsuccessful because of how the entity merging rules are defined. Each of these detectors detects a "host" entity, and each needs to use some attributes as the Id of the entity however they naturally use different Ids. The net result of trying to use host and hostid detectors at the same time is that one of the detectors takes priority over the over because of this merging rule:

If the entity identity is different: drop the new entity d'.

So, only one of these detector's attributes are effective, the other gets dropped.

It is unclear how to overcome this problem.

One possible way is to combine these 2 detectors into just one detector of "host" type which will detect both host.name and host.id attributes. However, this means a breaking change in the SDK since these detectors are part of public API.

Another way is to introduce a new host detector which will do this new detection job while keeping the old ones for backward compatibility purposes. The downside is that existing end-user code will not automatically benefit from upgrading to a newer SDK version.

Yet another way is to rethink the merging rules so that we allow multiple detectors of the same entity type and their results are merged. This would allow to keep the existing detectors' set intact, keep the API the same and allow existing end-user code to automatically gain entity-awareness by simply upgrading to a newer SDK version. It is not clear though if a reasonable merging rule exists.

@tigrannajaryan tigrannajaryan converted this from a draft issue Oct 18, 2024
@tigrannajaryan
Copy link
Member Author

As an example output with merging rules implemented according to OTEP (i.e. "If the entity identity is different: drop the new entity d'.") and with Host and HostId detectors:

                "EntityRefs": [
                        {
                                "Type": "host",
                                "Id": [
                                        "host.id"
                                ],
                                "Attrs": [],
                                "SchemaURL": "https://opentelemetry.io/schemas/1.24.0"
                        },
                ]

As you can see the "Attrs" are empty, basically the "Host" detector that had lower priority is completely ignored.

However if we change the rule to merge both "host" entities instead of dropping one we get this:

                "EntityRefs": [
                        {
                                "Type": "service",
                                "Id": [
                                        "service.name",
                                        "service.version"
                                ],
                                "Attrs": null,
                                "SchemaURL": "https://opentelemetry.io/schemas/1.24.0"
                        },
                        {
                                "Type": "host",
                                "Id": [
                                        "host.id"
                                ],
                                "Attrs": [
                                        "host.name"
                                ],
                                "SchemaURL": "https://opentelemetry.io/schemas/1.24.0"
                        },
                ]

Now both detector's output is merged and we get both a correct Id and an additional descriptive attribute.

@danielgblanco danielgblanco added the sig-issue A specific SIG should look into this before discussing at the spec label Oct 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
entities sig-issue A specific SIG should look into this before discussing at the spec
Projects
Status: Todo
Development

No branches or pull requests

2 participants