Add logging guidelines

[liu-cong]

This is the first attempt to define logging guidelines in our current code base. This is not meant to be perfect/permanent, but something that can improve our logging quickly.

I am happy to follow up with a PR to update logging once this is approved.

Related issue: #108

cc @danehans @ahg-g

[kfswain]

This looks reasonable. Can we give a crisp name to each log level?

That would let us make some enums that we can expect to be used as the log level params. It'll give us semantics within the code that makes readability, and following this style decision a tad easier.

[liu-cong]

This looks reasonable. Can we give a crisp name to each log level?

I added the definitions from the k8s logging guidelines. They didn't define "crisp names" but I find it pretty helpful to follow. Pls suggest names if you have better ideas. Thanks!

[kfswain]

I wonder if we do this anywhere else in K8s but we could do something like:

const(
REQUIRED=0
DEFAULT=1
VERBOSE_DEFAULT=2
DETAILED=3
DEBUG=5
TRACE=5
)

Names are a quick attempt and probably suck. But this would make reasoning about what log level to use a little easier and keeps someone from having to constantly refer to our dev guide in the middle of development.

[liu-cong]

I wonder if we do this anywhere else in K8s but we could do something like:

const(
REQUIRED=0
DEFAULT=1
VERBOSE_DEFAULT=2
DETAILED=3
DEBUG=5
TRACE=5
)

Names are a quick attempt and probably suck. But this would make reasoning about what log level to use a little easier and keeps someone from having to constantly refer to our dev guide in the middle of development.

I like it. I think it covers most cases, and people can refer to the dev guide for more nuanced cases if unsure. Will wait to see if others have any suggestions, otherwise I will update the doc.

[danehans]

A few non-blocking nits.

/lgtm

[danehans]

@liu-cong you should also consider including a formatting subsection to resolve 2 from #108.

[netlify]

✅ Deploy Preview for gateway-api-inference-extension ready!

Name	Link
🔨 Latest commit	c0f4727
🔍 Latest deploy log	https://app.netlify.com/sites/gateway-api-inference-extension/deploys/67893dfda26d55000839f043
😎 Deploy Preview	https://deploy-preview-182--gateway-api-inference-extension.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site configuration.

[liu-cong]

@liu-cong you should also consider including a formatting subsection to resolve 2 from #108.

@danehans I am skipping this for now. Also I think we can better address point 2 by creating a logger wrapper in common places to include those context.

@kfswain I slightly changed the logging level constant names you suggested. PTAL!

[ahg-g]

This should be logged as error so that it shows up clearly in the logs for the platform admin

[liu-cong]

This loop runs very frequently (current default is 20ms). Even some transient error will create a spam. So I am suggesting using DEBUG for any logging here.

I think to solve the error visibility problem, we can have that in the debug loop under the "Misc" section below, which runs every 5s. Wdyt?

[ahg-g]

ok, we can discuss on the specific implementation PRs.

[kfswain]

Small comment, but otherwise LGTM! Thanks for doing this, I'll put a hold as well so others can still make a pass, fix small nits, what have you. But feel free to pull the hold off at your leisure.

/hold
/lgtm
/approve

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: danehans, kfswain, liu-cong

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [kfswain]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[ahg-g]

/lgtm

[liu-cong]

/unhold