Customize logging parameters

[navi86]

Description

Deploying application in gcp creates a huge amount of noise in logging, because all logs that are written in stderr marked as "ERROR"
Moreover, debug logs creates lots of messages in the pod, so it will be nice to have possibility to decrease log level.

Type of change

• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)

How has this been tested?

this changes is pretty small so it was tested by running "helm template"

Checklist

• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation
• I have added tests that prove my fix is effective or that my feature works

[guidoiaquinti]

👋 Hi @navi86 and thank you for your PR!

All files in charts/posthog/templates/clickhouse-operator are generated by the scripts/clickhouse_operator_sync.sh tool and they are coming from the upstream template at https://raw.githubusercontent.com/Altinity/clickhouse-operator/0.16.1/deploy/operator/clickhouse-operator-install-template.yaml. My suggestion is to make the change you are suggesting directly in the https://github.com/Altinity/clickhouse-operator repo and then cherry pick it here once it gets released.

Let me know if something is not clear, happy to help!

[navi86]

hi @guidoiaquinti
it's not possible to make any changes there, because it requires to create a helm chart
Altinity/clickhouse-operator#837 (comment)
Is it possible to have an extra sed operation in clickhouse_operator_sync.sh for now ?

[guidoiaquinti]

hi @guidoiaquinti it's not possible to make any changes there, because it requires to create a helm chart Altinity/clickhouse-operator#837 (comment) Is it possible to have an extra sed operation in clickhouse_operator_sync.sh for now ?

My take is that we should expose the logging config in the ClickHouseInstallation/operator in order to be able to do something like:

apiVersion: "clickhouse.altinity.com/v1"
kind: "ClickHouseInstallation"
metadata:
  name: "posthog"
spec:
  configuration:
    users:
      admin/password: <something>
      admin/networks/ip: "0.0.0.0/0"
      admin/profile: default
      admin/quota: default

    profiles:
      default/allow_experimental_window_functions: "1"

    clusters:
      - name: "posthog"
        templates:
          podTemplate: pod-template
          clusterServiceTemplate: service-template
          dataVolumeClaimTemplate: data-volumeclaim-template
        layout:
          replicasCount: 1
          shardsCount: 1
        logging:
          logtostderr: something
          alsologtostderr: something
[...]

and without modifying hardcoded config map manually.

Is this issue a blocker for your rollout?

[navi86]

@guidoiaquinti
we are using sed for updating some confirmation already and there is no a hardcoded values. Moreover, how you can update log level if it's only configured in ConfigMap ?
This is just logs so it's not a blocker, but for my installation it generates about 80k messages per hour and I don't think these logs have any value.

[guidoiaquinti]

@guidoiaquinti we are using sed for updating some confirmation already and there is no a hardcoded values. Moreover, how you can update log level if it's only configured in ConfigMap ? This is just logs so it's not a blocker, but for my installation it generates about 80k messages per hour and I don't think these logs have any value.

The only 2 sed I see are to add an if/else condition to make the ClickHouse installation opt-in (as people might want to use an external cluster) and the other is to specify a custom namespace.

how you can update log level if it's only configured in ConfigMap ?

I might be missing something but the upstream operator should simply expose a logging configuration option via ClickHouseInstallation like it already does for other config like users and profiles.

Let me know if it make sense @navi86 🙇

[guidoiaquinti]

👋 Hey @navi86, I'm going to close the PR without merging. Let me know if something is not clear or if you want to continue the conversation above.

Thank you!