Yicong-Huang commented on code in PR #5375:
URL: https://github.com/apache/texera/pull/5375#discussion_r4323376517
##########
amber/src/main/scala/org/apache/texera/web/TexeraWebApplication.scala:
##########
@@ -164,6 +170,17 @@ class TexeraWebApplication
AuthResource.createAdminUser()
+ // Tag every log record emitted during request handling with the
+ // workflow / execution / computing-unit id when one is visible in
+ // the URL or headers. This lets downstream log tooling filter
+ // records by workflow or computing unit (the JVM otherwise emits
+ // them without any per-request context).
+ environment.getApplicationContext.addFilter(
+ new FilterHolder(new
org.apache.texera.web.observability.RequestContextMdcFilter()),
Review Comment:
**Must fix:**
These two filters are registered only in `TexeraWebApplication`, yet two of
the three URL patterns target routes served by other Dropwizard apps in
separate JVMs. `/execution/{wid}/{cuid}/run` and `/wsapi/workflow-websocket`
are served by `ComputingUnitMaster` (`SyncExecutionResource` is registered at
`ComputingUnitMaster.scala:205`; the websocket at `:132`), which has no such
filter. `/api/computing-unit/{cuid}/...` is served by
`computing-unit-managing-service`, which does not depend on amber. On this app
only `/workflow/<digits>` extracts anything, so the sync-execution and
websocket-session logs carry no MDC context. Register both filters in
`ComputingUnitMaster.run()` too (same module, no dependency change). Once
registered there, note that `ExecutionPattern` as written would file the wid of
`/execution/{wid}/{cuid}/run` under `texera.execution.id` (that route carries
no eid). Shape it to extract wid and cuid for that URL instead.
##########
amber/src/main/scala/org/apache/texera/web/observability/UserContextMdcFilter.scala:
##########
@@ -0,0 +1,84 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.texera.web.observability
+
+import org.apache.texera.auth.SessionUser
+import org.slf4j.MDC
+
+import javax.annotation.Priority
+import javax.ws.rs.container.{
+ ContainerRequestContext,
+ ContainerRequestFilter,
+ ContainerResponseContext,
+ ContainerResponseFilter
+}
+import javax.ws.rs.ext.Provider
+import javax.ws.rs.Priorities
+
+/**
+ * Jersey filter that pushes the authenticated user's id into the
+ * SLF4J MDC so every log record emitted while handling the request
+ * carries `texera.user.id`.
+ *
+ * Why Jersey and not a Servlet filter:
+ * - Authentication is wired as a Jersey `ContainerRequestFilter`
+ * (see [[org.apache.texera.auth.JwtAuthFilter]]): by the time
+ * Servlet filters run, `SecurityContext.getUserPrincipal` is
+ * still null. A Jersey filter ordered after auth picks it up.
+ * - Priorities.AUTHENTICATION + 100 places us in the
+ * AUTHORIZATION bucket which is guaranteed to run AFTER auth.
+ *
+ * Symmetric request/response interfaces let us pair set + clear
+ * without leaking MDC across threads in the worker pool.
+ */
+@Provider
+@Priority(Priorities.AUTHORIZATION)
+class UserContextMdcFilter extends ContainerRequestFilter with
ContainerResponseFilter {
+
+ override def filter(requestContext: ContainerRequestContext): Unit = {
+ val secCtx = requestContext.getSecurityContext
+ if (secCtx != null) {
+ secCtx.getUserPrincipal match {
+ case user: SessionUser =>
+ val uid = user.getUid
+ if (uid != null) MDC.put(UserContextMdcFilter.UserIdKey,
uid.toString)
+ case _ => // anonymous request, or auth chose a different Principal
type
+ }
+ }
+ }
+
+ override def filter(
+ requestContext: ContainerRequestContext,
+ responseContext: ContainerResponseContext
+ ): Unit = {
+ // Defence in depth: clear only the key we own. The Servlet-layer
+ // filter clears its own keys via the same pattern.
+ MDC.remove(UserContextMdcFilter.UserIdKey)
Review Comment:
**Advisory:**
Cleanup lives only in the response filter. If a request takes a path where
Jersey never invokes response filters, `texera.user.id` stays on the pooled
thread and the next request's logs are misattributed to the previous user. An
unmapped exception escaping to the servlet container is such a path, and no
`ExceptionMapper` is registered in the web module. Is there another such path
here, or would you take the one-line hardening of `MDC.remove(UserIdKey)` at
the start of the request filter?
##########
amber/src/main/scala/org/apache/texera/web/TexeraWebApplication.scala:
##########
@@ -141,6 +142,11 @@ class TexeraWebApplication
)
environment.jersey.register(classOf[RolesAllowedDynamicFeature])
+ // Tag every log record emitted while handling a request with the
+ // authenticated user's id (MDC key `texera.user.id`). Registered
+ // here so it runs after the JWT auth filter set up by setupJwtAuth.
+
environment.jersey.register(classOf[org.apache.texera.web.observability.UserContextMdcFilter])
Review Comment:
**Advisory:**
The PR description still documents only the original three pieces (OtelInit,
appender, sanitizer). This round's two MDC filters and their registration here
are never mentioned. A short section on what each filter tags would close the
gap.
##########
amber/src/main/scala/org/apache/texera/web/observability/RequestContextMdcFilter.scala:
##########
@@ -0,0 +1,165 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.texera.web.observability
+
+import org.slf4j.MDC
+
+import javax.servlet._
+import javax.servlet.http.HttpServletRequest
+
+/**
+ * Servlet filter that pushes request-scoped IDs into the SLF4J MDC
+ * so every log record emitted while handling a request carries
+ * `texera.workflow.id`, `texera.execution.id`, and
+ * `texera.computing_unit.id` when those are visible in the URL or
+ * request headers.
+ *
+ * Without this, the OTel log appender bridges every Logback event
+ * with no per-request context, so the dashboard's CU/workflow
+ * filters match nothing live: the only records carrying those keys
+ * are seed data pushed via the VictoriaLogs ingest API.
+ *
+ * Sources of IDs, in priority order:
+ * 1. HTTP headers `X-Texera-Workflow-Id`, `X-Texera-Execution-Id`,
Review Comment:
**Polish:**
"Set explicitly by the Angular client when known" is unverified: nothing in
the repo sends these headers (checked across ts/scala/java/py). Reword to
"reserved for clients" or similar so the next reader does not go looking for
the sender.
##########
amber/src/main/scala/org/apache/texera/web/observability/UserContextMdcFilter.scala:
##########
@@ -0,0 +1,84 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.texera.web.observability
+
+import org.apache.texera.auth.SessionUser
+import org.slf4j.MDC
+
+import javax.annotation.Priority
+import javax.ws.rs.container.{
+ ContainerRequestContext,
+ ContainerRequestFilter,
+ ContainerResponseContext,
+ ContainerResponseFilter
+}
+import javax.ws.rs.ext.Provider
+import javax.ws.rs.Priorities
+
+/**
+ * Jersey filter that pushes the authenticated user's id into the
+ * SLF4J MDC so every log record emitted while handling the request
+ * carries `texera.user.id`.
+ *
+ * Why Jersey and not a Servlet filter:
+ * - Authentication is wired as a Jersey `ContainerRequestFilter`
+ * (see [[org.apache.texera.auth.JwtAuthFilter]]): by the time
Review Comment:
**Polish:**
The `[[org.apache.texera.auth.JwtAuthFilter]]` link points at the
`common/auth` (jakarta) class, which amber never wires. The auth filter in this
runtime is the toastshaman one installed by `JwtAuth.setupJwtAuth`; point the
doc there instead. The comment also says `Priorities.AUTHENTICATION + 100`, but
the code is `@Priority(Priorities.AUTHORIZATION)`.
##########
amber/src/main/scala/org/apache/texera/web/observability/RequestContextMdcFilter.scala:
##########
@@ -0,0 +1,165 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.texera.web.observability
+
+import org.slf4j.MDC
+
+import javax.servlet._
+import javax.servlet.http.HttpServletRequest
+
+/**
+ * Servlet filter that pushes request-scoped IDs into the SLF4J MDC
+ * so every log record emitted while handling a request carries
+ * `texera.workflow.id`, `texera.execution.id`, and
+ * `texera.computing_unit.id` when those are visible in the URL or
+ * request headers.
+ *
+ * Without this, the OTel log appender bridges every Logback event
+ * with no per-request context, so the dashboard's CU/workflow
+ * filters match nothing live: the only records carrying those keys
+ * are seed data pushed via the VictoriaLogs ingest API.
+ *
+ * Sources of IDs, in priority order:
+ * 1. HTTP headers `X-Texera-Workflow-Id`, `X-Texera-Execution-Id`,
+ * `X-Texera-Computing-Unit-Id`: set explicitly by the
+ * Angular client when known.
+ * 2. URL path segments matching `/workflow/<digits>`,
+ * `/execution/<digits>`, `/computing-unit/<digits>` (also the
+ * `/{wid}` / `/{cuid}` patterns used by some resources).
+ *
+ * MDC is ALWAYS cleared in a `finally` block so a thread reused for
+ * a different request doesn't leak the previous request's labels.
+ *
+ * Keys here must stay out of [[LogSanitizer.DeniedMdcKeys]]: the
+ * sanitizer is default-allow, so any key added here is exported unless
+ * it is denied there, in which case the OTel appender drops it.
+ */
+class RequestContextMdcFilter extends Filter {
+
+ override def init(filterConfig: FilterConfig): Unit = ()
+ override def destroy(): Unit = ()
+
+ override def doFilter(
+ request: ServletRequest,
+ response: ServletResponse,
+ chain: FilterChain
+ ): Unit = {
+ val pushed = scala.collection.mutable.ArrayBuffer.empty[String]
+ request match {
+ case http: HttpServletRequest =>
+ // Header overrides come first: an explicit client header is
+ // the most precise signal we have.
+ applyHeader(http, "X-Texera-Workflow-Id", "texera.workflow.id", pushed)
+ applyHeader(http, "X-Texera-Execution-Id", "texera.execution.id",
pushed)
+ applyHeader(http, "X-Texera-Computing-Unit-Id",
"texera.computing_unit.id", pushed)
+
+ // URL extraction: only fill keys that weren't already set
+ // by headers.
+ val path = Option(http.getRequestURI).getOrElse("")
+ applyPath(path, RequestContextMdcFilter.WorkflowPattern,
"texera.workflow.id", pushed)
+ applyPath(path, RequestContextMdcFilter.ExecutionPattern,
"texera.execution.id", pushed)
+ applyPath(path, RequestContextMdcFilter.CuPattern,
"texera.computing_unit.id", pushed)
+
+ // Query-parameter extraction: the WebSocket upgrade URL is
+ // `/wsapi/workflow-websocket?wid=<w>&cuid=<c>` and Texera
+ // also accepts `?cuid=N` on the REST PVE endpoints. Filling
+ // the MDC here means every record emitted during the WS
+ // handshake AND the long-running session that follows
+ // carries the ids the client supplied.
+ applyParam(http, "wid", "texera.workflow.id", pushed)
+ applyParam(http, "eid", "texera.execution.id", pushed)
+ applyParam(http, "cuid", "texera.computing_unit.id", pushed)
+ case _ =>
+ // Non-HTTP request (websocket upgrades fall through here on
Review Comment:
**Polish:**
Websocket upgrade requests are `HttpServletRequest`s, so they do not fall
through to this branch. The parenthetical is inaccurate; consider dropping it.
##########
amber/src/main/scala/org/apache/texera/web/observability/RequestContextMdcFilter.scala:
##########
@@ -0,0 +1,165 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.texera.web.observability
+
+import org.slf4j.MDC
+
+import javax.servlet._
+import javax.servlet.http.HttpServletRequest
+
+/**
+ * Servlet filter that pushes request-scoped IDs into the SLF4J MDC
+ * so every log record emitted while handling a request carries
+ * `texera.workflow.id`, `texera.execution.id`, and
+ * `texera.computing_unit.id` when those are visible in the URL or
+ * request headers.
+ *
+ * Without this, the OTel log appender bridges every Logback event
+ * with no per-request context, so the dashboard's CU/workflow
+ * filters match nothing live: the only records carrying those keys
+ * are seed data pushed via the VictoriaLogs ingest API.
+ *
+ * Sources of IDs, in priority order:
+ * 1. HTTP headers `X-Texera-Workflow-Id`, `X-Texera-Execution-Id`,
+ * `X-Texera-Computing-Unit-Id`: set explicitly by the
+ * Angular client when known.
+ * 2. URL path segments matching `/workflow/<digits>`,
+ * `/execution/<digits>`, `/computing-unit/<digits>` (also the
+ * `/{wid}` / `/{cuid}` patterns used by some resources).
+ *
+ * MDC is ALWAYS cleared in a `finally` block so a thread reused for
+ * a different request doesn't leak the previous request's labels.
+ *
+ * Keys here must stay out of [[LogSanitizer.DeniedMdcKeys]]: the
+ * sanitizer is default-allow, so any key added here is exported unless
+ * it is denied there, in which case the OTel appender drops it.
+ */
+class RequestContextMdcFilter extends Filter {
+
+ override def init(filterConfig: FilterConfig): Unit = ()
+ override def destroy(): Unit = ()
+
+ override def doFilter(
+ request: ServletRequest,
+ response: ServletResponse,
+ chain: FilterChain
+ ): Unit = {
+ val pushed = scala.collection.mutable.ArrayBuffer.empty[String]
+ request match {
+ case http: HttpServletRequest =>
+ // Header overrides come first: an explicit client header is
+ // the most precise signal we have.
+ applyHeader(http, "X-Texera-Workflow-Id", "texera.workflow.id", pushed)
+ applyHeader(http, "X-Texera-Execution-Id", "texera.execution.id",
pushed)
+ applyHeader(http, "X-Texera-Computing-Unit-Id",
"texera.computing_unit.id", pushed)
+
+ // URL extraction: only fill keys that weren't already set
+ // by headers.
+ val path = Option(http.getRequestURI).getOrElse("")
+ applyPath(path, RequestContextMdcFilter.WorkflowPattern,
"texera.workflow.id", pushed)
+ applyPath(path, RequestContextMdcFilter.ExecutionPattern,
"texera.execution.id", pushed)
+ applyPath(path, RequestContextMdcFilter.CuPattern,
"texera.computing_unit.id", pushed)
+
+ // Query-parameter extraction: the WebSocket upgrade URL is
+ // `/wsapi/workflow-websocket?wid=<w>&cuid=<c>` and Texera
+ // also accepts `?cuid=N` on the REST PVE endpoints. Filling
+ // the MDC here means every record emitted during the WS
+ // handshake AND the long-running session that follows
+ // carries the ids the client supplied.
+ applyParam(http, "wid", "texera.workflow.id", pushed)
+ applyParam(http, "eid", "texera.execution.id", pushed)
+ applyParam(http, "cuid", "texera.computing_unit.id", pushed)
+ case _ =>
+ // Non-HTTP request (websocket upgrades fall through here on
+ // some Servlet versions). No MDC context to add.
+ }
+ try {
+ chain.doFilter(request, response)
+ } finally {
+ // Defence in depth: clear only what THIS filter set so we don't
+ // step on MDC populated by other middleware.
+ pushed.foreach(MDC.remove)
+ }
+ }
+
+ private def applyHeader(
+ req: HttpServletRequest,
+ headerName: String,
+ mdcKey: String,
+ pushed: scala.collection.mutable.ArrayBuffer[String]
+ ): Unit = {
+ val raw = Option(req.getHeader(headerName)).map(_.trim).filter(_.nonEmpty)
+ raw.foreach { value =>
+ // Cheap allowlist: only digits get through. Prevents log
+ // forging via a CRLF in the header value.
+ if (value.forall(_.isDigit) && value.length <= 19) {
+ MDC.put(mdcKey, value)
+ pushed += mdcKey
+ }
+ }
+ }
+
+ private def applyPath(
+ path: String,
+ pattern: scala.util.matching.Regex,
+ mdcKey: String,
+ pushed: scala.collection.mutable.ArrayBuffer[String]
+ ): Unit = {
+ if (MDC.get(mdcKey) == null) {
+ pattern.findFirstMatchIn(path).foreach { m =>
+ val v = m.group(1)
+ MDC.put(mdcKey, v)
+ pushed += mdcKey
+ }
+ }
+ }
+
+ /** Read a query parameter, validate it's a positive integer, push
+ * to MDC. Same shape as applyHeader: numeric-only allowlist and a
+ * length cap so a forged value can't inject newlines into the
+ * rendered log line.
+ */
+ private def applyParam(
+ req: HttpServletRequest,
+ paramName: String,
+ mdcKey: String,
+ pushed: scala.collection.mutable.ArrayBuffer[String]
+ ): Unit = {
+ if (MDC.get(mdcKey) == null) {
+ val raw =
Option(req.getParameter(paramName)).map(_.trim).filter(_.nonEmpty)
+ raw.foreach { value =>
+ if (value.forall(_.isDigit) && value.length <= 19) {
+ MDC.put(mdcKey, value)
+ pushed += mdcKey
+ }
+ }
+ }
+ }
+}
+
+object RequestContextMdcFilter {
Review Comment:
**Polish:**
The three key literals are repeated at nine call sites. The sibling
`UserContextMdcFilter` centralizes `UserIdKey` in its companion object; doing
the same here removes a silent-mistag hazard.
##########
amber/src/main/scala/org/apache/texera/web/observability/RequestContextMdcFilter.scala:
##########
@@ -0,0 +1,165 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.texera.web.observability
+
+import org.slf4j.MDC
+
+import javax.servlet._
+import javax.servlet.http.HttpServletRequest
+
+/**
+ * Servlet filter that pushes request-scoped IDs into the SLF4J MDC
+ * so every log record emitted while handling a request carries
+ * `texera.workflow.id`, `texera.execution.id`, and
+ * `texera.computing_unit.id` when those are visible in the URL or
+ * request headers.
+ *
+ * Without this, the OTel log appender bridges every Logback event
+ * with no per-request context, so the dashboard's CU/workflow
+ * filters match nothing live: the only records carrying those keys
+ * are seed data pushed via the VictoriaLogs ingest API.
+ *
+ * Sources of IDs, in priority order:
+ * 1. HTTP headers `X-Texera-Workflow-Id`, `X-Texera-Execution-Id`,
+ * `X-Texera-Computing-Unit-Id`: set explicitly by the
+ * Angular client when known.
+ * 2. URL path segments matching `/workflow/<digits>`,
+ * `/execution/<digits>`, `/computing-unit/<digits>` (also the
+ * `/{wid}` / `/{cuid}` patterns used by some resources).
+ *
+ * MDC is ALWAYS cleared in a `finally` block so a thread reused for
+ * a different request doesn't leak the previous request's labels.
+ *
+ * Keys here must stay out of [[LogSanitizer.DeniedMdcKeys]]: the
+ * sanitizer is default-allow, so any key added here is exported unless
+ * it is denied there, in which case the OTel appender drops it.
+ */
+class RequestContextMdcFilter extends Filter {
+
+ override def init(filterConfig: FilterConfig): Unit = ()
+ override def destroy(): Unit = ()
+
+ override def doFilter(
+ request: ServletRequest,
+ response: ServletResponse,
+ chain: FilterChain
+ ): Unit = {
+ val pushed = scala.collection.mutable.ArrayBuffer.empty[String]
+ request match {
+ case http: HttpServletRequest =>
+ // Header overrides come first: an explicit client header is
+ // the most precise signal we have.
+ applyHeader(http, "X-Texera-Workflow-Id", "texera.workflow.id", pushed)
+ applyHeader(http, "X-Texera-Execution-Id", "texera.execution.id",
pushed)
+ applyHeader(http, "X-Texera-Computing-Unit-Id",
"texera.computing_unit.id", pushed)
+
+ // URL extraction: only fill keys that weren't already set
+ // by headers.
+ val path = Option(http.getRequestURI).getOrElse("")
+ applyPath(path, RequestContextMdcFilter.WorkflowPattern,
"texera.workflow.id", pushed)
+ applyPath(path, RequestContextMdcFilter.ExecutionPattern,
"texera.execution.id", pushed)
+ applyPath(path, RequestContextMdcFilter.CuPattern,
"texera.computing_unit.id", pushed)
+
+ // Query-parameter extraction: the WebSocket upgrade URL is
+ // `/wsapi/workflow-websocket?wid=<w>&cuid=<c>` and Texera
+ // also accepts `?cuid=N` on the REST PVE endpoints. Filling
+ // the MDC here means every record emitted during the WS
+ // handshake AND the long-running session that follows
+ // carries the ids the client supplied.
+ applyParam(http, "wid", "texera.workflow.id", pushed)
+ applyParam(http, "eid", "texera.execution.id", pushed)
+ applyParam(http, "cuid", "texera.computing_unit.id", pushed)
+ case _ =>
+ // Non-HTTP request (websocket upgrades fall through here on
+ // some Servlet versions). No MDC context to add.
+ }
+ try {
+ chain.doFilter(request, response)
+ } finally {
+ // Defence in depth: clear only what THIS filter set so we don't
+ // step on MDC populated by other middleware.
+ pushed.foreach(MDC.remove)
+ }
+ }
+
+ private def applyHeader(
+ req: HttpServletRequest,
+ headerName: String,
+ mdcKey: String,
+ pushed: scala.collection.mutable.ArrayBuffer[String]
+ ): Unit = {
+ val raw = Option(req.getHeader(headerName)).map(_.trim).filter(_.nonEmpty)
+ raw.foreach { value =>
+ // Cheap allowlist: only digits get through. Prevents log
+ // forging via a CRLF in the header value.
+ if (value.forall(_.isDigit) && value.length <= 19) {
+ MDC.put(mdcKey, value)
+ pushed += mdcKey
+ }
+ }
+ }
+
+ private def applyPath(
+ path: String,
+ pattern: scala.util.matching.Regex,
+ mdcKey: String,
+ pushed: scala.collection.mutable.ArrayBuffer[String]
+ ): Unit = {
+ if (MDC.get(mdcKey) == null) {
+ pattern.findFirstMatchIn(path).foreach { m =>
+ val v = m.group(1)
+ MDC.put(mdcKey, v)
+ pushed += mdcKey
+ }
+ }
+ }
+
+ /** Read a query parameter, validate it's a positive integer, push
+ * to MDC. Same shape as applyHeader: numeric-only allowlist and a
+ * length cap so a forged value can't inject newlines into the
+ * rendered log line.
+ */
+ private def applyParam(
+ req: HttpServletRequest,
+ paramName: String,
+ mdcKey: String,
+ pushed: scala.collection.mutable.ArrayBuffer[String]
+ ): Unit = {
+ if (MDC.get(mdcKey) == null) {
+ val raw =
Option(req.getParameter(paramName)).map(_.trim).filter(_.nonEmpty)
+ raw.foreach { value =>
+ if (value.forall(_.isDigit) && value.length <= 19) {
+ MDC.put(mdcKey, value)
+ pushed += mdcKey
+ }
+ }
+ }
+ }
+}
+
+object RequestContextMdcFilter {
Review Comment:
**Advisory:**
`/api/executions/{wid}/...` routes (`WorkflowExecutionsResource`, registered
in this very app) match none of the three patterns, so a wid and eid visible in
the URL are never extracted. The class doc's "`/{wid}` / `/{cuid}` patterns"
parenthetical describes matching that does not exist. Either add the pattern or
correct the doc.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
