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

compute: allow "instance" label name to be set (behind a flag) #266

Closed
wants to merge 2 commits into from

Conversation

jjo
Copy link
Contributor

@jjo jjo commented Jul 23, 2024

Fixes #262.

What

Allow instance label name to be set via new -compute.instance-label flag.

Why

See #262 for the rationale and discussion.

How

  • for each compute Collector, create new collectorMetrics type
    containing compute metrics (moved from global var), to let each
    Collector own an allocated instance obtained via
    newCollectorMetrics(instanceLabel)

  • changing metrics from global var to dynamically allocated object is
    needed for:

  • there's a new config.CommonConfig type, currently only having a
    single field ComputeInstanceLabel, which is passed down from cmd/
    to each instantiated Provider, then to each compute Collector

Testing

Adapted testing with a couple ComputeInstanceLabel="node" changes,
although it should be improved to test changes to the label name.

--
Signed-off-by: JuanJo Ciarlante [email protected]

Fixes #262.

*## What

Allow `instance` label name to be set via new `-compute.instance-label` flag.

*## Why

See #262 for the rationale and discussion.

*## How

* for each compute `Collector`, create new `collectorMetrics` type
  containing compute metrics (moved from _global var_), to let each
  `Collector` own an allocated instance obtained via
  `newCollectorMetrics(instanceLabel)`

* changing metrics from _global var_ to dynamically allocated object is
  needed for:
  * setting its label at runtime
  * allowing tests to be self-contained, else the 1st test to run would "own"
    setting `instanceLabel`

* there's a new `config.CommonConfig` type, currently only having a
  single field `ComputeInstanceLabel`, which is passed down from `cmd/`
  to each instantiated _Provider_, then to each compute _Collector_

*## Testing

Adapted testing with a couple `ComputeInstanceLabel="node"` changes,
although it should be improved to test changes to the label name.

--
Signed-off-by: JuanJo Ciarlante <[email protected]>
@jjo jjo requested a review from a team as a code owner July 23, 2024 17:38
@jjo
Copy link
Contributor Author

jjo commented Jul 23, 2024

BTW also manually tested it locally with:

$ ./cloudcost-exporter  --aws.profile $DEV_PROFILE --aws.region us-east-1 --aws.services ec2 --compute.instance-label node
[...]

$ curl -s http://localhost:8080/metrics | egrep ^cloudcost_aws
[...]
cloudcost_aws_ec2_instance_total_usd_per_hour{cluster_name="<EDITED>",family="General purpose",machine_type="m6g.xlarge",node="ip-10-60-5-156.us-east-2.compute.internal",price_tier="ondemand",region="us-east-2"} 0.154
cloudcost_aws_ec2_instance_total_usd_per_hour{cluster_name="<EDITED>",family="General purpose",machine_type="m7g.2xlarge",node="ip-10-60-11-138.us-east-2.compute.internal",price_tier="spot",region="us-east-2a"} 0.1292
cloudcost_aws_ec2_instance_total_usd_per_hour{cluster_name="<EDITED>",family="General purpose",machine_type="m7g.2xlarge",node="ip-10-60-3-71.us-east-2.compute.internal",price_tier="spot",region="us-east-2a"} 0.1292
[...]

@jjo jjo changed the title compute: allow "instance" label to be set (behind a flag) compute: allow "instance" label name to be set (behind a flag) Jul 23, 2024
@jjo
Copy link
Contributor Author

jjo commented Sep 2, 2024

Closing as this was intended as a PoC / exploration to sense the feasibility and complexity on adding such flag, then we also agreed on not moving fwd with this approach.

@jjo jjo closed this Sep 2, 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.

compute: support using node instead/above of instance label
1 participant