You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by "ysymi (via GitHub)" <gi...@apache.org> on 2026/09/09 12:31:25 UTC

[PR] [dist] Shade and relocate kafka-clients and jackson in flink-agents-dist [flink-agents]

ysymi opened a new pull request, #1110:
URL: https://github.com/apache/flink-agents/pull/1110

   Linked issue: #652
   
      ### Purpose of change
   
      `flink-agents-dist-*.jar` is a fat JAR that bundles third-party dependencies
      without relocation. When users add it via `--jars` or place it in Flink's
      `lib/`, its dependencies share the ClassLoader with the user's job JAR, and
      version mismatches surface as `NoSuchMethodError` / `ClassCastException` at
      runtime.
   
      This PR adds shade relocation to `dist/pom.xml`. Since `dist` is the parent
      pom (packaging=pom), a single change applies to all five Flink-version
      submodules (flink-1.20 / 2.0 / 2.1 / 2.2 / 2.3).
   
      What is relocated (the two entries rated High/Medium in the issue's risk
      table, present in almost every streaming job):
   
      - `org.apache.kafka` → `org.apache.flink.agents.shaded.org.apache.kafka`
      - `com.fasterxml` → `org.apache.flink.agents.shaded.com.fasterxml`
   
      What is intentionally **not** relocated, and why:
   
      - **kotlin-stdlib / okhttp3 / okio**: the shade plugin's resource-name string
        replacement corrupts Kotlin's `.kotlin_builtins` metadata files (verified in
        our internal production build). okhttp3/okio are Kotlin-based and must move
        together with kotlin-stdlib, so they stay unshaded as a group.
      - **vendor SDKs (openai-java, anthropic-java)**: rated Low/Medium-low in the
        issue itself; relocating them risks breaking SDK-internal hardcoded class
        names for little benefit. Can be follow-ups if real conflict reports appear.
      - Packages Flink already shades in `flink-shaded-*` (e.g. guava, netty) are
        not repeated.
   
      Necessary companion changes:
   
      - `ServicesResourceTransformer` merges `META-INF/services` entries across
        bundled JARs so relocated SPI implementations stay discoverable.
      - Third-party `META-INF/versions/21/` classes are dropped — the shade plugin
        cannot reliably process them and the base versions are used instead. Our own
        `versions/21/org/apache/flink/**` entries (JDK 21 Continuation support) are
        preserved.
   
      ### Tests
   
      Verified by building `dist/flink-2.3`:
   
      - Relocated packages contain no un-relocated residue (0 classes under the
        original `org/apache/kafka/` and `com/fasterxml/` paths).
      - Bytecode spot-check (`javap -c`): runtime/plan classes reference only the
        shaded paths; remaining mentions are log-message string constants.
      - runtime unit tests pass unchanged.
   
      Note: this is a pom-only change; community CI (`./tools/ut.sh`,
      `./tools/lint.sh`) covers the full build across versions.
   
      ### API
   
      No public API changes.
   
      ### Documentation
   
      - [ ] `doc-needed`
      - [x] `doc-not-needed`
      - [ ] `doc-included`
   
      ### Was this patch authored or co-authored using generative AI tooling?
   
      - [x] Yes
      - [ ] No
   
      Generated-by: Pi coding agent 0.85.1 (glm-5.3)
   


-- 
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: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [dist] Shade and relocate kafka-clients and jackson in flink-agents-dist [flink-agents]

Posted by "wenjin272 (via GitHub)" <gi...@apache.org>.
wenjin272 commented on code in PR #1110:
URL: https://github.com/apache/flink-agents/pull/1110#discussion_r4003178842


