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

Initial import of the WIP from https://github.com/prometheus-community/helm-charts/pull/3872 #132

Closed
wants to merge 3 commits into from

Conversation

mattray
Copy link
Collaborator

@mattray mattray commented Oct 20, 2023

The Prometheus OpenCost Exporter Helm chart needs a primary source.

@mattray mattray force-pushed the prometheus_opencost_exporter branch from 7428b73 to 314e345 Compare October 20, 2023 03:17
@mattray
Copy link
Collaborator Author

mattray commented Oct 20, 2023

Fixes #128

@mattray mattray marked this pull request as ready for review October 20, 2023 03:21
Copy link
Collaborator

@toscott toscott left a comment

Choose a reason for hiding this comment

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

Reading through the comments on the prometheus-community helm chart pull request, I'm a little confused on what we are trying to accomplish with this. I would expect the home url monotek was referencing to point to the source application that the community helm chart is installing, not another helm chart. I would expect to just set that to:
https://github.com/opencost/opencost

@toscott
Copy link
Collaborator

toscott commented Oct 25, 2023

I guess just other opinions - I don't think that this necessarily fits into the prometheus-XXX-exporter model. If OpenCost was a product that did not produce prometheus metrics natively and we built a second tool just for exporting metrics, it would make sense to have a prometheus-opencost-exporter helm chart. Since OpenCost exports metrics natively, I don't think we need a new helm chart?

@mattray mattray force-pushed the prometheus_opencost_exporter branch from 0409157 to b12d63e Compare October 27, 2023 05:20
@mattray
Copy link
Collaborator Author

mattray commented Oct 27, 2023

@toscott I'm don't have a good understanding what they want in the Prometheus Community hosted version of the chart. I'm happy to make that the only location for it and change it to https://github.com/opencost/opencost and close this out.

I think it belongs in the prometheus-XXX-exporter model because some folks only use OpenCost as a metric exporter and don't use the UI at all. https://www.opencost.io/docs/integrations/opencost-exporter

I'll try to get some more clarity on their side what they're looking for, this can wait.

@toscott
Copy link
Collaborator

toscott commented Oct 27, 2023

@toscott I'm don't have a good understanding what they want in the Prometheus Community hosted version of the chart. I'm happy to make that the only location for it and change it to https://github.com/opencost/opencost and close this out.

Based on other charts in that repository, I think specifically what they were after on the other PR for the "home" field was a link to the underlying application code of the exporter: e.g. https://github.com/opencost/opencost.

I think it belongs in the prometheus-XXX-exporter model because some folks only use OpenCost as a metric exporter and don't use the UI at all. https://www.opencost.io/docs/integrations/opencost-exporter

Obviously up to you. Personally I'd think we wouldn't want to try to maintain two separate helm charts, but rather a single helm chart and just make sure its straightforward to use it with a disabled UI component. (This is actually how we are using it at my company already). I suspect splitting off a new helm chart will also split up community involvement between the two and result in inconsistent feature support between the charts.

@mattray mattray marked this pull request as draft November 1, 2023 06:43
@mattray mattray closed this Nov 27, 2023
@mattray mattray deleted the prometheus_opencost_exporter branch June 3, 2024 07:25
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.

2 participants