Add fallback for host.id in Cloud #576

christos68k · 2023-12-01T18:29:23Z

This PR changes the semantics behind host.id calculation for Cloud.

Changes

In Cloud, it is possible for instance_id to not be retrievable from inside the instance (e.g. security concerns / restricted instance metadata). In this case, the host.id can be set from machine-id (same steps as non-containerized systems).

Without a fallback, the must wording has strong implications for clients and users in cases where the instance_id can not be automatically retrieved:

Do not automatically populate host.id
Urge the user to either provide instance_id or change the configuration to make it retrievable

A (breaking) alternative to the current approach is to always populate host.id from machine-id, and use a different field for instance_id (e.g. service.instance.id)

Merge requirement checklist

CONTRIBUTING.md guidelines followed.
CHANGELOG.md updated for non-trivial changes.
schema-next.yaml updated with changes to existing conventions.

AlexanderWert · 2023-12-02T14:22:02Z

@christos68k please do the change in the corresponding yaml file:

semantic-conventions/model/registry/host.yaml

Line 8 in ed7d3e0

- id: id

The tables in markdown files are generated from the model yaml files. So once you did the change in the yaml file you need to run: make table-generation. This will automatically update the tables in the markdown files.

AlexanderWert · 2023-12-02T14:25:40Z

@open-telemetry/semconv-system-approvers @open-telemetry/semconv-container-approvers PTAL

mx-psi · 2023-12-04T11:12:25Z

docs/attributes-registry/host.md

@@ -14,7 +14,7 @@
 | `host.cpu.model.name` | string | Model designation of the processor. | `11th Gen Intel(R) Core(TM) i7-1185G7 @ 3.00GHz` |
 | `host.cpu.stepping` | int | Stepping or core revisions. | `1` |
 | `host.cpu.vendor.id` | string | Processor manufacturer identifier. A maximum 12-character string. [1] | `GenuineIntel` |
-| `host.id` | string | Unique host ID. For Cloud, this must be the instance_id assigned by the cloud provider. For non-containerized systems, this should be the `machine-id`. See the table below for the sources to use to determine the `machine-id` based on operating system. | `fdbf79e8af94cb7f9e8df36789187052` |
+| `host.id` | string | Unique host ID. For Cloud, this must be the instance_id assigned by the cloud provider. If the instance_id can not be retrieved, this should be the `machine-id`. For non-containerized systems, this should be the `machine-id`. See the table below for the sources to use to determine the `machine-id` based on operating system. | `fdbf79e8af94cb7f9e8df36789187052` |


Overloading the meaning of host.id makes it unusable for some use cases. Right now, if cloud.platform is set to aws_ec2 and host.id is present, I know that host.id is the EC2 instance id. After this change, it would be impossible for me to know what host.id refers to. Knowing it is the EC2 instance id is very important for correlation across signals because it allows us to tell that the resource refers to the same machine.

Maybe the solution is to follow the steps of GCP which introduced gcp.gce.instance.name and have a similar one for each cloud provider, but until that happens I don't think we should accept this change.

[...] very important for correlation across signals because it allows us to tell that the resource refers to the same machine.

