search

gravitational / teleport

The easiest, and most secure way to access and protect all of your infrastructure.
https://goteleport.com
GNU Affero General Public License v3.0
17.66k stars 1.77k forks source link

Log kind in reconciler logs #49293

Open kopiczko opened 2 days ago

aws-amplify-us-west-2[bot] commented 2 days ago

This pull request is automatically being deployed by Amplify Hosting (learn more).

Access this pull request here: https://pr-49293.d3pp5qlev8mo18.amplifyapp.com

kopiczko commented 1 day ago

@smallinsky I reverted to my previous version because of https://github.com/gravitational/teleport/pull/49293/files#r1851984320. Please see if you're ok with it.

kopiczko commented 1 day ago

Is this really needed? The logger is already configurable, and can be adorned with the kind when it is passed in to the reconciler. This could now cause an additional kind attribute to show up in the logs.

See:

https://github.com/gravitational/teleport/blob/master/lib/srv/app/watcher.go#L44 https://github.com/gravitational/teleport/blob/master/lib/srv/db/watcher.go#L47

I mean, it isn't needed because as you said there is this With function on the Logger. I didn't figure it's being used in so many places. I proposed those changes because Okta-related reconcilers don't log kind and names are pretty cryptic.

I'm happy to remove all the currently used With("kind", ...) calls if you think it's ok. It shouldn't be too difficult to grep for them. Alternatively, I can replace this PR with adding With calls in Okta reconcilers. I think having kind added automatically by the Reconciler is nicer, but I won't insist.

@rosstimothy please let me know your thoughts.

rosstimothy commented 1 day ago

It may be small in the grand scheme of things, but the biggest benefit to users manually annotating the kind by calling With on the provided logger to the reconciler is that the argument is only formatted a single time, instead of being formatted each time a log is emitted. If we can automatically annotate the logger with kind a single time via With instead of adding "kind", kind to each logging statement let's do it.