##########
dist/pom.xml:
##########
@@ -160,9 +160,45 @@ under the License.
                                 <exclude>META-INF/org/apache/logging/log4j/**</exclude>
                                 <exclude>META-INF/services/org.apache.logging.log4j.*</exclude>
                                 <exclude>META-INF/licenses/LICENSE.slf4j*</exclude>
+                                <!-- The shade plugin relocates bytecode but not file
+                                     paths under META-INF/versions/**: relocated MR entries
+                                     keep the original package path while their bytecode
+                                     declares the shaded name, which breaks the JDK 9+ MR
+                                     lookup of the original class with NoClassDefFoundError
+                                     (wrong name). The base classes are kept, so dropping the
+                                     MR copies is lossless. -->
+                                <exclude>META-INF/versions/*/com/fasterxml/**</exclude>
                             </excludes>
                         </filter>
                     </filters>
+                    <!--
+                        Relocate third-party libraries that are commonly present in user job JARs
+                        or other JARs on the Flink classpath. When flink-agents-dist is submitted
+                        alongside user code (e.g. via the "jars" option or placed in lib/) it lands
+                        in the same ClassLoader as user code, so a version mismatch on these
+                        packages causes NoSuchMethodError or ClassCastException.
+
+                        Packages that Flink already relocates in flink-shaded-* (jackson, guava,
+                        netty, asm) are NOT repeated here: Flink's copies live under
+                        org.apache.flink.shaded.* and therefore never conflict with ours.
+
+                        kotlin / okhttp3 / okio are intentionally NOT relocated: the shade plugin
+                        applies string replacement to all resource file names, corrupting
+                        Kotlin's .kotlin_builtins metadata files. okhttp3/okio are Kotlin-based
+                        and must move together with kotlin stdlib, so all three are left unshaded.
+                    -->
+                    <relocations>
+                        <!-- kafka-clients is ubiquitous in Flink streaming jobs -->
+                        <relocation>
+                            <pattern>org.apache.kafka</pattern>
+                            <shadedPattern>org.apache.flink.agents.shaded.org.apache.kafka</shadedPattern>
+                        </relocation>
+                        <!-- jackson is widely used for JSON serialisation in user code -->
+                        <relocation>
+                            <pattern>com.fasterxml</pattern>
+                            <shadedPattern>org.apache.flink.agents.shaded.com.fasterxml</shadedPattern>

Review Comment:
   Could we avoid relocating Jackson until the serialization boundary is designed end to end? Flink can use `flink-shaded-jackson` because its Jackson DTOs and mapper are framework-owned and compiled against the same shaded namespace; its shaded mapper does not need to interpret `com.fasterxml.jackson.*` annotations on user-defined classes.
   
   Flink Agents is different: custom `Event` subclasses are a public extension point, the documentation requires users to annotate their constructors with the standard `@JsonCreator`/`@JsonProperty`, and `ActionStateSerde` uses Jackson polymorphic deserialization to restore those user classes. The user job JAR is not processed by this shade execution, so the relocated mapper will not recognize those annotations.
   
   This is therefore more than a shade configuration change. We first need a complete serialization design—for example, persisting a framework-owned Event envelope and reconstructing user types through an explicit codec/registration contract—and packaged tests covering custom-event recovery. As implemented, relocating all of `com.fasterxml` changes the documented serialization contract despite the PR declaring no public API change.



##########
dist/pom.xml:
##########
@@ -160,9 +160,45 @@ under the License.
                                 <exclude>META-INF/org/apache/logging/log4j/**</exclude>
                                 <exclude>META-INF/services/org.apache.logging.log4j.*</exclude>
                                 <exclude>META-INF/licenses/LICENSE.slf4j*</exclude>
+                                <!-- The shade plugin relocates bytecode but not file
+                                     paths under META-INF/versions/**: relocated MR entries
+                                     keep the original package path while their bytecode
+                                     declares the shaded name, which breaks the JDK 9+ MR
+                                     lookup of the original class with NoClassDefFoundError
+                                     (wrong name). The base classes are kept, so dropping the
+                                     MR copies is lossless. -->
+                                <exclude>META-INF/versions/*/com/fasterxml/**</exclude>
                             </excludes>
                         </filter>
                     </filters>
+                    <!--
+                        Relocate third-party libraries that are commonly present in user job JARs
+                        or other JARs on the Flink classpath. When flink-agents-dist is submitted
+                        alongside user code (e.g. via the "jars" option or placed in lib/) it lands
+                        in the same ClassLoader as user code, so a version mismatch on these
+                        packages causes NoSuchMethodError or ClassCastException.
+
+                        Packages that Flink already relocates in flink-shaded-* (jackson, guava,
+                        netty, asm) are NOT repeated here: Flink's copies live under
+                        org.apache.flink.shaded.* and therefore never conflict with ours.
+
+                        kotlin / okhttp3 / okio are intentionally NOT relocated: the shade plugin
+                        applies string replacement to all resource file names, corrupting
+                        Kotlin's .kotlin_builtins metadata files. okhttp3/okio are Kotlin-based
+                        and must move together with kotlin stdlib, so all three are left unshaded.
+                    -->
+                    <relocations>
+                        <!-- kafka-clients is ubiquitous in Flink streaming jobs -->
+                        <relocation>
+                            <pattern>org.apache.kafka</pattern>
+                            <shadedPattern>org.apache.flink.agents.shaded.org.apache.kafka</shadedPattern>

Review Comment:
   Please also adjust the thin-JAR filter. Relocated classes are placed under `org/apache/flink/agents/shaded/**`, which matches the existing `org/apache/flink/agents/**` include and causes Kafka/Jackson classes to be copied into every `-thin.jar`. Excluding the shaded subtree from that execution should keep the thin JARs thin; please verify that they contain no relocated dependency classes.



-- 
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: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org