In the following code pattern, the log message will get logged with v=0 in JSON
output although conceptually it has a higher verbosity:
if klog.V(5).Enabled() {
klog.Info("hello world")
}
Having the actual verbosity in the JSON output is relevant, for example for
filtering out only the important info messages. The solution is to use
klog.V(5).Info or something similar.
Whether the outer if is necessary at all depends on how complex the parameters
are. The return value of klog.V can be captured in a variable and be used
multiple times to avoid the overhead for that function call and to avoid
repeating the verbosity level.
The field names in godoc for various core storage structs have been
corrected with this commit.
Additional Ref# #105963 (comment)
Signed-off-by: Humble Chirammal <hchiramm@redhat.com>
The API was deprecated in 1.23 when output/v1alpha2 was
added. v1alpha1 is problematic since it embeds kubeadm/v1beta2
BootstrapToken related types directly. v1alpha2 imports
a new group dedicated to bootstrap tokens apis/bootstraptoken.
The `rest_client_request_duration_seconds` and
`rest_client_rate_limiter_duration_seconds` metrics have a url label
that used to contain the whole uri of the request. This is very
dangerous and can lead to cardinality explosions since its values aren't
bounded. We don't really need to expose the whole uri since these
metrics are used to mesure the availability of the different proxy in
front the apiserver. The most valuable information is the host to be
able to differentiate between the different proxy. In the future, we
might also want to add the path to be able to add some granularity, but
since there is no immediate use case for that, so there is no need to
add it now.
Signed-off-by: Damien Grisonnet <dgrisonn@redhat.com>
We don't need to worry about data loss once the data has been written to an
output stream. Calling fsync unnecessarily has been the reason for performance
issues in the past.
The recent regression https://github.com/kubernetes/kubernetes/issues/107033
shows that we need a way to automatically measure different logging
configurations (structured text, JSON with and without split streams) under
realistic conditions (time stamping, caller identification).
System calls may affect the performance and thus writing into actual files is
useful. A temp dir under /tmp (usually a tmpfs) is used, so the actual IO
bandwidth shouldn't affect the outcome. The "normal" json.Factory code is used
to construct the JSON logger when we have actual files that can be set as
os.Stderr and os.Stdout, thus making this as realistic as possible.
When discarding the output instead of writing it, the focus is more on the rest
of the pipeline and changes there can be investigated more reliably.
The benchmarks automatically gather "log entries per second" and "bytes per
second", which is useful to know when considering requirements like the ones
from https://github.com/kubernetes/kubernetes/issues/107029.
logcheck complains:
Additional arguments to ErrorS should always be Key Value pairs. Please check if there is any key or value missing.
That check is intentional, but not applicable here. The check can be worked
around by calling the functions through variables.
The benchmark depends on k8s.io/api (for v1.Container). Such a dependency is
not desirable for k8s.io/component-base/logs, even if it's just for
testing. The solution is to create a separate directory where such a dependency
isn't a problem.
The alternative, a separate package with its own go.mod file under
k8s.io/component-base/logs wouldd have been more complicated to maintain (yet
another go.mod file and different whitelisted dependencies).
The benchmark reads a JSON log file and measures how long it takes to re-encode
it. The focus is on the encoding of message and values, therefore additional
work (time stamping, caller, writing to file) gets avoided.
The encoder configuration can now be chosen by the caller. This will be used by
a benchmark to write messages without caller and time stamp.
While at it, some places where the logger was unnecessarily tested with split
output streams writing into the same actual stream were replaced with writing
as single stream. This is a leftover from a previous incarnation of the split
output stream patch where identical streams were used instead of nil for the
error stream to indicate "single stream".