Exactly! And correlation is not possible at all if host.id cannot be retrieved and stays empty.
I think there should be an unambiguous algorithm (similar to #312) to derive the host.id in a way that providing a value is ALWAYS possible in order to enable correlation of signals. Things like the AWS EC2 instance id can be part of that, but assumptions like the following are implicit and therefore sort of unreliable:

Right now, if cloud.platform is set to aws_ec2 and host.id is present, I know that host.id is the EC2 instance id.

Wouldn't it be more appropriate to always have a host.id and set something like aws.ec2.instance_id when it is available?

I see main use cases for host.id as similar to service.instance.id, quoted from here:

The ID helps to distinguish instances of the same service that exist at the same time.

service.instance.id can be a K8s pod name, but that's not guaranteed. If I want that, I look at k8s.pod.name.

I'd advocate for a similar understanding of host.id, thus I'm in principle fine with the change proposed here.

Wouldn't it be more appropriate to always have a host.id and set something like aws.ec2.instance_id when it is available?

I am fine with that, but that convention does not exist today. I would like to add these conventions first and only then change the semantics of host.id. I am happy to help listing what conventions that would entail based on existing implementations of detectors on different SDKs and the resource detection processor on the Collector

I am happy to help listing what conventions that would entail based on existing implementations of detectors on different SDKs and the resource detection processor on the Collector

That'd be great! Thanks @mx-psi !

Will file an issue for this, but here are the uses of host.id that I was able to find that are not machine-id:

Azure VM ID (.NET contrib, Python contrib. Ruby contrib, Collector contrib)

Azure App Service's WEBSITE_HOSTNAME (Python contrib)

AWS EC2 instance id (PHP contrib, Collector contrib)

GKE Host id, GCE Host id (Go contrib, Ruby contrib, Collector contrib)

Consul node id (Collector)

@christos68k @AlexanderWert PTAL at #600. I want to start with this and see what approvers and maintainers of semconv think

github-actions · 2024-01-25T03:19:53Z

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

christos68k · 2024-01-29T16:42:18Z

Adding a comment to keep this PR from being closed.

jsuereth · 2024-02-12T14:58:50Z

docs/attributes-registry/host.md

@@ -14,7 +14,7 @@
 | `host.cpu.model.name` | string | Model designation of the processor. | `11th Gen Intel(R) Core(TM) i7-1185G7 @ 3.00GHz` |
 | `host.cpu.stepping` | int | Stepping or core revisions. | `1` |
 | `host.cpu.vendor.id` | string | Processor manufacturer identifier. A maximum 12-character string. [1] | `GenuineIntel` |
-| `host.id` | string | Unique host ID. For Cloud, this must be the instance_id assigned by the cloud provider. For non-containerized systems, this should be the `machine-id`. See the table below for the sources to use to determine the `machine-id` based on operating system. | `fdbf79e8af94cb7f9e8df36789187052` |
+| `host.id` | string | Unique host ID. For Cloud, this must be the instance_id assigned by the cloud provider. If the instance_id can not be retrieved, this should be the `machine-id`. For non-containerized systems, this should be the `machine-id`. See the table below for the sources to use to determine the `machine-id` based on operating system. | `fdbf79e8af94cb7f9e8df36789187052` |


Nit:

If the instance_id can not be retrieved, this should be the `machine-id`. For non-containerized systems, this should be the `machine-id`.

Should be rewritten for better clarity.

If the instance_id can not be retrieved, or for non-containerized systems, this should be the `machine-id`.

mx-psi

Filed #739 for the general issue I think needs to be solved before accepting this (see also the unresolved thread above)

github-actions · 2024-02-29T03:20:01Z

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

github-actions · 2024-03-08T03:18:52Z

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

christos68k · 2024-03-26T19:49:40Z

Could someone with rights (@mx-psi @AlexanderWert maybe?) to do so reopen this pull request?

ChrsMark · 2024-03-27T10:57:16Z

@christos68k I have reopened this one. You will need to update the changelog entry to follow the new format: https://github.com/open-telemetry/semantic-conventions/blob/main/CONTRIBUTING.md#adding-a-changelog-entry

If the instance_id can not be retrieved, use machine-id value.

github-actions · 2024-04-19T03:20:07Z

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

github-actions · 2024-05-08T03:20:03Z

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

jsuereth · 2024-05-16T15:08:57Z

Given activity happening in Entity SIG - I think it's best to "wait" on major changes to resource related attributes. My own opinion on this Pr is that:

Yes, using machine id makes sense as the default for host.id
Having cloud-specific ids land in host.id seems problematic from a consistency/implementation standpoint. We need to define mechanisms where we would understand how a cloud-provider Resource detector would interact with a Host-specific resource detector. This change would force the ability for us to understand merge conflcts and overrides between these two disjoint components.

As such - (while I think machine id should be the default for host.id) I think we need to hold off on this PR until more progress has been made in Entity SiG - particularly the mapping between Resource <-> Entity and conflict resolution.

github-actions · 2024-06-01T03:20:30Z

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

github-actions · 2024-06-09T03:20:13Z

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

christos68k requested review from a team December 1, 2023 18:29

github-actions bot assigned jsuereth Dec 1, 2023

christos68k added a commit to christos68k/semantic-conventions that referenced this pull request Dec 1, 2023

Add changelog entry for open-telemetry#576

ce0e074

mx-psi reviewed Dec 4, 2023

View reviewed changes

christos68k added a commit to christos68k/semantic-conventions that referenced this pull request Dec 4, 2023

Add changelog entry for open-telemetry#576

6b71321

christos68k force-pushed the ck/host.id branch from ce0e074 to 6b71321 Compare December 4, 2023 14:45

christos68k added a commit to christos68k/semantic-conventions that referenced this pull request Dec 4, 2023

Add changelog entry for open-telemetry#576

b750cb8

christos68k force-pushed the ck/host.id branch from 6b71321 to b750cb8 Compare December 4, 2023 14:49

christos68k added a commit to christos68k/semantic-conventions that referenced this pull request Dec 4, 2023

Add changelog entry for open-telemetry#576

e5d083b

christos68k force-pushed the ck/host.id branch from b750cb8 to e5d083b Compare December 4, 2023 14:51

mx-psi mentioned this pull request Dec 12, 2023

[cloud provider] Add AWS EC2 instance id semantic convention #600

Open

3 tasks

github-actions bot added the Stale label Jan 25, 2024

github-actions bot removed the Stale label Jan 30, 2024

jsuereth reviewed Feb 12, 2024

View reviewed changes

christos68k added a commit to christos68k/semantic-conventions that referenced this pull request Feb 12, 2024

Add changelog entry for open-telemetry#576

83d813b

christos68k force-pushed the ck/host.id branch from 6857a2d to 83d813b Compare February 12, 2024 19:37

christos68k added a commit to christos68k/semantic-conventions that referenced this pull request Feb 12, 2024

Add changelog entry for open-telemetry#576

ca8b12f

christos68k force-pushed the ck/host.id branch from 83d813b to ca8b12f Compare February 12, 2024 19:39

mx-psi mentioned this pull request Feb 13, 2024

[cloud provider] host.id semantics are too broad #739

Open

mx-psi reviewed Feb 13, 2024

View reviewed changes

github-actions bot added the Stale label Feb 29, 2024

ChrsMark mentioned this pull request Mar 5, 2024

Add missing ECS cloud fields to Semantic Conventions Cloud Resource attributes #761

Open

github-actions bot closed this Mar 8, 2024

ChrsMark reopened this Mar 26, 2024

github-actions bot removed the Stale label Mar 27, 2024

christos68k added a commit to christos68k/semantic-conventions that referenced this pull request Apr 3, 2024

Add changelog entry for open-telemetry#576

b660e22

christos68k force-pushed the ck/host.id branch from ca8b12f to b660e22 Compare April 3, 2024 17:28

christos68k added 2 commits April 3, 2024 13:32

Add fallback for host.id in Cloud

95e76f6

If the instance_id can not be retrieved, use machine-id value.

Add changelog entry for open-telemetry#576

a45bbc2

christos68k force-pushed the ck/host.id branch from b660e22 to a45bbc2 Compare April 3, 2024 17:34

christos68k mentioned this pull request Apr 3, 2024

RFC: Evolve host.id into a default, always present attribute (rethinking machine-id) #581

Open

github-actions bot added the Stale label Apr 19, 2024

ChrsMark removed the Stale label Apr 22, 2024

christos68k mentioned this pull request Apr 24, 2024

REQUEST: New membership for christos68k open-telemetry/community#2068

Closed

6 tasks

github-actions bot added the Stale label May 8, 2024

jsuereth removed the Stale label May 13, 2024

github-actions bot added the Stale label Jun 1, 2024

github-actions bot closed this Jun 9, 2024

lmolkova mentioned this pull request Jul 1, 2024

Add semantic convention for AWS ECS Service #1199

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add fallback for host.id in Cloud #576

Add fallback for host.id in Cloud #576

christos68k commented Dec 1, 2023 •

edited

Loading

AlexanderWert commented Dec 2, 2023 •

edited

Loading

AlexanderWert commented Dec 2, 2023

mx-psi Dec 4, 2023 •

edited

Loading

AlexanderWert Dec 4, 2023

pyohannes Dec 4, 2023 •

edited

Loading

mx-psi Dec 4, 2023

AlexanderWert Dec 4, 2023

mx-psi Dec 11, 2023

mx-psi Dec 12, 2023

github-actions bot commented Jan 25, 2024

christos68k commented Jan 29, 2024

jsuereth Feb 12, 2024

mx-psi left a comment

github-actions bot commented Feb 29, 2024

github-actions bot commented Mar 8, 2024

christos68k commented Mar 26, 2024 •

edited

Loading

ChrsMark commented Mar 27, 2024

github-actions bot commented Apr 19, 2024

github-actions bot commented May 8, 2024

jsuereth commented May 16, 2024

github-actions bot commented Jun 1, 2024

github-actions bot commented Jun 9, 2024

Add fallback for host.id in Cloud #576

Add fallback for host.id in Cloud #576

Conversation

christos68k commented Dec 1, 2023 • edited Loading

Changes

Merge requirement checklist

AlexanderWert commented Dec 2, 2023 • edited Loading

AlexanderWert commented Dec 2, 2023

mx-psi Dec 4, 2023 • edited Loading

Choose a reason for hiding this comment

AlexanderWert Dec 4, 2023

Choose a reason for hiding this comment

pyohannes Dec 4, 2023 • edited Loading

Choose a reason for hiding this comment

mx-psi Dec 4, 2023

Choose a reason for hiding this comment

AlexanderWert Dec 4, 2023

Choose a reason for hiding this comment

mx-psi Dec 11, 2023

Choose a reason for hiding this comment

mx-psi Dec 12, 2023

Choose a reason for hiding this comment

github-actions bot commented Jan 25, 2024

christos68k commented Jan 29, 2024

jsuereth Feb 12, 2024

Choose a reason for hiding this comment

mx-psi left a comment

Choose a reason for hiding this comment

github-actions bot commented Feb 29, 2024

github-actions bot commented Mar 8, 2024

christos68k commented Mar 26, 2024 • edited Loading

ChrsMark commented Mar 27, 2024

github-actions bot commented Apr 19, 2024

github-actions bot commented May 8, 2024

jsuereth commented May 16, 2024

github-actions bot commented Jun 1, 2024

github-actions bot commented Jun 9, 2024

christos68k commented Dec 1, 2023 •

edited

Loading

AlexanderWert commented Dec 2, 2023 •

edited

Loading

mx-psi Dec 4, 2023 •

edited

Loading

pyohannes Dec 4, 2023 •

edited

Loading

christos68k commented Mar 26, 2024 •

edited

Loading