fix(logging): migrate logback <if condition> attribute to <condition> element

[chakru-r]

Problem

Setting LOG_AGGREGATOR_ENDPOINT has no effect. The optional Loki log-shipping appender is never created, and nothing is logged to indicate why — logback reports the underlying cause as a deprecation warning rather than an error, so the feature fails silently.

Console logging is not affected: STDOUT is defined unconditionally in all of these files.

Root cause

Logback stopped evaluating the condition attribute on <if> somewhere between 1.5.32 and 1.5.37. In 1.5.38 (the version pinned in build.gradle), the only consumer of IfModel.getCondition() is the deprecation warning:

// IfModelHandler.handle()
String conditionStr = ifModel.getCondition();
emitDeprecationWarningIfNecessary(conditionStr);   // ...and that is all it does
if (micTopObject instanceof BranchState) { ... }   // state comes from a <condition> handler

In the attribute form nothing pushes that BranchState, so IfModel.branchState stays null, and both ThenModelHandler and ElseModelHandler call deepMarkAsSkipped(). Neither branch executes, so the AGGREGATOR appender is never instantiated and never attached to <root>.

Because the compose profiles default the variable to empty (LOG_AGGREGATOR_ENDPOINT: ${LOG_AGGREGATOR_ENDPOINT:-}), no CI job ever takes the branch, which is why this went unnoticed.

Logback's docs describe the attribute as deprecated with removal scheduled for January 2027, so this migration is needed regardless of the current breakage.

Fix

Replace the gate in all six logback.xml files with the supported <condition> element, which precedes <if> rather than nesting inside it, using the built-in Janino-free ExpressionPropertyCondition:

<condition class="ch.qos.logback.core.boolex.ExpressionPropertyCondition">
    <expression>isDefined("LOG_AGGREGATOR_ENDPOINT") &amp;&amp; !propertyEquals("LOG_AGGREGATOR_ENDPOINT", "")</expression>
</condition>
<if>
    <then>
        <appender name="AGGREGATOR" ...>

before (no longer evaluated)	after
!property("LOG_AGGREGATOR_ENDPOINT").isEmpty()	isDefined("LOG_AGGREGATOR_ENDPOINT") && !propertyEquals("LOG_AGGREGATOR_ENDPOINT", "")

property() returns "" for an unset key, so both clauses are required — propertyEquals alone returns false for an unset key, which would invert the meaning of the gate. && is written && because & is reserved in XML.

Verification

Ran each of the six config files on logback 1.5.38 in a standalone harness (real configs, janino 3.0.12 as pinned, the Loki appender on the classpath), emitting one INFO event per configured logger and then inspecting root.iteratorForAppenders():

LOG_AGGREGATOR_ENDPOINT	before	after
unset	no AGGREGATOR ✅	no AGGREGATOR ✅
empty string	no AGGREGATOR ✅	no AGGREGATOR ✅
http://host:port	no AGGREGATOR ❌	AGGREGATOR attached ✅

Console output (event counts and formatting) is unchanged in every case, for all six services — gms, mae-consumer, mce-consumer, datahub-upgrade, and both frontend configs.

Checklist

• The PR conforms to DataHub's Contributing Guideline (particularly PR Title Format)
• Links to related issues (if applicable) — fixes the log-shipping feature added in feat(logging): optional log shipping to a Loki-compatible aggregator for core services #18393 / feat(frontend): optional log shipping to a Loki-compatible aggregator #18421
• Tests for the changes have been added/updated (if applicable) — verified with an out-of-tree harness; no automated guard added, see note below
• Docs related to the changes have been added/updated (if applicable) — none needed; LOG_AGGREGATOR_ENDPOINT behaviour is unchanged from what is already documented, this only makes it work again
• For any breaking change/potential downtime/deprecation/big changes an entry has been made in Updating DataHub — n/a, restores documented behaviour

Note for reviewers

This class of failure is silent by design on logback's side. A cheap guard would be a test that parses each logback.xml and asserts logback's StatusManager reports zero errors and that expected appenders are attached — worth considering separately, since the same footgun applies to any future <if> usage.

