Fix Gorouter's debugserver endpoint to allow log level changes at runtime #452
Conversation
| "go.uber.org/zap" | ||
| "go.uber.org/zap/exp/zapslog" | ||
| "go.uber.org/zap/zapcore" | ||
| ) | ||
|
|
||
| var ( | ||
| conf dynamicLoggingConfig | ||
| Conf DynamicLoggingConfig |
There was a problem hiding this comment.
why is this exported now? I assume for the change in main.go?
exposing the variable means anyone anywhere can change it. what about adding a getter function instead?
There was a problem hiding this comment.
We need the satisfy the debugserver's reconfigurable sink: https://github.com/cloudfoundry/debugserver/blob/main/server.go#L24-L26
The base gorouter logger is inited at the package level and privately puts us in a bit of an odd spot with further changes to the logger. I exported the config as we need the debugserver to actually change the config via SetMinLevel()
Basically since the grlogger.baseLogger is initialized on importation via init() and private to the package, I pushed the config out as public as we need something to satisfy the ReconfigurableSinkInterface. A getter doesn't quite fit the pattern. There is an argument for pushing it further down, but this was a bit simpler.
I went with the config as it's the area where adding the method to satisfy the ReconfigurableSinkInterface was the least intrusive.
MarcPaquette
force-pushed
the
WIP_debugserver-loglevel
branch
from
9a9826f to
3ec0125
Compare
| mutex.Lock() | ||
| defer mutex.Unlock() |
There was a problem hiding this comment.
@ameowlia
I added the Mutex here since there are a few different ways the logging level can get changed.
|
@ameowlia Is this okay to merge in now? |
We have a debugserver in gorouter + explicit code trying to support reconfiguring the log level on the fly. Being able to do this helps operators + support troubleshoot live issues without having to restart Gorouters. This has been broken since ~2017.
This change allows CF Administrators to change gorouter's logging on the fly to triage issue in much finer detail.
This is related to the change in the debug server cloudfoundry/debugserver#72