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

Add a datacenter resource. #1413

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

hdost
Copy link

@hdost hdost commented Sep 17, 2024

The purpose of this is to allow for people running in a hybrid environment.

Relates #1409

Changes

Please provide a brief description of the changes here.

Note: if the PR is touching an area that is not listed in the existing areas, or the area does not have sufficient domain experts coverage, the PR might be tagged as experts needed and move slowly until experts are identified.

Merge requirement checklist

@hdost hdost requested review from a team September 17, 2024 12:25
@hdost hdost force-pushed the feat/1458-adding-datacenter-convention branch from 1ae444e to eb6b348 Compare September 17, 2024 12:58
docs/attributes-registry/datacenter.md Outdated Show resolved Hide resolved
model/datacenter/resources.yaml Outdated Show resolved Hide resolved
@hdost hdost requested review from a team as code owners September 22, 2024 16:23
model/datacenter/registry.yaml Outdated Show resolved Hide resolved
model/datacenter/registry.yaml Outdated Show resolved Hide resolved
model/datacenter/registry.yaml Outdated Show resolved Hide resolved
model/datacenter/registry.yaml Outdated Show resolved Hide resolved
model/datacenter/registry.yaml Outdated Show resolved Hide resolved
model/datacenter/registry.yaml Outdated Show resolved Hide resolved
model/datacenter/registry.yaml Outdated Show resolved Hide resolved
model/datacenter/resources.yaml Outdated Show resolved Hide resolved
@hdost hdost force-pushed the feat/1458-adding-datacenter-convention branch 3 times, most recently from 2369cb4 to 01b85e3 Compare September 22, 2024 20:50
@hdost hdost requested a review from jsuereth September 25, 2024 19:06
Copy link

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

@github-actions github-actions bot added the Stale label Oct 11, 2024
@hdost
Copy link
Author

hdost commented Oct 12, 2024

This is a bit of an anti pattern ... stale because i haven't received a review?

Also check failure is unrelated to this change.

@hdost hdost force-pushed the feat/1458-adding-datacenter-convention branch from 10b7e40 to 48cfd1c Compare October 12, 2024 14:38
@github-actions github-actions bot removed the Stale label Oct 13, 2024
<!-- NOTE: THIS FILE IS AUTOGENERATED. DO NOT EDIT BY HAND. -->
<!-- see templates/registry/markdown/attribute_namespace.md.j2 -->

# DC
Copy link
Member

Choose a reason for hiding this comment

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

Not sure it is a good idea to use the abbreviation here. I'd be more inclined to go with "Data Center" instead.

Comment on lines +23 to +36
- id: dc.site
type: string
stability: experimental
brief: 'Name of the datacenter site'
note: >
The name of the physical building.
examples: ['site-1']
- id: dc.suite
type: string
stability: experimental
brief: 'Name of the datacenter suite'
note: >
Typically the room in the building where the server cages are located.
examples: ['suite-5']
Copy link
Contributor

Choose a reason for hiding this comment

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

this is a good case to reuse future geo namespace #1116

cc @trisch-me @gregkalapos

I'd postpone adding those two since physical address is not a datacenter-specific thing and we need to be able to express it in a generic way.

Copy link
Author

@hdost hdost Oct 20, 2024

Choose a reason for hiding this comment

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

This is not necessarily related to geolocation it's relative to the building it's in.

stability: experimental
brief: 'Type of datacenter'
examples: ['cloud', 'colocation', 'internal']
- id: dc.device.type
Copy link
Contributor

Choose a reason for hiding this comment

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

we already have a few attributes that can describe a device id/name #1474 - is there some we can reuse instead of adding a new one?

Copy link
Author

Choose a reason for hiding this comment

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

I could see a scenario where you might need both a device ID and a hw component id in a parent child relationship. Maybe a server with a network card which you're measuring tx/rx.

- id: dc.cage
type: string
stability: experimental
brief: 'Name of the cage'
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this a standard property described somewhere we can link to? Also, let's add an additional layer saying what it is. E.g. dc.cage.name or dc.cage.id so that there is a room to record different properties of the cage and also the attribute name is descriptive.