🤖 Generated with Claude Code

---

Summary by cubic

Restores Loki log-shipping by migrating Logback conditionals from deprecated <if condition="..."> to a preceding <condition> element. On 1.5.38 the attribute was ignored, so LOG_AGGREGATOR_ENDPOINT had no effect and the AGGREGATOR appender was never created; now the gate evaluates and attaches the appender only when the variable is set.

• Updates all six logback.xml files (frontend conf/run, datahub-upgrade, mae-consumer, mce-consumer, metadata-service).
• Uses ch.qos.logback.core.boolex.ExpressionPropertyCondition with isDefined("LOG_AGGREGATOR_ENDPOINT") && !propertyEquals("LOG_AGGREGATOR_ENDPOINT", ""); <condition> must precede <if>.
• Behavior: console STDOUT unchanged; AGGREGATOR attaches only when LOG_AGGREGATOR_ENDPOINT is non-empty.
• Docs: updates docs/how/updating-datahub.md to warn that 1.5.37+ ignores <if condition="..."> and that <condition> cannot be inside an <appender>, <logger>, or <root>; no action if using bundled configs, migrate custom files accordingly.

^{Written for commit 4c4af78. Summary will update on new commits.}

[cursor]

PR Summary

Overview
Restores optional Loki log shipping (LOG_AGGREGATOR_ENDPOINT) that silently stopped working after Logback 1.5.37+ ignored the deprecated condition attribute on <if>.

All six service logback.xml files now use a preceding <condition> with ExpressionPropertyCondition so the AGGREGATOR appender attaches only when the endpoint is defined and non-empty. Console logging is unchanged.

Upgrade notes document the same syntax change for operators who mount a custom logback.xml.

^{Reviewed by Cursor Bugbot for commit 4c4af78. Bugbot is set up for automated code reviews on this repo. Configure here.}

[alwaysmeticulous]

✅ Meticulous spotted 0 visual differences across 1085 screens tested: view results.

Meticulous evaluated ~10 hours of user flows against your PR.

_{Expected differences? Click here. Last updated for commit 4c4af78 docs(updating): record the logback conditional migration under Next. This comment will update as new commits are pushed.}

[cubic-dev-ai]

1 issue found across 6 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="datahub-upgrade/src/main/resources/logback.xml">

<violation number="1" location="datahub-upgrade/src/main/resources/logback.xml:91">
P1: Custom agent: **Flag Security Vulnerabilities**

When `LOG_AGGREGATOR_ENDPOINT` is set to an `http://` URL, this new condition activates `Loki4jAppender`, which sends logs over plaintext HTTP. Require and validate an `https://` endpoint before attaching the appender.</violation>
</file>

<sub>Reply with feedback, questions, or to request a fix.

Re-trigger cubic</sub>

[cubic-dev-ai]

P1: Custom agent: Flag Security Vulnerabilities

When LOG_AGGREGATOR_ENDPOINT is set to an http:// URL, this new condition activates Loki4jAppender, which sends logs over plaintext HTTP. Require and validate an https:// endpoint before attaching the appender.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At datahub-upgrade/src/main/resources/logback.xml, line 91:

<comment>When `LOG_AGGREGATOR_ENDPOINT` is set to an `http://` URL, this new condition activates `Loki4jAppender`, which sends logs over plaintext HTTP. Require and validate an `https://` endpoint before attaching the appender.</comment>

<file context>
@@ -87,7 +87,10 @@
          push API under that prefix rather than at the bare path. -->
-    <if condition='!property("LOG_AGGREGATOR_ENDPOINT").isEmpty()'>
+    <condition class="ch.qos.logback.core.boolex.ExpressionPropertyCondition">
+        <expression>isDefined("LOG_AGGREGATOR_ENDPOINT") &amp;&amp; !propertyEquals("LOG_AGGREGATOR_ENDPOINT", "")</expression>
+    </condition>
+    <if>
</file context>

[github-actions]

Initial JS bundle size ➡️

	Gzipped
Base (933be3a)	3.09 MB
This PR	3.09 MB
Diff	+0 KB (+0.0%)

Eager entry chunks (dist/assets/index-*.js) only. Codecov reports total bundle including lazy chunks — these intentionally differ.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ All tests successful. No failed tests found.

📢 Thoughts on this report? Let us know!

[cubic-dev-ai]

1 issue found across 1 file (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="docs/how/updating-datahub.md">

<violation number="1" location="docs/how/updating-datahub.md:88">
P3: The two entries describe the same 1.5.37+ behavior differently: line 86 says logback "silently ignores" the `condition` attribute, while line 88 says it "logs a deprecation warning" before skipping both branches. Only one can be right, and readers will rely on it to decide whether to watch the logs. Pick the accurate one and use it in both entries (if a deprecation warning is logged, change line 86's "silently ignores" to say the attribute is no longer honored and the failure is otherwise silent).</violation>
</file>

<sub>Reply with feedback, questions, or to request a fix.

Re-trigger cubic</sub>

[cubic-dev-ai]

P3: The two entries describe the same 1.5.37+ behavior differently: line 86 says logback "silently ignores" the condition attribute, while line 88 says it "logs a deprecation warning" before skipping both branches. Only one can be right, and readers will rely on it to decide whether to watch the logs. Pick the accurate one and use it in both entries (if a deprecation warning is logged, change line 86's "silently ignores" to say the attribute is no longer honored and the failure is otherwise silent).

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At docs/how/updating-datahub.md, line 88:

<comment>The two entries describe the same 1.5.37+ behavior differently: line 86 says logback "silently ignores" the `condition` attribute, while line 88 says it "logs a deprecation warning" before skipping both branches. Only one can be right, and readers will rely on it to decide whether to watch the logs. Pick the accurate one and use it in both entries (if a deprecation warning is logged, change line 86's "silently ignores" to say the attribute is no longer honored and the failure is otherwise silent).</comment>

<file context>
@@ -83,7 +83,21 @@ Requirements:
-- **(Security / Dependencies)** Logback (`logback-classic` / `logback-core`) bumped from 1.5.32 to **1.5.38** for CVE-2026-9828 (`HardenedObjectInputStream` overly broad `java.lang`/`java.util` deserialization allowlist; fixed in 1.5.33+) and CVE-2026-10532 (Proxy class deserialization; fixed in 1.5.38). **Action:** none for operators; rebuild/redeploy picks up the new JARs. DataHub does not expose Logback `SimpleSocketServer` / `SimpleSSLSocketServer` by default.
+- **(Security / Dependencies)** Logback (`logback-classic` / `logback-core`) bumped from 1.5.32 to **1.5.38** for CVE-2026-9828 (`HardenedObjectInputStream` overly broad `java.lang`/`java.util` deserialization allowlist; fixed in 1.5.33+) and CVE-2026-10532 (Proxy class deserialization; fixed in 1.5.38). **Action:** none for operators using the bundled logging config; rebuild/redeploy picks up the new JARs. If you supply your own `logback.xml` that uses `<if condition="...">`, see the #19375 entry below — 1.5.37+ silently ignores that attribute. DataHub does not expose Logback `SimpleSocketServer` / `SimpleSSLSocketServer` by default.
+
+- #19375 **(Operations / logging)** Conditional blocks in `logback.xml` are evaluated again. Logback 1.5.37+ stopped honouring the `condition` attribute on `<if>`: it logs a deprecation warning and then skips both the `<then>` and the `<else>` branch, so the failure is silent. On `master` this left `LOG_AGGREGATOR_ENDPOINT` with no effect, because the optional Loki log-shipping appender was never created. No released version is affected — releases up to and including v1.7.0 ship Logback 1.5.32, which still evaluates the attribute. The bundled logging configs now use the supported `<condition>` element instead. **Action:** none if you use the bundled logging config. If you mount your own `logback.xml` containing `<if condition="...">`, those blocks are being ignored on 1.5.37+ — move the expression into a `<condition>` element placed immediately **before** the `<if>`:
+
+  ```xml
</file context>