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 some new labels to every container #4324

Merged
merged 3 commits into from
Dec 18, 2024

Conversation

Julien-R44
Copy link

@Julien-R44 Julien-R44 commented Nov 17, 2024

Changes

I strongly encourage to read the comment that initiated this PR: #3569 (comment)

TL;DR: The main issue is the difficulty in monitoring due to automatically generated container names. Currently, when I look at my Grafana/cAdvisor dashboard tracking CPU usage for each container managed by Coolify, I only see cuid names , which makes it impossible to identify what each container corresponds to.

So we need a way to have "human-readable" names for each container. This PR addresses the issue by adding three new labels to each container managed by Coolify :

  • coolify.resourceName: The name of the Application or Service
  • coolify.projectName: The name of the project where the resource lives
  • coolify.serviceName: This one might be a bit confusing, and perhaps we can find a better name. If the resource is of type Service, it will be the sub-service name. If it's of type Application, it will be the application name. This label is necessary because, using my monitoring use case as an example, I need metrics/logs for each of my containers. To differentiate them, I need a unique name. If I'm using a Service, I can't rely on coolify.resourceName since all the services in my docker-compose file would have the same value. I hope this explanation makes sense.

This is my first contribution to Coolify, and I'm not familiar with PHP either, so feel free to point out any corrections or improvements I should made

( And thanks a lot for making Coolify 💙 )

Issues

@peaklabs-dev
Copy link
Member

I get why this would be needed overall but I have a questions:

  1. Why does it have to be docker labels and not env variabels as we do have those COOLIFY_CONTAINER_NAME for example? And I will soon be adding things like Project name and so on as well to env vars.

@peaklabs-dev peaklabs-dev added the 🛠️ Feature Issues requesting a new feature. label Nov 17, 2024
@peaklabs-dev peaklabs-dev self-assigned this Nov 17, 2024
@Julien-R44
Copy link
Author

Because environment variables aren't collected by cAdvisor or anything I think. I suppose there's a way to tweak something, but with labels it works straight away. cAdvisor collects them, and I can use them in my PromQL queries ( Prometheus )

@Julien-R44
Copy link
Author

Is there anything I can do to help move this PR forward or we just need some time? Let me know if anything is needed 🙂

bootstrap/helpers/docker.php Show resolved Hide resolved
bootstrap/helpers/docker.php Show resolved Hide resolved
@peaklabs-dev peaklabs-dev added the 💤 Waiting for changes PRs awaiting changes from the author. label Dec 2, 2024
@peaklabs-dev peaklabs-dev removed the 💤 Waiting for changes PRs awaiting changes from the author. label Dec 3, 2024
Copy link
Member

@peaklabs-dev peaklabs-dev left a comment

Choose a reason for hiding this comment

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

I have been thinking a lot about this, this week and I have some ideas:
coolify.projectName -> Project Name
coolify.environmentName -> Environment Name --> Needs to be changed to this form coolify.environment

coolify.resourceName -> The name of the Application or Service --> For example WordPress or so (stack name of compose)
coolify.containerName: The name of the container inside the application or inside the WordPress service (for wp it would be MariaDB and WordPress (with uuid ofc). --> Basically, this containerName would be the same as what we store in the COOLIFY_CONTAINER_NAME env var https://coolify.io/docs/knowledge-base/environment-variables/#coolify_container_name

Does this make sense for your use case, or am I missing something that the service name did and the containerName would not now?

@peaklabs-dev peaklabs-dev added the 💤 Waiting for feedback Issues awaiting a response from the author. label Dec 6, 2024
@Julien-R44
Copy link
Author

Julien-R44 commented Dec 8, 2024

Yes, that won’t be enough. If I understand correctly, coolify.containerName and the container name will be exactly the same.

If that’s the case, then it’s kind of pointless because the real container name is already exposed and retrievable, as it’s literally the container’s name 😅

This is problematic because every time I restart the containers in a stack, the UUID will change, resulting in a "break" in a Grafana graph. I’ll end up with two separate series, making operations more tedious. This is what I was trying to explain here: #3569

In fact, the second one might be more practical in some cases: it would allow me to get consistent metrics for a single container, even if it restarts and gets a new name. With the labels, the values would remain the same even if it restarts, allowing me to maintain a continuous series instead of having two separate series labeled my-prefix-3422394823948234 and my-prefix-3249238429384. Hope that makes sense 😅!

I'm talking about Grafana here, but it'll be the same with any tool. For example, I want to retrieve my logs with Loki? Same problem, I will have two different series/projects on which I have logs : “wordpress-mariadb-234923842” and “wordpress-mariadb-324298234”, making it impossible to retrieve all the logs in a continuous way without hacking something

The ideal solution would really be to have a fixed and consistent name. And since we can assign names to services in a Compose stack, I think it makes perfect sense to use that. That’s what we were doing with coolify.serviceName

@peaklabs-dev peaklabs-dev removed the 💤 Waiting for feedback Issues awaiting a response from the author. label Dec 10, 2024
@peaklabs-dev peaklabs-dev changed the base branch from next to main December 11, 2024 13:37
@peaklabs-dev peaklabs-dev changed the base branch from main to next December 11, 2024 13:37
@peaklabs-dev peaklabs-dev removed their assignment Dec 12, 2024
@andrasbacsai
Copy link
Member

Thank you for the PR and sorry for the long wait time.

Everything looks good, so it will be released in the next version.

@andrasbacsai andrasbacsai merged commit 340075a into coollabsio:next Dec 18, 2024
1 check passed
@github-actions github-actions bot removed the 🛠️ Feature Issues requesting a new feature. label Dec 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants