robusta-dev / krr

Prometheus-based Kubernetes Resource Recommendations
MIT License
2.85k stars 152 forks source link

Add support for DataDog metrics instead of Prometheus #9

Open andrew-kolesnikov opened 1 year ago

andrew-kolesnikov commented 1 year ago

Our organization no longer uses Prometheus, so I am very curious what would it take to integrate with DataDog as a metrics source, either as what they call an "integration" or just using plain old DataDog API.

aantn commented 1 year ago

It shouldn't be too hard. Is this something you'd be interested in taking a shot at yourself?

andrew-kolesnikov commented 1 year ago

Would love to contribute this from us, but it's looking like no one on my team will have any capacity for it in the next few months

LeaveMyYard commented 1 year ago

@aantn Maybe we can add DataDog metrics together with Thanos integration? (#18) Should not be that hard, WDYT?

francRang commented 3 months ago

I have created something like this in the past, I think I should be able to add DataDog support to this project. Let me know if I could get this assigned so I can commit some of my time to contribute.

aantn commented 3 months ago

100% yes!

Can you do it on top of the prometheus-workload-loader branch? There are some fairly major changes in that branch to the internal architecture, and we plan to merge it soon. So best to base your work off of that.

To the degree that you will need to change internal interfaces/abstractions to support this, we are open to it! We initially only had Prometheus in mind, so there might be some changes needed here, which is completely fine.

francRang commented 3 months ago

100% yes!

Can you do it on top of the prometheus-workload-loader branch? There are some fairly major changes in that branch to the internal architecture, and we plan to merge it soon. So best to base your work off of that.

To the degree that you will need to change internal interfaces/abstractions to support this, we are open to it! We initially only had Prometheus in mind, so there might be some changes needed here, which is completely fine.

Sounds good, appreciate the fast response.

aantn commented 3 months ago

Of course, can't wait to see what you do.

On Thu, Jun 20, 2024, 17:26 Francisco Rangel @.***> wrote:

100% yes!

Can you do it on top of the prometheus-workload-loader branch? There are some fairly major changes in that branch to the internal architecture, and we plan to merge it soon. So best to base your work off of that.

To the degree that you will need to change internal interfaces/abstractions to support this, we are open to it! We initially only had Prometheus in mind, so there might be some changes needed here, which is completely fine.

Sounds good, appreciate the fast response.

— Reply to this email directly, view it on GitHub https://github.com/robusta-dev/krr/issues/9#issuecomment-2180849642, or unsubscribe https://github.com/notifications/unsubscribe-auth/AADYUBZR6IAHOIJ7677GQ5LZILRCXAVCNFSM6AAAAABJTHHZTWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCOBQHA2DSNRUGI . You are receiving this because you were mentioned.Message ID: @.***>

francRang commented 3 months ago

I've started dedicating some time to PR. Will push commits to a branch that I will request to be reviewed so I can merge to prometheus-workload-loader and then that branch can eventually be merged into main.

The OP mentioned DataDog Integrations, from what I've read from the documentation, the so called integrations would be a bit overkill since they are meant to pull/push data, and we only want to pull (link to api integrations states the same thing).

So the approach I am taking will be hitting the DataDog API via the existing DataDog Python client.

As far as integrating this to krr, I think I will create a boolean flag --datadog that will basically route all of the existing parameters to be used with DataDog and not Prometheus, given that we'll only have 2 options and not > 2 for the metrics source (only the flags/options that make sense, e.g: if you set a prometheus URL that will be ignored, or maybe the CLI call will be denied so you only set things that make sense, e.g: --cpu-min), so putting all of the logic under core/integrations/datadog makes sense.

Should be simpler than the Prometheus implementation since DataDog is centralized, the only thing will be making sure the current cluster you're in has metrics in DataDog you're querying.

Anyways, just wanted to put my approach out there to keep y'all in sync. Let me know if it makes sense. Any feedback would be appreciated.

aantn commented 3 months ago

Yes, makes total sense. There's a question regarding the interaction with the new --mode flag that controls how resources are discovered. If you set --datadog are resources is the cluster (e.g. list of deployments) still discovered via kubectl or is it discovered via DataDog?

The latter is probably better in which case setting --datadog probably ignored the mode flag or automatically enables a DataDog discovery mode. But if it is simpler to implement, you can leave discovery to kubectl and only worry about getting the metrics for recommendations from DD.

francRang commented 3 months ago

Thinking getting the list of resources via kubectl then just querying the metrics might be easier, that way I don't have to write queries that specifically look for k8s resources, but just get the CPU/Mem values/datapoints.

aantn commented 3 months ago

That works, sounds good.

On Fri, Jun 21, 2024, 17:58 Francisco Rangel @.***> wrote:

Thinking getting the list of resources via kubectl then just querying the metrics might be easier, that way I don't have to write queries that specifically look for k8s resources, but just get the CPU/Mem values/datapoints.

— Reply to this email directly, view it on GitHub https://github.com/robusta-dev/krr/issues/9#issuecomment-2182915647, or unsubscribe https://github.com/notifications/unsubscribe-auth/AADYUBYWJPR63R3TIEBCT6TZIQ5PVAVCNFSM6AAAAABJTHHZTWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCOBSHEYTKNRUG4 . You are receiving this because you were mentioned.Message ID: @.***>

francRang commented 3 months ago

WIP: https://github.com/francRang/krr/tree/add-datadog-support-as-metric-source 🐶

changhyuni commented 3 months ago

I love this feature.

francRang commented 2 months ago

Haven't had much time to work on this as of this month. Things should clear off soon. Will try to commit some time to it this week. Thanks y'all.

aantn commented 2 months ago

No problem, thank you for the update!

On Thu, Jul 25, 2024, 07:10 Francisco Rangel @.***> wrote:

Haven't had much time to work on this as of this month. Things should clear off soon. Will try to commit some time to it this week. Thanks y'all.

— Reply to this email directly, view it on GitHub https://github.com/robusta-dev/krr/issues/9#issuecomment-2249340338, or unsubscribe https://github.com/notifications/unsubscribe-auth/AADYUBY4NUVOKFMNZNBC3RTZOB3DJAVCNFSM6AAAAABJTHHZTWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDENBZGM2DAMZTHA . You are receiving this because you were mentioned.Message ID: @.***>