Copy link
Author

Choose a reason for hiding this comment

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

I can multiply it. I could see people using one, either , both.

note: >
This is a literal cage which is used to protect servers from potential intruders.
examples: ['cage-1']
- id: dc.pod
Copy link
Contributor

Choose a reason for hiding this comment

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

same as for the cage - please link some external documentation and also update the name to be more specific (dc.pod.name, or dc.pod.id or something else)

note: >
This is a grouping of many servers typically several racks of servers.
examples: ['pod-4']
- id: dc.chassis
Copy link
Contributor

Choose a reason for hiding this comment

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

same here, please link docs and provide specific property in the name

type: resource
name: dc
brief: >
Generic DataCenter infrastructure tagging.
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should call out that it's intended for the telemetry that describes datacenters themselves (and not the user code running somewhere that data center)

Copy link
Contributor

Choose a reason for hiding this comment

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

Until we sort out Entity / Resource fully - I think you should be really explicit who looks up this data and reports it.

Is this something you want an SDK to fill out? This feels like something that belongs in an entity side-channel signal...

cc @tigrannajaryan (and rest of Entities SiG) for thoughts

brief: >
Generic DataCenter infrastructure tagging.
attributes:
- id: dc.name
Copy link
Member

Choose a reason for hiding this comment

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

it may be better to spell this out, check out the attribute name abbreviation guidelines

Copy link

github-actions bot commented Nov 5, 2024

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

@github-actions github-actions bot added the Stale label Nov 5, 2024
@hdost
Copy link
Author

hdost commented Nov 9, 2024

Unstale please 🙏 bot

@github-actions github-actions bot removed the Stale label Nov 10, 2024
| Attribute | Type | Description | Examples | [Requirement Level](https://opentelemetry.io/docs/specs/semconv/general/attribute-requirement-level/) | Stability |
|---|---|---|---|---|---|
| [`cloud.provider`](/docs/attributes-registry/cloud.md) | string | Name of the cloud provider. | `alibaba_cloud`; `aws`; `azure` | `Recommended` | ![Experimental](https://img.shields.io/badge/-experimental-blue) |
| [`dc.cage`](/docs/attributes-registry/dc.md) | string | Name of the cage [1] | `cage-1` | `Recommended` | ![Experimental](https://img.shields.io/badge/-experimental-blue) |
Copy link
Contributor

Choose a reason for hiding this comment

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

Should these be conditionally recommended based on availability?
Should these be opt-in or on by default?

E.g. do cloud providers give you this data?

|---|---|---|---|---|---|
| [`cloud.provider`](/docs/attributes-registry/cloud.md) | string | Name of the cloud provider. | `alibaba_cloud`; `aws`; `azure` | `Recommended` | ![Experimental](https://img.shields.io/badge/-experimental-blue) |
| [`dc.cage`](/docs/attributes-registry/dc.md) | string | Name of the cage [1] | `cage-1` | `Recommended` | ![Experimental](https://img.shields.io/badge/-experimental-blue) |
| [`dc.name`](/docs/attributes-registry/dc.md) | string | Name of datacenter. | `dc-name` | `Recommended` | ![Experimental](https://img.shields.io/badge/-experimental-blue) |
Copy link
Contributor

Choose a reason for hiding this comment

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

How do you get this information on a cloud?

is it correlated with cloud.availability_zone or cloud.region ?

- id: dc.site
type: string
stability: experimental
brief: 'Name of the datacenter site'
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you outline more what this means and how to generate this field? Can I discover these from within a datacenter or do I need to look it up?

type: resource
name: dc
brief: >
Generic DataCenter infrastructure tagging.
Copy link
Contributor

Choose a reason for hiding this comment

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

Until we sort out Entity / Resource fully - I think you should be really explicit who looks up this data and reports it.

Is this something you want an SDK to fill out? This feels like something that belongs in an entity side-channel signal...

cc @tigrannajaryan (and rest of Entities SiG) for thoughts

@jsuereth jsuereth dismissed their stale review November 12, 2024 16:37

Autogenerated code is up-to-date. Still concerns around documentation of how to fill things out and interaction with other semconv attributes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

5 participants