grafana / unused

CLI tool, Prometheus exporter, and Go module to list your unused disks in all cloud providers
Apache License 2.0
51 stars 1 forks source link

make provider name configurable via cli flags #92

Closed jjo closed 2 months ago

jjo commented 2 months ago

There are use cases where you may need to make the provider name configurable, e.g. to make exported labels match existing ones, to ease promQL queries (especially joins as e.g. * on (provider, ...)).

This PR allows the provider name to be set via CLI, via e.g.:

unused-exporter --aws.profile my-profile --aws.providername=EKS

so that exported metrics will show as e.g.

unused_disk_size_bytes{disk="vol-xxx",k8s_namespace="my-ns",provider="eks",provider_id="my-profile",region="us-east-2",type="ssd",zone="us-east-2a"} 1.042042e+11

-- Signed-off-by: JuanJo Ciarlante juanjosec@gmail.com

inkel commented 2 months ago

Before reviewing, note that this will be tricky to implement with a new flag: the way the flags work, you can specify many -aws.profile, -gcp.project or -azure.sub at the same time if you want to check many accounts at once, eg:

$ unused -aws.profile=dev -aws.profile=staging -aws.profile=prod

This would list all unused disks in all three different AWS accounts at once (this is useful when you running on your machine, for instance). So if we add an extra flag for the name, which of the three providers would that name corresponds to?

Having said that, this gave me an idea: how about making passing the name an optional part of the of existing flag instance, like:

$ unused -aws.profile=dev:EKS-dev -aws.profile=staging:Billing

What do you think?

jjo commented 2 months ago

Before reviewing, note that this will be tricky to implement with a new flag: the way the flags work, you can specify many -aws.profile, -gcp.project or -azure.sub at the same time if you want to check many accounts at once, eg:

$ unused -aws.profile=dev -aws.profile=staging -aws.profile=prod

This would list all unused disks in all three different AWS accounts at once (this is useful when you running on your machine, for instance). So if we add an extra flag for the name, which of the three providers would that name corresponds to?

Having said that, this gave me an idea: how about making passing the name an optional part of the of existing flag instance, like:

$ unused -aws.profile=dev:EKS-dev -aws.profile=staging:Billing

What do you think?

IMO the provider name itself is pretty "global", as you already have per-provider project/profile/subs, i.e. no need to multi-value it.

inkel commented 2 months ago

Oh, I see, maybe I misunderstood the idea behind this 🤔

jjo commented 2 months ago

I'm not convinced, but I've left some comments on how I think it would be implemented in a (IMNSHO) simpler way. All the inline comments are part of a cohesive, bigger comment, and should be taken into account all at once.

Happy to discuss it!

Cool!, will apply and adjust to all providers.

jjo commented 2 months ago

@inkel implemented your suggestions, PTAL thanks!

jjo commented 2 months ago

Oh, I see, maybe I misunderstood the idea behind this 🤔

In short: currently it's a global (rather fixed string) provider label (in the case of the exporter) that will show e.g. as, for every AWS profile metrics show as:

{...,provider="aws",provider_id="profile-1",...}
{...,provider="aws",provider_id="profile-2",...}

the intention of this PR is to change the default provider(name) that's useful for people already using a their own (in the case of GrafanaLabs is eks), thus the above if run with --aws.providername=EKS will change to:

{...,provider="eks",provider_id="profile-1",...}
{...,provider="eks",provider_id="profile-2",...}