Obsolete Helm value logging.operator.timeFormat ?

[lsolovey]

The README.md mentions logging.operator.timeFormat value with the description:

Logging time format for KEDA Operator. Allowed values are 'epoch', 'millis', 'nano', or 'iso8601'.

Using keda Helm chart version 2.6.2 - setting this Helm value doesn't seem to make any difference. I don't see it respected by the Helm chart.

• Is it obsolete and needs to be taken out of README ?
• How can we change the format of time in the logs without this Helm value supported?

Expected Behavior

Helm value logging.operator.timeFormat changes the time format of keda-operator logs.

Actual Behavior

Helm value logging.operator.timeFormat doesn't change the time format of keda-operator logs.

Specifications

• KEDA Version: 2.6.1 (Helm chart 2.6.2)
• Platform & Version: Docker Desktop Kubernetes
• Kubernetes Version: 1.21.5
• Scaler(s): n/a

[zroubalik]

https://github.com/kedacore/keda/blob/main/BUILD.md#keda-operator-logging

This is the source of truth, the helm chart should be updated.

[tomkerkhove]

Are you willing to send a PR for this @lsolovey?

[lsolovey]

https://github.com/kedacore/keda/blob/main/BUILD.md#keda-operator-logging

This is the source of truth, the helm chart should be updated.

Hi @zroubalik, the link above doesn't explain how to change the time format in logs. Do you mean that time format is immutable, and we just need to remove any mention of logging.operator.timeFormat from the Helm chart?

[zroubalik]

@lsolovey it seems like it's like that. We rely on https://sdk.operatorframework.io/docs/building-operators/golang/references/logging/ for logging. On a quick check it seems that it doesn't support timeformat. But I might be wrong. Could you please investigate whether there is any advanced option to do so? Thx

[lsolovey]

@zroubalik I have no experience with Go, but I'll try to do some research on this. Let me dig into the docs for Operator Framework logging and Zap.

[lsolovey]

@zroubalik, @tomkerkhove Sorry for the delay, finally I got some time to look into this!

I found that controller-runtime actually supports --zap-time-encoding CLI flag in the BindFlags() function.

It was added in this PR: kubernetes-sigs/controller-runtime#1688
However documentation was not updated as part of the original PR, and they updated docs later here: https://github.com/kubernetes-sigs/controller-runtime/pull/1837/files

So, I was able to pass this CLI flag to KEDA Helm chart by setting these Helm values (using KEDA Helm chart 2.6.2):

extraArgs:
  keda:
    zap-time-encoding: rfc3339

Now KEDA operator logs looks like:

{"level":"info","ts":"2022-04-28T20:03:08Z","logger":"controller-runtime.metrics","msg":"Metrics server is starting to listen","addr":":8080"}

I think the next step for KEDA would be to start respecting logging.operator.timeFormat Helm value (to match the README) and generate --zap-time-encoding CLI flag in the KEDA Operator Deployment yaml.

Alternatively - just get rid of logging.operator.timeFormat in the README, and stick with generic extraArgs.keda.zap-time-encoding: rfc3339 approach that works as of today.

What do you think?

[karl-sprig]

extraArgs:
keda:
zap-time-encoding: rfc3339

Just a FYI to future google searchers, the deployment template now has explicitly support for the logging faculties:

https://github.com/kedacore/charts/blob/main/keda/templates/12-keda-deployment.yaml#L67

Via values.yaml:

logging:
  operator:
    level: debug

[tomkerkhove]

Yes that is correct. I think we can close this issue?

[lsolovey]

@tomkerkhove No, the focus of this issue is to allow customizable timestamp formats in logs.

In other words, to support logging.operator.timeFormat Helm value. Please see my detailed comment above from April 28th.

[lsolovey]

@tomkerkhove Ah, never mind. Timestamp format is now customizable. It was implemented here:
kedacore/keda#3066

So we can close this one.