Lirt / velero-plugin-for-openstack

Openstack Cinder, Manila and Swift plugin for Velero backups
MIT License
26 stars 12 forks source link

Add an ability to print API debug logs #72

Closed kayrus closed 1 year ago

kayrus commented 1 year ago

Resolves #71

kayrus commented 1 year ago

@Lirt it looks like the clientconfig.AuthenticatedClient func never respected the transport. It just omits the specified HTTPClient. See a https://github.com/gophercloud/utils/issues/192 I pushed a workaround.

I also tried to play with --log-level=debug and print logs using logrus.Debugf, but they were not printed. So I left it as is with the level=os_debug, it also makes it easier to grep without changing the --log-level in velero server.

Ready for review.

Lirt commented 1 year ago

Hi @kayrus,

Thank you for all PRs. I will try to look at this as soon as possible (please keep in mind that this early summer is very busy for me) and approve/merge in correct order.

kayrus commented 1 year ago

@Lirt no rush, take your time.

Lirt commented 1 year ago

@Lirt it looks like the clientconfig.AuthenticatedClient func never respected the transport. It just omits the specified HTTPClient. See a https://github.com/gophercloud/utils/issues/192

Thank you for creating issue. Your workaround looks good to me in this MR.

Regarding debug log level, you are right, it doesn't work. I don't see any issue in what we do inside the plugin. It looks more like issue in the Velero itself, but I cannot find anything suspicious in the code so far. I found that AWS plugin is using debug logs in the same way as you suggested and as I tried myself. Also the log line during startup looks correct in my environment.

time="2023-06-27T15:33:41Z" level=debug msg="Setting log level to DEBUG" cmd=/velero logSource="pkg/plugin/framework/server.go:213" pluginName=velero

For me it would be cleanest if the --log-level could be set to debug and we didn't have to set OS_DEBUG env. In both cases enabling logging requires change in velero pod args or envs. The code would be more simple if we just omit the env. variable and configure the interfaced function func (d osDebugger) Printf to do log.Debugf() + set --log-level=debug.

I'll try to dig deeper into the Velero code and if I fail to find why the debug log doesn't work I'll probably ask for help in Velero issues/slack and try to merge this MR. It can be corrected later in future anyway.

Also pls note I left 1 comment in code review.

kayrus commented 1 year ago

@Lirt maybe this is the answer?

https://github.com/vmware-tanzu/velero-plugin-for-aws/blob/109ba05302ff24dc61ec1b33121a0a6fff1b3b86/velero-plugin-for-aws/main.go#L27C3-L27C32

I just added this line into main.go

UPD: verified, adding this line did the trick. I don't know whether it's a good idea to call the debugf all the time. There should be a way to detect that the debug logs were triggered, but I don't know yet how.

kayrus commented 1 year ago

@Lirt I found a way to detect the velero debug level. Updated the PR.

Lirt commented 1 year ago

Great and good job! Thanks for increasing code quality of this repo.

I will merge this when you give me :+1:.

kayrus commented 1 year ago

👍