Skip to content

featurecontrol: log auto-gomemlimit activation at INFO - #5415

Open
byBrodowski wants to merge 1 commit into
prometheus:mainfrom
byBrodowski:featurecontrol-gomemlimit-info
Open

featurecontrol: log auto-gomemlimit activation at INFO#5415
byBrodowski wants to merge 1 commit into
prometheus:mainfrom
byBrodowski:featurecontrol-gomemlimit-info

Conversation

@byBrodowski

Copy link
Copy Markdown

Narrowing #5412 to the one part that isn't a matter of taste.

@TheMeier's point in the issue is fair: whether a warning-heavy start-up log is a problem at all
depends on how you read logs, and I'm not going to argue anyone out of their preference. But the
agreement in that reply — that auto-gomemlimit should be promoted — doesn't depend on taste, so
here it is as a one-line change instead of a longer discussion.

auto-gomemlimit is how the Go heap learns about a container memory limit. In a memory-limited
container, running without it is the worse of the two configurations. Logging its activation at
WARN therefore announces the safer choice as a caution, which is backwards regardless of how
loudly you think a log should speak.

What this changes

One switch branch in featurecontrol/featurecontrol.go:

case FeatureAutoGOMEMLIMIT:
	opts = append(opts, enableAutoGOMEMLIMIT())
-	logger.Warn("Automatically set GOMEMLIMIT to match the Linux container or system memory limit.")
+	logger.Info("Automatically set GOMEMLIMIT to match the Linux container or system memory limit.")

The message text is unchanged. Each feature has its own branch with its own log call, so this
reaches nothing else — in particular the unbounded-cardinality metrics flags
(alert-names-in-metrics, group-key-in-metrics) keep WARN, which I think is right: their
message carries a real operational cost, so a caution is the correct level for them. The
Experimental … messages also keep WARN. I've deliberately left the wider question in #5412
alone; if a maintainer wants the same treatment for utf8-strict-mode, that's a separate call and
I'm happy to follow up.

Verification

go build, go test and go vet on ./featurecontrol/ pass with the change applied (Go 1.25,
matching go.mod). featurecontrol_test.go asserts the resulting flag state and makes no
assertion about log level, so it neither needed nor received a change — the behaviour under test is
identical.

Pull Request Checklist

Which user-facing changes does this PR introduce?

[CHANGE] Log activation of the `auto-gomemlimit` feature flag at INFO instead of WARN. #5412

auto-gomemlimit is not experimental in effect: it is how the Go heap
learns about a container memory limit, and running without it inside a
limited container is the worse choice. Logging its activation at WARN
announces the safer configuration as a caution, so a deliberately
configured start-up emits a warning line with nothing wrong.

Log that one activation at INFO. Every other branch is left at WARN,
including the unbounded-cardinality metrics flags, whose message does
carry an operational caution. The message text is unchanged.

Fixes prometheus#5412

Signed-off-by: brodowski <by.brodowski@pm.me>
@coderabbitai

coderabbitai Bot commented Jul 31, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 486c7bc8-4e08-4910-9b99-ff328d187912

📥 Commits

Reviewing files that changed from the base of the PR and between 846bdbf and 9fc93a9.

📒 Files selected for processing (1)
  • featurecontrol/featurecontrol.go

📝 Walkthrough

Walkthrough

The GOMEMLIMIT feature activation message in NewFlags now uses info-level logging instead of warning-level logging.

Changes

Feature control logging

Layer / File(s) Summary
Change GOMEMLIMIT activation logging
featurecontrol/featurecontrol.go
NewFlags logs GOMEMLIMIT activation with Info instead of Warn.

Estimated code review effort: 1 (Trivial) | ~2 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the feature area and the specific log-level change.
Description check ✅ Passed The description explains the change, scope, verification, checklist status, and release-note impact.
Linked Issues check ✅ Passed The change satisfies the applicable objective in [#5412] by logging auto-gomemlimit activation at INFO while preserving warning levels for cautionary flags.
Out of Scope Changes check ✅ Passed The one-line change is limited to auto-gomemlimit logging and matches the stated pull request scope.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@TheMeier

Copy link
Copy Markdown
Contributor

Thanks @byBrodowski. By promtion I meant move it from a feature-flag to a normal config flag

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.

featurecontrol: feature-flag activation is logged at WARN, so a clean start-up emits warnings

2 participants