Skip to content

pkg/settings/limits: raise missing org ID tenant log level from warn to error#2263

Open
jmank88 wants to merge 1 commit into
mainfrom
org-err
Open

pkg/settings/limits: raise missing org ID tenant log level from warn to error#2263
jmank88 wants to merge 1 commit into
mainfrom
org-err

Conversation

@jmank88

@jmank88 jmank88 commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Towards https://smartcontract-it.atlassian.net/browse/CRE-1707

Raise level, and add logs for when just doing a setting lookup, which was a silent failure before via omission of the key.

I also cleaned up some production loggers being used from tests (which just spams std out in real time, even when a test is passing)

@github-actions

github-actions Bot commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

✅ API Diff Results - github.com/smartcontractkit/chainlink-common

✅ Compatible Changes (3)

pkg/loop.(*AtomicSettings) (1)
  • SetGetter — ➕ Added
pkg/loop.AtomicSettings (1)
  • Lggr — ➕ Added
pkg/settings (1)
  • GetterConfig — ➕ Added

📄 View full apidiff report

if !b.scope.IsTenantRequired() {
kvs := contexts.CREValue(ctx).LoggerKVs()
b.lggr.Warnw("Unable to get scoped bound limit due to missing tenant: failing open", append([]any{"scope", b.scope}, kvs...)...)
b.lggr.Errorw("Unable to get scoped bound limit due to missing tenant: failing open", append([]any{"scope", b.scope}, kvs...)...)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As discussed offline, will this just spam the logs if the limit in question was designed to only work at a global scope?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, global limits won't enter this block b/c of the check at the top.

@jmank88
jmank88 force-pushed the org-err branch 2 times, most recently from 1677104 to c5d3052 Compare July 21, 2026 14:56
@jmank88
jmank88 marked this pull request as ready for review July 21, 2026 15:37
@jmank88
jmank88 requested review from a team as code owners July 21, 2026 15:37
@jmank88
jmank88 requested review from bolekk and removed request for thomaska July 21, 2026 15:37
Comment thread pkg/settings/toml.go
if ok && tme.Scope.IsTenantRequired() {
return "", fmt.Errorf("failed to get raw keys: %w", err)
} else {
t.lggr.Errorf("Settings lookup for "+key+" limited by missing tenant at scope "+tme.Scope.String(), "key", key, "err", err)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@prashantkumar1982 These could be noisier than the other kind, since they are per lookup, but there are still not false positives.

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.

3 participants