Skip to content

Comments

Add dynamic log level change support#1447

Merged
daneshk merged 43 commits intoballerina-platform:masterfrom
daneshk:dynamic_log_config
Feb 20, 2026
Merged

Add dynamic log level change support#1447
daneshk merged 43 commits intoballerina-platform:masterfrom
daneshk:dynamic_log_config

Conversation

@daneshk
Copy link
Member

@daneshk daneshk commented Feb 17, 2026

Purpose

Fixes: ballerina-platform/ballerina-library#6213

Examples

Checklist

  • Linked to an issue
  • Updated the changelog
  • Added tests
  • Updated the spec
  • Checked native-image compatibility

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

  • Runtime APIs:
    • Logger.getLevel() to query the effective level.
    • Logger.setLevel(Level) to change levels at runtime (errors on unsupported child-logger operations).
  • Logger registry:
    • New LoggerRegistry singleton with getIds() and getById(id) to discover and retrieve loggers.
    • Support for explicit Config.id and auto-generated module:function-based IDs; module-prefixed IDs for module-level loggers.
    • Child loggers inherit parent levels and are not registered independently.
  • Native integration:
    • New Java singleton LogConfigManager for logger ID generation and module-level level storage.
    • New native bindings: generateLoggerIdNative, getModuleLevelNative, setModuleLevelNative to coordinate Ballerina ⇄ Java runtime behavior.
  • Logging internals:
    • Centralized effective-level evaluation supporting module overrides.
    • New ChildLogger type and refactored printLog flow for context merging, formatting, rotation, and multi-destination writes.
    • Thread-safe registry operations and level updates; module-level changes synchronized with native side.
  • Tests, docs and packaging:
    • Extensive tests covering registry behavior, get/set level, inheritance, ID generation, and edge cases.
    • Integration sample and spec/docs updated to document new APIs and Config.id/enableSensitiveDataMasking fields.
    • Version bumps to 2.17.0 and related dependency updates; SpotBugs exclusion added for the singleton.

Performance notes

  • Benchmarks show no meaningful regression on the common suppressed-call path (~+0.4%).
  • Module-override lookups are faster (≈−7% on suppressed paths, ≈−4% on live paths) due to concurrent registry lookups.
  • Registry operations are lightweight (getById ≈40 ns, getIds ≈216 ns); child loggers incur additional cost when merging key-values.

Outcomes

  • Enables dynamic adjustment of global and module-specific log levels at runtime via public APIs and native hooks.
  • Preserves common-call performance while improving module-override lookup throughput.
  • Provides a test-covered, observable API surface and integration points for runtime components to manage logger configuration.

daneshk and others added 22 commits February 3, 2026 22:34
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>
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 17, 2026

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Adds 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

Cohort / File(s) Summary
Version & Dependency Updates
ballerina/Ballerina.toml, ballerina/CompilerPlugin.toml, ballerina/Dependencies.toml, gradle.properties
Bump versions to 2.17.0 / 2.17.0-SNAPSHOT and update compiler/plugin JAR paths and distribution-version.
Core Logger API & Natives
ballerina/logger.bal, ballerina/natives.bal
Add public getLevel()/setLevel() to Logger; replace per-module comparison with centralized LOG_LEVEL_WEIGHT and isLevelEnabled; add native bindings for ID generation and module-level get/set.
Root Logger, Registry & Print Flow
ballerina/root_logger.bal, ballerina/init.bal
Introduce LoggerRegistry singleton, logger IDs, RootLogger changes (currentLevel/loggerId/isModuleLogger), ChildLogger class, unified printLog, rotation and file-write helpers, and registration of root/module loggers at init.
Runtime Tests
ballerina/tests/log_config_test.bal, ballerina/tests/log_rotation_test.bal
Add extensive runtime config tests covering registry, ID generation, inheritance and level set/get; small unused-value discard change in rotation test.
Integration Tests & Samples
integration-tests/.../logger-registry/*, integration-tests/tests/tests_logger.bal, integration-tests/build.gradle, integration-tests/Ballerina.toml
Add logger-registry sample, integration test, copy resources, and EOF newline fix in manifest.
Custom Logger Sample
integration-tests/tests/resources/samples/logger/custom-logger/main.bal
Add getLevel() and setLevel() to CustomLogger sample for API parity.
Native Java Support
native/src/main/java/io/ballerina/stdlib/log/LogConfigManager.java
Add singleton LogConfigManager with StackWalker-based logger ID generation, per-function counters, and module-level get/set APIs exposed to Ballerina.
Build Config / SpotBugs
build-config/spotbugs-exclude.xml
Add SpotBugs exclusion for MS_EXPOSE_REP on LogConfigManager.getInstance() singleton method.
Docs & Changelog
docs/spec/spec.md, changelog.md
Document getLevel()/setLevel(), Config.id? and masking option; add changelog entry for runtime log-level modification.

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
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~65 minutes

Possibly related issues

Poem

🐇 Soft paws tap the registry log,
IDs sprout where callers jog,
Levels bend without a restart,
Roots and children play their part,
The warren hums — we rabbit-hop the chart.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title 'Add dynamic log level change support' clearly and concisely summarizes the main objective of the PR: enabling runtime modification of log levels without application restart.
Description check ✅ Passed The PR description includes the required sections from the template: Purpose (with linked issue), Examples (blank but acceptable), and a completed Checklist confirming all required items were addressed.
Linked Issues check ✅ Passed The PR successfully implements the requirements from issue #6213: provides Java API for runtime log level modification, supports global/module-level changes, exposes queryable API via logger registry, and eliminates Config.toml restart requirement.
Out of Scope Changes check ✅ Passed All changes are directly aligned with adding runtime log level modification: new Logger methods (getLevel/setLevel), registry infrastructure, native Java support, comprehensive tests, documentation updates, and version bumps are all within scope.
Docstring Coverage ✅ Passed Docstring coverage is 83.33% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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

@codecov
Copy link

codecov bot commented Feb 17, 2026

Codecov Report

❌ Patch coverage is 82.50000% with 21 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.66%. Comparing base (f8beeaa) to head (892dac1).
⚠️ Report is 44 commits behind head on master.

Files with missing lines Patch % Lines
ballerina/root_logger.bal 81.81% 12 Missing ⚠️
...java/io/ballerina/stdlib/log/LogConfigManager.java 78.04% 2 Missing and 7 partials ⚠️
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.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@daneshk daneshk marked this pull request as ready for review February 19, 2026 00:56
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 4

🧹 Nitpick comments (3)
ballerina/root_logger.bal (1)

195-210: Registry lookup on every log call may impact performance.

The print method acquires a lock and performs a registry lookup on every log call when a module name is present. For high-throughput logging, this could become a bottleneck.

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 logLevelWeight map 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 testAutoIdFirstNoSuffix test iterates through registry IDs but only breaks 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.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

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.

@daneshk
Copy link
Member Author

daneshk commented Feb 20, 2026

Performance Comparison: ballerina/log 2.16.1 vs 2.17.0

Overview

This document compares the performance of the released ballerina/log:2.16.1 against the upcoming
2.17.0 which introduces runtime log level modification via a logger registry.

Benchmark setup:

  • 2,000,000 iterations × 5 runs per scenario (avg ns/call reported)
  • 200,000 iteration warmup before each scenario
  • Live log output redirected to file to exclude I/O variance from suppressed-call benchmarks
  • Machine: macOS Darwin 25.2.0

Scenario 1 — No Module Override, Suppressed Call

Root level = INFO. printDebug is suppressed. Exercises the level-check hot path with no
module-level registry hit.

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. printInfo is 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.toml sets the benchmark module level to WARN. printInfo and printDebug are
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.toml sets the benchmark module level to WARN. printWarn is 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(), and withContext child 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 ⚠️ 2× root (key-value merge overhead)

Key Findings

  1. 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.

  2. 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.

  3. Registry lookups are cheap. getById costs only 40 ns and getIds 216 ns, making
    runtime logger discovery negligible.

  4. fromConfig loggers perform on par with the root logger on both the suppressed and live
    paths.

  5. withContext child 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.

  6. 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.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

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>
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

🧹 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 moduleName or level is null, the call throws an undocumented NullPointerException. 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.

@sonarqubecloud
Copy link

@daneshk daneshk merged commit 53a45a4 into ballerina-platform:master Feb 20, 2026
9 checks passed
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.

Add native support for modifying the log level programmatically at runtime

3 participants