neondatabase / autoscaling

Postgres vertical autoscaling in k8s
Apache License 2.0
153 stars 21 forks source link

neonvm-controller: Fix log sampling #968

Closed sharnoff closed 3 months ago

sharnoff commented 3 months ago

Fixes #962.

The previous attempted fix was in #947, but that didn't actually work.

As it turns out, this assumption:

There's no direct way to go from a zap.Logger to a logr.Logger

was actually false; we can just use zapr, which is what controller-runtime uses internally.


Tested locally using the reproduction steps from #962.

Omrigan commented 3 months ago

How exactly does this fix #962, and why the previous attempt didn't work?

sharnoff commented 3 months ago

The only thing I can imagine is that log sampling is probably implemented outside of zapcore.Core, but I'm not actually sure.

This fixes #962 by fully building the *zap.Logger ourselves, taking the controller-runtime zap package out of the loop.

Not sure if that answers your questions, happy to elaborate and/or dig further :sweat_smile: