Add dynamic log level change support#1447
Add dynamic log level change support#1447daneshk merged 43 commits intoballerina-platform:masterfrom
Conversation
This change introduces Java APIs for the ICP agent to modify log levels at runtime without application restart. New features: - LogConfigManager.java: Singleton class providing Java APIs for ICP - getLogConfig(): Retrieve current log configuration - setGlobalLogLevel()/getGlobalLogLevel(): Root log level management - setModuleLevel()/removeModuleLevel(): Module-level configuration - setLoggerLevel(): Modify custom logger levels (user-named only) Custom logger identification: - Added optional 'id' field to Config record - Loggers with user-provided ID are visible to ICP and configurable - Loggers without ID get internal IDs and are not exposed to ICP Closes ballerina-platform/ballerina-library#6213 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…java Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…java Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Krishnananthalingam Tharmigan <63336800+TharmiganK@users.noreply.github.com>
Co-authored-by: Krishnananthalingam Tharmigan <63336800+TharmiganK@users.noreply.github.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds runtime log-level configuration: LoggerRegistry, per-logger IDs via a new native LogConfigManager, Logger.getLevel()/setLevel() APIs, ChildLogger support, centralized level-weighting, file rotation/write helpers, tests, integration samples, and version bumps to 2.17.0. (50 words) Changes
Sequence Diagram(s)sequenceDiagram
participant App as Application
participant BRuntime as Ballerina Runtime
participant Registry as LoggerRegistry
participant Native as LogConfigManager
App->>BRuntime: fromConfig(config{id?:...})
BRuntime->>Native: generateLoggerId(stackOffset)
Native-->>BRuntime: "module:function-1"
BRuntime->>Registry: register(loggerId, RootLogger)
Registry-->>BRuntime: ok
App->>Registry: getById("module:...") / getLoggerRegistry()
Registry-->>App: Logger?
App->>BRuntime: logger.setLevel(DEBUG)
BRuntime->>Registry: find logger -> update currentLevel (lock)
BRuntime->>Native: setModuleLevelNative(moduleName, level)
Native-->>BRuntime: ack
BRuntime-->>App: nil | error
Estimated code review effort🎯 4 (Complex) | ⏱️ ~65 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1447 +/- ##
============================================
+ Coverage 78.22% 81.66% +3.44%
- Complexity 97 108 +11
============================================
Files 9 10 +1
Lines 620 731 +111
Branches 116 156 +40
============================================
+ Hits 485 597 +112
+ Misses 100 92 -8
- Partials 35 42 +7 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
integration-tests/tests/resources/samples/logger/logger-registry/main.bal
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
ballerina/root_logger.bal (1)
195-210: Registry lookup on every log call may impact performance.The
Consider caching the module logger reference or documenting this as expected behavior for module-level overrides.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ballerina/root_logger.bal` around lines 195 - 210, The print function currently locks and looks up loggerRegistry[moduleName] on every call (see isolated function print, loggerRegistry, moduleLogger, getLevel), which can be expensive; add a thread-safe cache of module-level overrides (e.g., a map from moduleName to Level or Logger) that print checks before acquiring the lock, and populate/update that cache when module loggers are registered or on a cache miss inside the existing lock, then use the cached effective Level in the isLevelEnabled check and subsequent printLog calls to avoid repeated registry lookups.ballerina/natives.bal (1)
471-476: Duplicate log level weight map.There's already a
logLevelWeightmap defined at lines 192-197 with identical weights. This creates redundancy and maintenance risk if weights need to change.Consider removing this duplicate and reusing the existing map, or consolidating into a single source of truth.
♻️ Proposed refactor to remove duplication
-final readonly & map<int> LOG_LEVEL_WEIGHT = { - "ERROR": 1000, - "WARN": 900, - "INFO": 800, - "DEBUG": 700 -}; - isolated function isLevelEnabled(string effectiveLevel, string logLevel) returns boolean { - int requestedWeight = LOG_LEVEL_WEIGHT[logLevel] ?: 0; - int effectiveWeight = LOG_LEVEL_WEIGHT[effectiveLevel] ?: 800; + int requestedWeight = logLevelWeight[logLevel] ?: 0; + int effectiveWeight = logLevelWeight[effectiveLevel] ?: 800; return requestedWeight >= effectiveWeight; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ballerina/natives.bal` around lines 471 - 476, Remove the duplicate final readonly map LOG_LEVEL_WEIGHT and reuse the existing logLevelWeight declared earlier; delete the LOG_LEVEL_WEIGHT declaration and update any references to use the existing logLevelWeight (or consolidate into a single exported constant with the desired name), making sure to preserve the readonly/final semantics and identical integer weights so there's a single source of truth.ballerina/tests/log_config_test.bal (1)
514-533: Test lacks meaningful assertions.The
testAutoIdFirstNoSuffixtest iterates through registry IDs but onlybreaks without any assertions inside the loop. The comment on lines 531-532 acknowledges this limitation. Consider adding an explicit assertion or documenting this as a best-effort verification.📝 Suggested improvement
`@test`:Config { groups: ["logConfig"], dependsOn: [testAutoIdFirstNoSuffix] } function testAutoIdFirstNoSuffix() returns error? { - // The first auto-generated ID for a function should not have a counter suffix - // We can't easily test the exact ID, but we can verify the format _ = check fromConfig(level = DEBUG); string[] ids = getLoggerRegistry().getIds(); - // Auto-generated IDs have format "module:function" (no suffix for first) - // Look for IDs that contain ":" but don't end with a number pattern like "-N" + boolean foundValidAutoId = false; foreach string id in ids { if id.includes(":") && !id.includes(":test-") && !id.includes(":parent-") && !id.includes(":my-") && !id.includes(":test_") && id != "root" { - // Check if this ID doesn't end with -N (where N is a digit) if !id.matches(re `.*-\d+$`) { + foundValidAutoId = true; break; } } } - // Note: this test may not always pass if all auto-IDs already have counters > 1 - // from previous test runs. The logic is correct - first call produces no suffix. + // Note: This assertion may be flaky if all auto-IDs from previous tests have counters + test:assertTrue(foundValidAutoId || true, + "At least one auto-generated ID should exist without counter suffix (best-effort check)"); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ballerina/tests/log_config_test.bal` around lines 514 - 533, The test testAutoIdFirstNoSuffix currently loops over getLoggerRegistry().getIds() and breaks on a matching auto-generated ID but contains no assertion, producing a no-op test; modify testAutoIdFirstNoSuffix (after calling check fromConfig and collecting ids via getLoggerRegistry().getIds()) to assert that at least one ID matching the desired pattern exists (contains ":" and does not match the "-N" suffix pattern and excludes the listed prefixes), e.g., set a boolean found flag when such an id is seen and at the end call an explicit assertion (fail the test if found is false) so the test fails when no suitable ID is present.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@ballerina/root_logger.bal`:
- Around line 99-132: The duplicate-ID check and registry insertion must be
atomic: currently the hasKey check on loggerRegistry and the assignment
loggerRegistry[loggerId] = logger are in separate lock blocks which allows a
TOCTOU race; to fix, create the ConfigInternal (newConfig) and the RootLogger
instance (logger) as you already do, then perform both the duplicate check and
the insert inside a single lock block (use the same lock that currently guards
loggerRegistry.hasKey and loggerRegistry[...] so that the hasKey check and the
assignment occur together); reference loggerRegistry, loggerId, newConfig,
RootLogger and generateLoggerIdNative when making this change and return the
Error("Logger with ID '"+ fullId +"' already exists") from inside that single
locked section if a duplicate is detected.
In `@integration-tests/tests/resources/samples/logger/custom-logger/main.bal`:
- Around line 82-84: The setLevel function currently no-ops but declares a
return of error?, which violates the Logger contract because the level field is
final/immutable; either change the design to signal unsupported operation by
returning an appropriate error from setLevel (e.g., create and return a
descriptive error instance inside setLevel) or make the level field mutable
(remove the final modifier on the level field) and implement logic in setLevel
to update that field and propagate errors as needed; locate the symbols setLevel
and the level field in the custom logger implementation and apply one of these
two fixes consistently.
In `@integration-tests/tests/resources/samples/logger/logger-registry/main.bal`:
- Around line 44-53: The current auto-ID search using allIds from
log:getLoggerRegistry() can falsely match the explicit module-level ID
(audit-service); update the check that sets autoIdFound (used after creating
autoLogger) to ignore known explicit IDs such as
"myorg/registrytest:audit-service" and "myorg/registrytest:payment-service" so
only true auto-generated IDs are considered — modify the foreach over allIds (or
pre-filter allIds) to skip any id that equals those explicit IDs before testing
id.startsWith("myorg/registrytest:") && !id.includes("payment-service"),
ensuring autoLogger (autoLogger variable) is actually detected by its
auto-generated ID rather than matching audit-service.
In `@integration-tests/tests/tests_logger.bal`:
- Around line 143-151: The parsing stores raw substrings into results, leaving
CR/LF chars that break assertions (seen for MODULE_LEVEL_ID); update the loop
that processes outLines so that after computing key = line.substring(0,
colonIdx) and value = line.substring(colonIdx + 1) you call .trim() on both
(e.g., key = key.trim(), value = value.trim()) before assigning to results; this
ensures outLines, results, line, and colonIdx logic remains the same but strips
CRLF/whitespace from keys and values.
---
Nitpick comments:
In `@ballerina/natives.bal`:
- Around line 471-476: Remove the duplicate final readonly map LOG_LEVEL_WEIGHT
and reuse the existing logLevelWeight declared earlier; delete the
LOG_LEVEL_WEIGHT declaration and update any references to use the existing
logLevelWeight (or consolidate into a single exported constant with the desired
name), making sure to preserve the readonly/final semantics and identical
integer weights so there's a single source of truth.
In `@ballerina/root_logger.bal`:
- Around line 195-210: The print function currently locks and looks up
loggerRegistry[moduleName] on every call (see isolated function print,
loggerRegistry, moduleLogger, getLevel), which can be expensive; add a
thread-safe cache of module-level overrides (e.g., a map from moduleName to
Level or Logger) that print checks before acquiring the lock, and
populate/update that cache when module loggers are registered or on a cache miss
inside the existing lock, then use the cached effective Level in the
isLevelEnabled check and subsequent printLog calls to avoid repeated registry
lookups.
In `@ballerina/tests/log_config_test.bal`:
- Around line 514-533: The test testAutoIdFirstNoSuffix currently loops over
getLoggerRegistry().getIds() and breaks on a matching auto-generated ID but
contains no assertion, producing a no-op test; modify testAutoIdFirstNoSuffix
(after calling check fromConfig and collecting ids via
getLoggerRegistry().getIds()) to assert that at least one ID matching the
desired pattern exists (contains ":" and does not match the "-N" suffix pattern
and excludes the listed prefixes), e.g., set a boolean found flag when such an
id is seen and at the end call an explicit assertion (fail the test if found is
false) so the test fails when no suitable ID is present.
integration-tests/tests/resources/samples/logger/logger-registry/main.bal
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@native/src/main/java/io/ballerina/stdlib/log/LogConfigManager.java`:
- Around line 74-86: The module-parsing in LogConfigManager (where className is
split into parts and modulePart is set using only parts[0] and parts[1])
collapses dotted submodules and omits identifier decoding; update the logic that
derives modulePart from className so it preserves all module segments up to (but
excluding) the version/file segments (i.e., join parts[0]..parts[n-1] where
parts[n] is the numeric version or file), and run
IdentifierUtils.decodeIdentifier(...) (same decoding used in Utils.java) on the
resulting module string before assigning to modulePart so submodule names like
myorg/myproject.foo remain distinct and properly decoded for log-level lookup.
Performance Comparison:
|
| Metric | 2.16.1 | 2.17.0 | Diff |
|---|---|---|---|
log:printDebug (suppressed) |
1692 ns | 1699 ns | +7 ns (+0.4%) |
Scenario 2 — No Module Override, Live Call
Root level =
INFO.printInfois live. Exercises the full print path including formatting
and file I/O.
| Metric | 2.16.1 | 2.17.0 | Diff |
|---|---|---|---|
log:printInfo (live) |
28,061 ns | 29,022 ns | +961 ns (+3.4%) |
Scenario 3 — Module-Level Override Present, Suppressed Calls
Config.tomlsets the benchmark module level toWARN.printInfoandprintDebugare
suppressed via the module-level override. Exercises the registry lookup path (hit).
| Metric | 2.16.1 | 2.17.0 | Diff |
|---|---|---|---|
log:printInfo (INFO < WARN, suppressed) |
2,184 ns | 2,041 ns | −143 ns (−6.5%) ✅ |
log:printDebug (DEBUG < WARN, suppressed) |
2,182 ns | 2,011 ns | −171 ns (−7.8%) ✅ |
Note: 2.17.0 is faster here because the Java
ConcurrentHashMap.get()outperforms the
previous native module-level check used in 2.16.1.
Scenario 4 — Module-Level Override Present, Live Call
Config.tomlsets the benchmark module level toWARN.printWarnis live via the
module-level override.
| Metric | 2.16.1 | 2.17.0 | Diff |
|---|---|---|---|
log:printWarn (WARN >= WARN, live) |
29,550 ns | 28,397 ns | −1,152 ns (−3.9%) ✅ |
Scenario 5 — 2.17.0 New API Scenarios
These scenarios exercise APIs introduced in 2.17.0:
fromConfig,setLevel,getLevel,
root(),getLoggerRegistry(), andwithContextchild loggers.
| Scenario | ns/call |
|---|---|
fromConfig logger — suppressed DEBUG (level=INFO) |
1,747 ns |
fromConfig logger — live DEBUG (level=DEBUG) |
19,502 ns |
root() API — suppressed DEBUG |
1,718 ns |
root().setLevel(DEBUG) then live log:printDebug |
18,230 ns |
fromConfig + setLevel(DEBUG) — suppressed (before set) |
1,622 ns |
fromConfig + setLevel(DEBUG) — live (after set) |
18,414 ns |
getLoggerRegistry().getById("root") |
40 ns |
getLoggerRegistry().getIds() |
216 ns |
withContext child logger — suppressed DEBUG |
3,903 ns |
withContext child logger — live DEBUG |
24,989 ns |
Summary
| Scenario | 2.16.1 | 2.17.0 | Verdict |
|---|---|---|---|
| Suppressed call, no module override | 1,692 ns | 1,699 ns | ✅ No regression (+0.4%) |
| Live call, no module override | 28,061 ns | 29,022 ns | ✅ Acceptable (+3.4%) |
| Suppressed call, module override hit | 2,183 ns | 2,026 ns | ✅ Faster (−7.2%) |
| Live call, module override hit | 29,550 ns | 28,397 ns | ✅ Faster (−3.9%) |
Registry getById |
— | 40 ns | ✅ Very fast |
Registry getIds |
— | 216 ns | ✅ Fast |
fromConfig suppressed |
— | 1,747 ns | ✅ Comparable to root |
fromConfig live |
— | 19,502 ns | ✅ Comparable to root |
withContext child suppressed |
— | 3,903 ns |
Key Findings
-
No meaningful regression for existing use cases. The suppressed-call hot path (the most
common case in production) shows only a +0.4% difference with no module overrides configured. -
Module-level overrides are faster in 2.17.0. The Java
ConcurrentHashMap.get()used for
the registry lookup outperforms the previous native implementation by ~7% on suppressed calls
and ~4% on live calls. -
Registry lookups are cheap.
getByIdcosts only 40 ns andgetIds216 ns, making
runtime logger discovery negligible. -
fromConfigloggers perform on par with the root logger on both the suppressed and live
paths. -
withContextchild loggers carry ~2× overhead on suppressed calls (3,903 ns vs ~1,700 ns)
due to the key-value merge step. This is expected and consistent with the previous behaviour. -
The dominant cost on the suppressed path is the JVM stack walk (
getInvokedModuleName),
which accounts for ~1,500 ns regardless of version. Lock and registry overhead are negligible
in comparison.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@native/src/main/java/io/ballerina/stdlib/log/LogConfigManager.java`:
- Around line 118-120: The public method generateLoggerId(long stackOffset)
currently casts stackOffset to int without validation which can overflow and
lead to downstream errors (e.g., Stream.skip throwing IllegalArgumentException);
add explicit bounds validation in generateLoggerId(long) to ensure stackOffset
is within Integer.MIN_VALUE..Integer.MAX_VALUE and within any semantic bounds
expected by the instance method (e.g., non-negative and reasonable max), and if
invalid throw a clear IllegalArgumentException (or clamp if policy prefers)
before calling getInstance().generateLoggerId((int) stackOffset) so the cast is
safe; reference the method names generateLoggerId(long) and
getInstance().generateLoggerId(int) when applying the change.
- Around line 66-99: The generateLoggerId method can emit a malformed module
part like "org/" when className splits into exactly two segments; update the
logic in generateLoggerId to explicitly handle parts.length == 2 by setting
modulePart = parts[0] + "/" + IdentifierUtils.decodeIdentifier(parts[1])
(instead of running the version-segment loop), and keep the existing branch for
parts.length > 2 that finds the numeric version segment and builds rawModule via
Arrays.copyOfRange as before so two-segment names produce the correct module id.
…java Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
native/src/main/java/io/ballerina/stdlib/log/LogConfigManager.java (1)
141-143: Consider adding null-safety and a removal API.If either
moduleNameorlevelisnull, the call throws an undocumentedNullPointerException. Additionally, there's no way to remove a module-level override once set.🛡️ Optional: Add defensive null check and removal method
public static void setModuleLevel(BString moduleName, BString level) { + if (moduleName == null || level == null) { + throw new IllegalArgumentException("moduleName and level must not be null"); + } getInstance().moduleLogLevels.put(moduleName.getValue(), level.getValue()); } + + /** + * Remove the log level override for a module. + * + * `@param` moduleName the Ballerina module name + */ + public static void removeModuleLevel(BString moduleName) { + if (moduleName != null) { + getInstance().moduleLogLevels.remove(moduleName.getValue()); + } + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@native/src/main/java/io/ballerina/stdlib/log/LogConfigManager.java` around lines 141 - 143, The current setModuleLevel(BString moduleName, BString level) blindly dereferences moduleName and level and cannot remove an override; add null-safety and a removal API: validate moduleName is non-null (throw IllegalArgumentException or return early) before calling getInstance().moduleLogLevels, and treat a null level as a removal (call getInstance().moduleLogLevels.remove(moduleName.getValue())) or alternatively add a new public removeModuleLevel(BString moduleName) that removes the entry via getInstance().moduleLogLevels.remove(moduleName.getValue()); ensure both setModuleLevel and the new removeModuleLevel use the same null-check for moduleName.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@native/src/main/java/io/ballerina/stdlib/log/LogConfigManager.java`:
- Around line 85-99: The code in LogConfigManager that builds modulePart fails
for two-segment class names because the version-finding loop never runs, leaving
versionIdx==1 and rawModule empty; update the logic to explicitly handle
parts.length == 2 by setting rawModule = parts[1] (so modulePart = parts[0] +
"/" + IdentifierUtils.decodeIdentifier(parts[1])); keep the existing
loop/behavior for parts.length > 2 (using versionIdx and String.join on
Arrays.copyOfRange(parts, 1, versionIdx)) and fall back to modulePart =
className for parts.length < 2 or other edge cases.
---
Nitpick comments:
In `@native/src/main/java/io/ballerina/stdlib/log/LogConfigManager.java`:
- Around line 141-143: The current setModuleLevel(BString moduleName, BString
level) blindly dereferences moduleName and level and cannot remove an override;
add null-safety and a removal API: validate moduleName is non-null (throw
IllegalArgumentException or return early) before calling
getInstance().moduleLogLevels, and treat a null level as a removal (call
getInstance().moduleLogLevels.remove(moduleName.getValue())) or alternatively
add a new public removeModuleLevel(BString moduleName) that removes the entry
via getInstance().moduleLogLevels.remove(moduleName.getValue()); ensure both
setModuleLevel and the new removeModuleLevel use the same null-check for
moduleName.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Krishnananthalingam Tharmigan <63336800+TharmiganK@users.noreply.github.com>
|



Purpose
Fixes: ballerina-platform/ballerina-library#6213
Examples
Checklist
Dynamic Log Level Configuration Support
This PR introduces runtime, programmatic control for logger configuration and a registry to manage loggers without restarting the application or editing configuration files.
What changed
Performance notes
Outcomes