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

feat: add attributes for cloud-native deployment #24

Closed

Conversation

thisthat
Copy link
Member

@thisthat thisthat commented May 15, 2023

thisthat added 4 commits May 15, 2023 15:00
Signed-off-by: Giovanni Liva <giovanni.liva@dynatrace.com>
Signed-off-by: Giovanni Liva <giovanni.liva@dynatrace.com>
Signed-off-by: Giovanni Liva <giovanni.liva@dynatrace.com>
Signed-off-by: Giovanni Liva <giovanni.liva@dynatrace.com>
@thisthat thisthat marked this pull request as ready for review May 15, 2023 14:51
@thisthat thisthat requested review from a team May 15, 2023 14:51
Copy link

@felixbarny felixbarny left a comment

Choose a reason for hiding this comment

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

Some of the concepts, such as application and workload aren't unique to k8s/cloud native deployments. I wouldn't segregate them under a specific deployment namespace but rather try to map them to more generic concepts that also make sense outside of cloud native deployments.

This is a good example where it would be good to have a common catalogue of attributes and logs and traces just reference from the common catalogue.


| Attribute | Type | Description | Examples | Requirement Level |
|---|---|---|---|---|
| `deployment.workload.name` | string | The name of the single workload under deployment. | `podtato-head-hat`; `podtato-head-left-leg` | Required |

Choose a reason for hiding this comment

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

Sounds like this is essentially the same as service.name?

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks @felixbarny for pointing that out. You are right, this has the same semantics as service.name. I'll adapt the PR to make use of that attribute.

@jsuereth
Copy link
Contributor

Does it make sense to spin up a CI/CD working group and have this as part of that?

@reyang
Copy link
Member

reyang commented May 16, 2023

Related to open-telemetry/oteps#223.

@horovits FYI.

| Attribute | Type | Description | Examples | Requirement Level |
|---|---|---|---|---|
| `deployment.workload.name` | string | The name of the single workload under deployment. | `podtato-head-hat`; `podtato-head-left-leg` | Required |
| `deployment.workload.namespace` | string | The namespace in which the workload will be deployed. | `podtatohead` | Recommended |
Copy link
Contributor

Choose a reason for hiding this comment

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

deployment.workload.{name|version|namespace} seem to overlap with service.* attributes. Should we just share them here?

Also deployment.workload.status seems to be the status of the deployment, less the workload itself.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thank @jsuereth for looking into this. I agree, these 3 attributes can be replaced with service.* attributes.

Also deployment.workload.status seems to be the status of the deployment, less the workload itself.

Deployment is an overloaded term, especially in K8s. Maybe delivery would be a better term 🤔
If I read "status of the deployment", I think about the specific Kubernetes Deployment CR, but here I'd like to capture also the different "deployment models" that K8s allows which are grouped around the term Workload.

|---|---|---|---|---|
| `deployment.task.name` | string | The name that identifies the executed task. | `database-migration`; `report-metric`; `provision-infrastructure` | Required |
| `deployment.task.status` | string | The status of the task execution. | `Pending` | Recommended |
| `deployment.application.name` | string | The name of the application under deployment. | `podtato-head` | Required |
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't understand the difference between workload + application here. Can you outline that somewhere?

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure, maybe this should be in the "Overview" section.
The difference is that an application is an abstraction that groups together different workloads. For example, an application could be my-awesome-website which comprises 3 different workloads: frontend, backend, and database.
Tasks are actions that are executed when delivering the application and its individual workloads. For example, a task could be sending a slack message that the delivery for a specific component started/finished.
Do you think this explanation is clear @jsuereth? If so, I would add it to the document.

@thisthat
Copy link
Member Author

Does it make sense to spin up a CI/CD working group and have this as part of that?

I would be interested in such thing 👍

@jsuereth jsuereth added the experts needed This issue or pull request is outside an area where general approvers feel they can approve label Jul 31, 2023
@joaopgrassi
Copy link
Member

Hi @thisthat! Sorry for the ping, but is this something you are still interested in working on?

@thisthat
Copy link
Member Author

Hey @joaopgrassi sure! I am still interested 👍

Copy link

github-actions bot commented Feb 3, 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 Feb 3, 2024
type: event
brief: >
This document defines attributes for Cloud Native deployments of applications.
These attributes can be attached to spans when performing operations for deploying applications on Kubernetes,
Copy link
Contributor

Choose a reason for hiding this comment

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

Why mention Kubernetes here?
Opentelemetry is open source and vendor neutral and as such not restricted to deployments on just K8s.
Maybe it should be phrased that "deploying applications on Kubernetes" is an example.

The same question applies for the brief of deployment.workload below.


### Application

Each stage of the lifecycle of a cloud-native application deployment SHOULD be recorded as an Event on the span during
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 events have any relation with the Continuous Deployment Events of CDEvents?

@joaopgrassi
Copy link
Member

Hi @thisthat !

We changed how the CHANGELOG.md is managed. Please take a look at https://github.com/open-telemetry/semantic-conventions/blob/main/CONTRIBUTING.md#adding-a-changelog-entry to see what needs to be done. Sorry for the disruption.

@arminru arminru removed the Stale label Feb 5, 2024
@adrielp
Copy link
Contributor

adrielp commented Mar 21, 2024

Referencing: #833

@jsuereth
Copy link
Contributor

Let's close this PR as being stale. We can reopen it with changes (including attribute registry usage) and align w/ exisitng CI/CD plans.

@jsuereth jsuereth closed this Apr 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
experts needed This issue or pull request is outside an area where general approvers feel they can approve
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

8 participants