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 3 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

# Datacenter

## Datacenter Attributes
Copy link
Contributor

Choose a reason for hiding this comment

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

We have tooling to autogenerate markdown, you should use <!-- semconv datacenter --> here for consistency.

See: https://github.com/open-telemetry/semantic-conventions/blob/main/model/README.md

Copy link
Author

Choose a reason for hiding this comment

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

🤔 I used the automated tool. I wonder if because the name was missing this resulted.

Copy link
Author

Choose a reason for hiding this comment

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

Well I found out what the issue was, perhaps I missed it in my reading of the various documentation, but I didn't see any mention that you need to first manually create a file which has the reference which you are mentioning, and then it will replace the contents between. I can understand the use, but it's not exactly clear as a first time user of the weaver tool.

Copy link
Author

Choose a reason for hiding this comment

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

@jsuereth am i missing something

@@ -0,0 +1,15 @@
groups:
- id: datacenter
Copy link
Contributor

Choose a reason for hiding this comment

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

This needs a name field, set it to datacenter for now

@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.

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

Successfully merging this pull request may close these issues.

4 participants