Add support for file-based configuration of logging

[venkatbvc]

Add support for writing logs to a file, console or both. Also support log rotation for the configured values. Changes to be committed: modified: README.md modified: src/logger.py modified: src/sidecar.py

This helps end users to transform logs to the required ones by using fluentd or any other mechanism

[venkatbvc]

@jekkel can you please review and share your comments.

[venkatbvc]

Hi Reviewers, can you please review and provide your comments

[venkatbvc]

@jekkel Can you please have a look at my Pull request?

[jekkel]

Hi @venkatbvc

is there a specific log format you have in mind for the transformation you want to enable with this change? Just wondering whether we should provide another pre-configured format instead.... :thinking:

[venkatbvc]

@jekkel in our company we have a custom log format, unfortunately i cannot share the format. but it is in json format. So we have used a sidecar approach where we write logs of kiwigrid sidecar to a logfile(using klog runner :https://pkg.go.dev/k8s.io/component-base/logs/kube-log-runner) and are transforming the current format to what we need. but this solution also needs to rotate logfiles. So was thinking if we support writing to a file with log rotation then it will help people to align logs to other formats.

[venkatbvc]

@jekkel We can set the log configuration from file. where user has flexibility to configure the log handlers. If this approach is fine with you , i can change and push it.

[jekkel]

@jekkel We can set the log configuration from file. where user has flexibility to configure the log handlers. If this approach is fine with you , i can change and push it.

From my POV I'd rather configure the logging system than adding a transformation pipeline. In fact, there'sthe same use case for custom log formats apply for "native" cloud integration, be it GCP, AWS or whatever. For example does GCP allow for rich structured logs via JSON but for getting the most out of it one need to conform to certain conventions.

So I'm looking forward to a generic log format configuration feature :+1:

[venkatbvc]

@jekkel Sure i will modify my changes according the config way.

[venkatbvc]

@jekkel I have made the configuration in a generic way. Please review and provide your comments.

[venkatbvc]

@jekkel Did you get a chance to go through the changes?

[jekkel]

Hi @tomrk-esteam8 could you please support here?

[venkatbvc]

Did you a chance to look at the code?

[tomrk-esteam8]

It seems checks does not work. Please adjust tests. _Build and Test / Test on k8s v1.27 (pullrequest) Failing after 4m

[venkatbvc]

Yes I am working on it.

[venkatbvc]

@tomrk-esteam8 Can you please trigger checks. I have modified code to fix the issue.

[venkatbvc]

@tomrk-esteam8 All the checks have passed. Can you please check and merge if all is fine?

[venkatbvc]

@tomrk-esteam8 Did you get a chance to look at the changes?

[tomrk-esteam8]

thanks for your contribution, could you please add some tests as well?

[ChristianGeie]

@tomrk-esteam8 my code review is done. Looks good to me. What do you think?

[venkatbvc]

@tomrk-esteam8 Added a test case. This covers the feature functionality. Please review

[venkatbvc]

@tomrk-esteam8 the tests are taking more time, due extra coma, i corrected it. can you please re-trigger the jobs

[ChristianGeie]

@venkatbvc some of your tests failed, see build check list. can you have a look?

[venkatbvc]

@ChristianGeie Can you trigger the pipeline. To see where the issue is coming. I added some debug logs.

[tomrk-esteam8]

Seems, that "hello from python script" in your log occurs only 5 times, when I see it correct, something is different here.

[ChristianGeie]

@venkatbvc looks good now, all checks are green. Thx

@tomrk-esteam8 I think we can merge, can you approve?

[tomrk-esteam8]

@venkatbvc looks good now, all checks are green. Thx

@tomrk-esteam8 I think we can merge, can you approve?

@ChristianGeie tests were commented out first for debug purpose, we can't merge yet :-)

[venkatbvc]

@tomrk-esteam8 @ChristianGeie i tried locally, i was able to see the logs 9 times, i am not getting why the count is not 9. can i add some debug information to the tests and push to debug? if you have any suggestions, please let me know

[tomrk-esteam8]

@venkatbvc please go for it :-) I will try as well to run checks in time manner. I suppose your pod in the tests might be differently configured at the end.

[venkatbvc]

@tomrk-esteam8 Can you trigger the build once.

[venkatbvc]

@tomrk-esteam8 I suspect the log configuration via file is dumping binary data. and the grep is giving error. I will see how to fix

[tomrk-esteam8]

@venkatbvc thanks :-)

[venkatbvc]

@tomrk-esteam8 can you trigger the build

[tomrk-esteam8]

great work @venkatbvc , please remove debug changes and I will be more than happy to run checks again and approve PR :-)

[venkatbvc]

@tomrk-esteam8 @ChristianGeie Thanks for the review. I have removed the debug changes. Please trigger the checks

[venkatbvc]

@tomrk-esteam8 Thanks. One query, Anything required from my side to be done for the PR to be merged? If yes, Let me know.

[venkatbvc]

@jekkel I have addressed the comments. Can you trigger the build to see if the tests goes through well

[jekkel]

PLease take a look at the workflow run: https://github.com/kiwigrid/k8s-sidecar/actions/runs/5520729921/jobs/10068885896?pr=274

I guess the file logging sidecar should not be waited for (unless we configure it to also log on the console?)

[venkatbvc]

@tomrk-esteam8 @jekkel @ChristianGeie can you please trigger the checks.

[venkatbvc]

@jekkel Yes i modified and pushed the code. can you please trigger the checks.

[ChristianGeie]

just triggered all checks

[venkatbvc]

@ChristianGeie Thanks, i see an issue in the syntax of the issue. I fixed it. Mostly it should go through. can you please trigger checks again.

[ChristianGeie]

@venkatbvc sure

[venkatbvc]

All, Added debug commands to identify the failure cause in the test. Can you please trigger check

[venkatbvc]

@jekkel @tomrk-esteam8 @ChristianGeie Can you trigger the checks. Issue is fixed.

[venkatbvc]

@jekkel All checks passed. Can you merge if the changes are OK.