Skip to content

fix(log): allow disabling the zap duplicate-message sampler#292

Open
armru wants to merge 2 commits into
mainfrom
feat/configurable-log-sampling
Open

fix(log): allow disabling the zap duplicate-message sampler#292
armru wants to merge 2 commits into
mainfrom
feat/configurable-log-sampling

Conversation

@armru

@armru armru commented Jul 6, 2026

Copy link
Copy Markdown
Member

The controller-runtime zap builder installs a sampler on every production-mode logger: only the first 100 messages sharing the same (level, message) pair each second get through, and roughly 1 in 100 afterwards. That is the right default for an operator, where a reconciliation storm can repeat the same message beyond any useful rate, but it is wrong for processes whose output must be complete. The CloudNativePG instance manager forwards every PostgreSQL log record with a constant message, so a burst of database activity (for example pgaudit under load) is silently collapsed by the sampler and most records never reach the pod's log stream (see cloudnative-pg/cloudnative-pg#10815).

This adds an optional WithDisabledSampling() argument to Flags.ConfigureLogging. The builder only installs the sampler when the configured level does not enable zapcore.Level(-2), so the option passes a fully-verbose level to make the builder skip it and then restores the level the user asked for by filtering the core with zap.IncreaseLevel. Everything else stays in the controller-runtime builder's hands (encoder, --zap-* flags, destination), so the unsampled logger's output is identical to the default one. Callers that don't pass the option keep the sampler exactly as before.

The new specs in pkg/log/flags_test.go exercise the real path, from flag parsing through ConfigureLogging to the --log-destination file: the default stays sampled, a 300-message burst is fully emitted with the option, --log-level filtering still applies, the trace level (where the builder never installs a sampler) keeps working, and the field-remapping flags are honored. The burst spec doubles as a tripwire if a future controller-runtime bump changes how the sampler is installed.

@armru
armru requested a review from a team as a code owner July 6, 2026 10:09
armru added a commit to cloudnative-pg/cloudnative-pg that referenced this pull request Jul 6, 2026
Track the latest commit of cloudnative-pg/machinery#292 in both
modules, and promote klog to a direct dependency now that the manager
tests import it.

Signed-off-by: Armando Ruocco <armando.ruocco@enterprisedb.com>
@armru

armru commented Jul 6, 2026

Copy link
Copy Markdown
Member Author

/test

@armru armru changed the title feat(log): allow disabling the zap duplicate-message sampler fix(log): allow disabling the zap duplicate-message sampler Jul 6, 2026
The controller-runtime zap builder installs a sampler on production-mode
loggers that passes only the first 100 messages sharing the same (level,
message) pair each second, and roughly 1 in 100 afterwards. That is fine
for a controller's reconcile noise, but it silently drops records from
processes whose output must be complete, such as the instance manager
forwarding the PostgreSQL log stream, where every record is logged with
the same message.

Add a WithDisabledSampling option to Flags.ConfigureLogging. The builder
only installs the sampler when the configured level does not enable
zapcore.Level(-2), so the option passes a fully-verbose level to make the
builder skip it and then restores the requested level by filtering the
core.

Signed-off-by: Armando Ruocco <armando.ruocco@enterprisedb.com>
@NiccoloFei
NiccoloFei force-pushed the feat/configurable-log-sampling branch from 1f63480 to 12f2c0c Compare July 24, 2026 13:59
Signed-off-by: Niccolò Fei <niccolo.fei@enterprisedb.com>
@NiccoloFei
NiccoloFei force-pushed the feat/configurable-log-sampling branch from 785522e to 61daa1c Compare July 24, 2026 14:55
NiccoloFei pushed a commit to cloudnative-pg/cloudnative-pg that referenced this pull request Jul 24, 2026
Replace the local machinery checkout with the pseudo-version of the
commit opened as cloudnative-pg/machinery#292, so CI builds against the
same code. To be bumped to the tagged machinery release once available.

Signed-off-by: Armando Ruocco <armando.ruocco@enterprisedb.com>
NiccoloFei pushed a commit to cloudnative-pg/cloudnative-pg that referenced this pull request Jul 24, 2026
Track the latest commit of cloudnative-pg/machinery#292 in both
modules, and promote klog to a direct dependency now that the manager
tests import it.

Signed-off-by: Armando Ruocco <armando.ruocco@enterprisedb.com>
NiccoloFei pushed a commit to cloudnative-pg/cloudnative-pg that referenced this pull request Jul 24, 2026
Replace the local machinery checkout with the pseudo-version of the
commit opened as cloudnative-pg/machinery#292, so CI builds against the
same code. To be bumped to the tagged machinery release once available.

Signed-off-by: Armando Ruocco <armando.ruocco@enterprisedb.com>
NiccoloFei pushed a commit to cloudnative-pg/cloudnative-pg that referenced this pull request Jul 24, 2026
Track the latest commit of cloudnative-pg/machinery#292 in both
modules, and promote klog to a direct dependency now that the manager
tests import it.

Signed-off-by: Armando Ruocco <armando.ruocco@enterprisedb.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants