langfuse / langfuse-k8s

Community-maintained Kubernetes config and Helm chart for Langfuse
https://langfuse.com
MIT License
44 stars 24 forks source link

Add helm chart #3

Closed mautini closed 5 months ago

mautini commented 5 months ago

This PR adds an Helm Chart to easily deploy Langfuse in a K8s cluster, with associated postgres database (from Bitnami helm chart)

This MR also create a Github action to publish the Helm Chart automatically. There are some Pre-requisites to get it working:

cf. https://github.com/helm/chart-releaser-action#example-workflow

Could you please do that?

Thanks,

CLAassistant commented 5 months ago

CLA assistant check
All committers have signed the CLA.

DevBey commented 5 months ago

Hello @maxdeichmann,

can we please check and merge any of these helm chart PR's ?? helm chart are essential for langfuse deployments.

mautini commented 5 months ago

Hey @DevBey, at the mean time, if you need an Helm Chart, you can use

helm repo add langfuse https://mautini.github.io/langfuse-k8s
helm repo update
helm install langfuse langfuse/langfuse

I plan to release it on my own if it's not included in this repository (I already pingued to team on Discord but it seems there is few interest regarding k8s deployment of Langfuse from the team...)

DevBey commented 5 months ago

brilliant thanks @mautini !!

mautini commented 5 months ago

And if you need it, you have information about configuration here: https://github.com/mautini/langfuse-k8s/blob/add_helm_chart/README.md

elephantum commented 5 months ago

Hey, @mautini love this PR!

The only question is: do you think it's really necessary to self-host postgres by default? Usually databases are used as a managed service from cloud provider. What do you think?

mautini commented 5 months ago

Hey, thanks!

I think it's interesting to provide the helm chart with a default postgres, this allow to deploy langfuse and have everything in your cluster with just one command. But I agree that it could be an issue if you already have a database in your cluster, we could improve this Helm Chart to use (or not) an external database.

elephantum commented 5 months ago

That's a good point, that batteries included approach would lower friction!

we could improve this Helm Chart to use (or not) an external database.

My vote is that we definitely should! And probably make self-hosted an optional, but mention this option in a README

marcklingen commented 5 months ago

I plan to release it on my own if it's not included in this repository (I already pingued to team on Discord but it seems there is few interest regarding k8s deployment of Langfuse from the team...)

Hi @mautini, sorry for moving slowly on this. We very much want this repo to include a helm chart to make langfuse easy to deploy on k8s. The last 2 weeks just have been extremely busy and I just got to this. I'll be way more responsive from now on, sorry about that!

Thank you for your contribution on this and thank you @elephantum for you review, appreciate it.

My vote is that we definitely should! And probably make self-hosted an optional, but mention this option in a README

I tend to agree as all teams that self-host Langfuse who I know run on RDS or other managed products and do not run Postgres within k8s. Is this a big change for you to make? I think this would make this chart way more versatile.

Publish via GitHub action sounds great. I just added the gh-pages branch and will confirm that the workflow works once this is merged.

mautini commented 5 months ago

Hi @marcklingen,

Thanks for your feedback and no worries.

I updated the MR to handle external databases, I think it should do the job.

Also note this MR that add the mention of the Helm Chart in the documentation: https://github.com/langfuse/langfuse-docs/pull/534

elephantum commented 5 months ago

@marcklingen in official Langfuse documentation three connections to DB mentioned: DATABASE_URL, DIRECT_URL and SHADOW_DATABASE_URL are they all used?

If yes, then probably we should have an ability to configure all three in helm chart?

marcklingen commented 5 months ago

@marcklingen in official Langfuse documentation three connections to DB mentioned: DATABASE_URL, DIRECT_URL and SHADOW_DATABASE_URL are they all used?

If yes, then probably we should have an ability to configure all three in helm chart?

Great point.

When using an external DB and scaling out the langfuse application container, DIRECT_URL is really important to perform migrations in order to use a connection pooler on DATABASE_URL. I'm unsure how this applies to the setup of running postgres within k8s but exposing this environment would be really helpful for everyone using a managed database.

I never used SHADOW_DATABASE_URL, but some folks needed it to successfully migrate their managed databases. More on this here. If we add DIRECT_URL, adding SHADOW_DATABASE_URL as well is a good idea.

@elephantum, @mautini can one of you add this change? I think both of you have way more experience regarding helm charts than I have.

marcklingen commented 5 months ago

How easy it is to pass environment variables to the application container that are not yet specified in this chart? Do they all need to be included here in order to be passed? We add a couple of new configuration options every month and I am just wondering if this chart then needs to be updated every time to be able to use these environments.

DevBey commented 5 months ago

not a helm expert but been using helm chart for cloudnative pg where they either use range or to yaml for thing like env variables. example https://github.com/cloudnative-pg/charts/blob/main/charts/cloudnative-pg/templates/deployment.yaml

mautini commented 5 months ago

--> Support for DIRECT_URL and SHADOW_DATABASE_URL: https://github.com/langfuse/langfuse-k8s/pull/3/commits/04af98b7c0c5cc643e769c4e34fd1057ad300521 --> Support for arbitrary env variables: https://github.com/langfuse/langfuse-k8s/pull/3/commits/ba718f1495f28565aaf20b7e4e8b569aadc648bb

Regarding env variable, I agree that this solution is more versatile but I think it's more explanatory if we could have explicit parameters in this Helm Chart when Langfuse app needs new env variables

marcklingen commented 5 months ago

--> Support for DIRECT_URL and SHADOW_DATABASE_URL: 04af98b Thanks!

--> Support for arbitrary env variables: ba718f1

Regarding env variable, I agree that this solution is more versatile but I think it's more explanatory if we could have explicit parameters in this Helm Chart when Langfuse app needs new env variables

Agree, this seems to be the best way to make it easy to understand what is mandatory but also allow to passthrough all optional ones without having to change the chart.

@mautini do you still want to change anything? Otherwise I'd go ahead and release this as this seems to be a great starting point that we can still iterate on down the road.

mautini commented 5 months ago

Thanks and yes I think it's a starting point. If you can wait a few hours I can do some tests to be sure it's working well. I will let you know

marcklingen commented 5 months ago

sounds good, just let me know and I'll try to release it later todat

thank again for your effort on this @mautini!

DevBey commented 5 months ago

briliant thanks both @marcklingen and especially @mautini.

will test this as well when it's out.

mautini commented 5 months ago

Ok, I'm good to go, @marcklingen. I just added a last commit with explicit examples about how to configure for embedded or external PostgreSQL.

Anyway, we could still iterate if we encounter issues in the future :)

marcklingen commented 5 months ago

Sounds good, thank you for testing this again!