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.

Sign up for GitHub

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

Jump to bottom

Allow to collect host architecture #1483

Merged
merged 8 commits into from
Mar 5, 2021
Merged

Allow to collect host architecture #1483

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
933ba47
Allow to collect host architecture
rakyll Mar 1, 2021
f3115a0
Add a link to the change
rakyll Mar 1, 2021
dc3fdce
Merge branch 'main' into os-arch
rakyll Mar 2, 2021
f6df42a
Add predefined architectures
rakyll Mar 3, 2021
3cf01a1
Switch to uname style arch names
rakyll Mar 3, 2021
cd048f9
Remove other
rakyll Mar 4, 2021
49fb678
Merge branch 'main' into os-arch
carlosalberto Mar 4, 2021
7a98082
Merge branch 'main' into os-arch
carlosalberto Mar 5, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ release.
([#1439](https://github.com/open-telemetry/opentelemetry-specification/pull/1439))
- Add [`ForceFlush`](https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/sdk.md#forceflush) to SDK's `TracerProvider` ([#1452](https://github.com/open-telemetry/opentelemetry-specification/pull/1452))
- Add `elasticsearch` to `db.system` semantic conventions ([#1463](https://github.com/open-telemetry/opentelemetry-specification/pull/1463))
- Add `arch` to `host` semantic conventions ([#1483](https://github.com/open-telemetry/opentelemetry-specification/pull/1483))
- Add `runtime` to `container` semantic conventions ([#1482](https://github.com/open-telemetry/opentelemetry-specification/pull/1482))
- Rename `gcp_gke` to `gcp_kubernetes_engine` to have consistency with other
Google products under `cloud.infrastructure_service` ([#1496](https://github.com/open-telemetry/opentelemetry-specification/pull/1496))
Expand Down
27 changes: 27 additions & 0 deletions semantic_conventions/resource/host.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,33 @@ groups:
brief: >
Type of host. For Cloud, this must be the machine type.
examples: ['n1-standard-1']
- id: arch
type:
allow_custom_values: true
members:
- id: amd64
value: 'amd64'
brief: "AMD64"
- id: arm32
value: 'arm32'
brief: "ARM32"
- id: arm64
value: 'arm64'
brief: "ARM64"
- id: ia64
value: 'ia64'
brief: "Itanium"
- id: ppc32
value: 'ppc32'
brief: "32-bit PowerPC"
- id: ppc64
value: 'ppc64'
brief: "64-bit PowerPC"
- id: x86
value: 'x86'
brief: "32-bit x86"
brief: >
The CPU architecture the host system is running on.
anuraaga marked this conversation as resolved.
Show resolved Hide resolved
carlosalberto marked this conversation as resolved.
Show resolved Hide resolved
- id: image.name
type: string
brief: >
Expand Down
13 changes: 13 additions & 0 deletions specification/resource/semantic_conventions/host.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,20 @@
| `host.id` | string | Unique host ID. For Cloud, this must be the instance_id assigned by the cloud provider. | `opentelemetry-test` | No |
| `host.name` | string | Name of the host. On Unix systems, it may contain what the hostname command returns, or the fully qualified hostname, or another name specified by the user. | `opentelemetry-test` | No |
| `host.type` | string | Type of host. For Cloud, this must be the machine type. | `n1-standard-1` | No |
| `host.arch` | string | The CPU architecture the host system is running on. | `amd64` | No |
| `host.image.name` | string | Name of the VM image or OS install the host was instantiated from. | `infra-ami-eks-worker-node-7d4ec78312`; `CentOS-8-x86_64-1905` | No |
| `host.image.id` | string | VM image ID. For Cloud, this value is from the provider. | `ami-07b06b442921831e5` | No |
| `host.image.version` | string | The version string of the VM image as defined in [Version Attributes](README.md#version-attributes). | `0.1` | No |

`host.arch` MUST be one of the following or, if none of the listed values apply, a custom value:
Copy link
Contributor

Choose a reason for hiding this comment

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

Values here neither match the common convention for Linux / Debian arch, e.g. not matching what's reported by uname -m, nor match what Node.js os.arch() API would have reported; nor what Rust architecture uses for the standard set of arch.... For different languages - you'd have different strings. And none of them would match the list proposed here 😄 So it's going to be quite an adventure to normalize to that proposed list 🤷

I believe a better set could be this:

Architecture Applies to
x86 x86, wow64, i386, i486, etc.
amd64 x86_64, amd64, x64
arm32 arm32
arm64 arm64, aarch64
ppc32 32-bit PowerPC
ppc64 64-bit PowerPC
ia64 Itanium
other indistinguishable, or not listed here. Architecture as reported by the language of choice without normalizing it / without bringing it to common denominator.

C++, Rust, or Node - typically have their own API to obtain the process arch string. This string is going to be in their own common format. For Node - this list is well-described here: https://nodejs.org/api/os.html#os_os_arch - I personally believe Node's list is quite reasonable.

More complete set of architectures is described here in Linux kernel tree:
https://github.com/torvalds/linux/tree/master/arch

If you truly intend to describe the host arch , it would be reasonable to respect the Linux standard names for the archs, as the most prominent host OS. That's also what uname -m command would typically report, namely [arm|arm64|ia64|mips|x86]. I would've left the loophole in this list to allow reporting what OS language-specific API reports as architecture. Any data analysis within a slice of data reported by agents written in specific language is consistent: a language (e.g. node) would report only a fixed set of archs. There is also not going to be any need to normalize.

I don't like this proposed list because it does not seem to align with the arch lists typically captured by prominent language APIs, neither matches the Linux kernel arch list.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This looks so much better. Go has a similar approach here. I initially wanted to propose something more aligned with uname but the formatting of OS names made me want to follow the UPPER_CASE style here. I'm modifying this PR in a bit.


| Value | Description |
|---|---|
| `amd64` | AMD64 |
| `arm32` | ARM32 |
| `arm64` | ARM64 |
| `ia64` | Itanium |
| `ppc32` | 32-bit PowerPC |
| `ppc64` | 64-bit PowerPC |
| `x86` | 32-bit x86 |
<!-- endsemconv -->