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] add openshift platform to all supported cloud providers #2985

Conversation

@frzifus frzifus requested review from a team November 24, 2022 08:02
@frzifus frzifus force-pushed the add_openshift_platform_to_supported_cps branch from 4244878 to b2eb39a Compare November 24, 2022 08:02
CHANGELOG.md Outdated Show resolved Hide resolved
semantic_conventions/resource/cloud.yaml Outdated Show resolved Hide resolved
semantic_conventions/resource/cloud.yaml Show resolved Hide resolved
@arminru arminru added area:semantic-conventions Related to semantic conventions spec:resource Related to the specification/resource directory enhancement New feature or request labels Nov 25, 2022
@frzifus frzifus force-pushed the add_openshift_platform_to_supported_cps branch from b2eb39a to 248ef25 Compare November 28, 2022 13:43
semantic_conventions/resource/cloud.yaml Outdated Show resolved Hide resolved
Copy link
Member

@joaopgrassi joaopgrassi left a comment

Choose a reason for hiding this comment

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

Looks good to the me!

specification/resource/semantic_conventions/cloud.md Outdated Show resolved Hide resolved
@Oberon00
Copy link
Member

This seems to be a non-native "Marketplace" feature. I don't really know how these work, is it built on top of other public-facing products of the platform? Do we actually need entries in the enum for that? If we include marketplace offerings, it might make the size of this enumeration potentially endless.

Signed-off-by: Benedikt Bongartz <[email protected]>
@frzifus
Copy link
Member Author

frzifus commented Nov 29, 2022

I don't really know how these work, is it built on top of other public-facing products of the platform?

Exactly, for example in aws openshift uses ec2 instances as nodes.

Do we actually need entries in the enum for that?

I would argue - yes. Although openshift uses ec2 instances, they are abstracted away. Which in my view makes openshift the platform. Nothing else happens with eks, which uses ec2 instances and its is also listed as a platform on aws. In both cases its more beneficial to know the underlying platform that potentially caused issues is eks or openshift then ec2.

If we include marketplace offerings, it might make the size of this enumeration potentially endless.

This depends very much on the definition of a platform. My feeling is that there are not many different platform solutions in the marketplaces.

@carlosalberto
Copy link
Contributor

Overall LGTM although @Oberon00 raised a good point regarding the potential explosion of values here. Let's try to get more eyes on this, just to stay on the safe side.

@joaopgrassi
Copy link
Member

joaopgrassi commented Nov 29, 2022

One could argue that we already have such things. We have in the enum several cloud k8s managed services (e.g., Azure AKS). I think adding OpenShift is pretty much following the same logic. If we don't want this then maybe we should rethink the existing entries. For ex, instead of listing the cloud products, just list "k8s". But I think that's not great, so I'm in favor of keeping the actual cloud "platforms" they offer.

@frzifus
Copy link
Member Author

frzifus commented Dec 1, 2022

Whats needed to move on?

@carlosalberto
Copy link
Contributor

Merging for completeness purposes (we have similar sections for other cloud providers/services). Let's consider discussing either limiting or redefining them if/when needed (hopefully not, etc).

@carlosalberto carlosalberto merged commit f7095dd into open-telemetry:main Dec 6, 2022
@frzifus frzifus deleted the add_openshift_platform_to_supported_cps branch December 6, 2022 16:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:semantic-conventions Related to semantic conventions enhancement New feature or request spec:resource Related to the specification/resource directory
Projects
None yet
Development

Successfully merging this pull request may close these issues.

add openshift platform to all supported cloud providers
5 participants