gnodet commented on code in PR #26055:
URL: https://github.com/apache/camel/pull/26055#discussion_r3931895150


##########
dsl/camel-kamelet-main/src/main/resources/camel-main-known-dependencies.properties:
##########
@@ -73,3 +73,17 @@ org.apache.qpid.jms.JmsConnectionFactory = 
org.apache.qpid:qpid-jms-client:${qpi
 org.messaginghub.pooled.jms.JmsPoolConnectionFactory = 
org.messaginghub:pooled-jms:${pooled-jms-version}
 org.postgresql.Driver = org.postgresql:postgresql:${pgjdbc-driver-version}
 org.postgresql.ds.PGSimpleDataSource = 
org.postgresql:postgresql:${pgjdbc-driver-version}
+
+dev.langchain4j.model.ollama = 
dev.langchain4j:langchain4j-ollama:${langchain4j-version}
+dev.langchain4j.model.openai = 
dev.langchain4j:langchain4j-open-ai:${langchain4j-version}
+dev.langchain4j.model.huggingface = 
dev.langchain4j:langchain4j-hugging-face:${langchain4j-beta-version}
+dev.langchain4j.model.anthropic = 
dev.langchain4j:langchain4j-anthropic:${langchain4j-version}
+dev.langchain4j.model.azure = 
dev.langchain4j:langchain4j-azure-open-ai:${langchain4j-version}
+dev.langchain4j.model.mistralai = 
dev.langchain4j:langchain4j-mistral-ai:${langchain4j-version}
+dev.langchain4j.model.vertexai = 
dev.langchain4j:langchain4j-vertex-ai:${langchain4j-version}
+dev.langchain4j.model.googleai = 
dev.langchain4j:langchain4j-google-ai-gemini:${langchain4j-version}
+dev.langchain4j.model.github = 
dev.langchain4j:langchain4j-github-models:${langchain4j-version}
+dev.langchain4j.model.embedding.onnx = 
dev.langchain4j:langchain4j-embeddings:${langchain4j-beta-version}

Review Comment:
   The last two entries use a different convention from the rest of the file:
   
   ```
   org.apache.camel.component.ai.observability.GenAiObservabilityImpl = 
camel:ai-observability
   camel.aiObservability = camel:ai-observability
   ```
   
   Existing entries in this file are keyed by either fully-qualified class 
names or property keys with `=` escaping (e.g., 
`org.apache.camel.component.activemq.ActiveMQComponent\:embedded\=true`). The 
`camel.aiObservability` entry is a camel-main property prefix, not a class name 
— `KnownDependenciesResolver.findGav()` does prefix-trimming on `.` separators, 
so this entry would match any property or class starting with 
`camel.aiObservability`. Is this intentional? It means any 
`camel.aiObservability.*` property access through the properties component 
triggers a download of `camel:ai-observability`, which seems like the desired 
behavior but is worth calling out since it's a novel use of the known-deps file.



##########
dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/common/GenAiDependencyHelper.java:
##########
@@ -0,0 +1,131 @@
+/*
+ * 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.camel.dsl.jbang.core.common;
+
+import java.util.Collection;
+import java.util.Properties;
+
+import org.apache.camel.catalog.CamelCatalog;
+import org.apache.camel.catalog.DefaultCamelCatalog;
+import org.apache.camel.tooling.model.ComponentModel;
+
+/**
+ * Adds optional GenAI observability dependencies using the same 
settings-driven approach as OpenTelemetry and LRA.
+ * <p>
+ * GenAI component and LangChain4j provider JARs are resolved by the existing 
silent-run download pipeline
+ * ({@code DependencyDownloaderComponentResolver}, {@code 
KnownDependenciesResolver}) — not by scanning route source.
+ * </p>
+ */
+public final class GenAiDependencyHelper {
+
+    public static final String AI_OBSERVABILITY_ENABLED = 
"camel.aiObservability.enabled";
+
+    private GenAiDependencyHelper() {
+    }
+
+    /**
+     * Adds {@code camel:ai-observability} when GenAI artifacts are already in 
the dependency set and observability is
+     * requested via {@code --observe} or {@code 
camel.aiObservability.enabled=true}.
+     */
+    public static void addAiObservabilityIfNeeded(Collection<String> deps, 
Properties properties, boolean observe) {
+        addAiObservabilityIfNeeded(deps, properties, observe, new 
DefaultCamelCatalog());
+    }
+
+    static void addAiObservabilityIfNeeded(
+            Collection<String> deps, Properties properties, boolean observe, 
CamelCatalog catalog) {
+        if (!includeAiObservability(properties, observe)) {
+            return;
+        }
+        if (!hasGenAiDependency(deps, catalog)) {
+            return;
+        }
+        if (catalog.otherModel("ai-observability") != null) {
+            deps.add("camel:ai-observability");
+        }
+    }
+
+    static boolean includeAiObservability(Properties properties, boolean 
observe) {
+        String enabled = properties != null ? 
properties.getProperty(AI_OBSERVABILITY_ENABLED) : null;
+        if ("false".equalsIgnoreCase(enabled)) {
+            return false;
+        }
+        return observe || "true".equalsIgnoreCase(enabled);
+    }
+
+    static boolean hasGenAiDependency(Collection<String> deps, CamelCatalog 
catalog) {
+        for (String dep : deps) {
+            if (dep == null || dep.isBlank()) {
+                continue;
+            }
+            if (isGenAiCamelComponent(dep, catalog)) {
+                return true;
+            }
+            if (isLangChain4jProviderJar(dep)) {
+                return true;
+            }
+        }
+        return false;
+    }
+
+    private static boolean isGenAiCamelComponent(String dep, CamelCatalog 
catalog) {
+        if (dep.startsWith("camel:")) {
+            String scheme = dep.substring("camel:".length());
+            int query = scheme.indexOf('?');
+            if (query > 0) {
+                scheme = scheme.substring(0, query);
+            }
+            ComponentModel model = catalog.componentModel(scheme);
+            return model != null && isAiLabel(model.getLabel());
+        }
+        return dep.contains(":camel-") && isAiArtifactId(dep);
+    }
+
+    private static boolean isAiArtifactId(String dep) {
+        int idx = dep.indexOf(":camel-");
+        if (idx < 0) {
+            return false;
+        }
+        String artifact = dep.substring(idx + 1);
+        int colon = artifact.indexOf(':');
+        if (colon > 0) {
+            artifact = artifact.substring(0, colon);
+        }
+        return artifact.startsWith("camel-langchain4j")
+                || artifact.startsWith("camel-openai")
+                || artifact.startsWith("camel-spring-ai")
+                || artifact.startsWith("camel-aws-bedrock")

Review Comment:
   The `contains("-ai-")` check is broad. Today's component names are safe, but 
future non-AI components with `-ai-` in their artifact ID (e.g., a hypothetical 
`camel-repair-aid-connector`) would be false-positived here. The catalog label 
check (`isAiLabel`) is the authoritative source of truth — could the `mvn:` 
branch also query the catalog by artifact ID instead of relying on string 
heuristics?



##########
dsl/camel-jbang/camel-jbang-core/src/test/java/org/apache/camel/dsl/jbang/core/common/GenAiDependencyHelperTest.java:
##########
@@ -0,0 +1,98 @@
+/*
+ * 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.camel.dsl.jbang.core.common;
+
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.List;
+import java.util.Properties;
+
+import org.apache.camel.catalog.CamelCatalog;
+import org.apache.camel.catalog.DefaultCamelCatalog;
+import org.apache.camel.tooling.model.ComponentModel;
+import org.junit.jupiter.api.Test;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+class GenAiDependencyHelperTest {
+
+    private final CamelCatalog catalog = new DefaultCamelCatalog();
+
+    @Test
+    void addsAiObservabilityWhenGenAiComponentPresentAndObserveEnabled() {
+        List<String> deps = new ArrayList<>(List.of("camel:langchain4j-chat"));
+
+        GenAiDependencyHelper.addAiObservabilityIfNeeded(deps, new 
Properties(), true, catalog);
+
+        assertThat(deps).contains("camel:ai-observability");
+    }
+
+    @Test
+    void addsAiObservabilityWhenGenAiPropertyEnabled() {
+        List<String> deps = new 
ArrayList<>(List.of("mvn:org.apache.camel:camel-openai"));
+        Properties properties = new Properties();
+        properties.setProperty(GenAiDependencyHelper.AI_OBSERVABILITY_ENABLED, 
"true");
+
+        GenAiDependencyHelper.addAiObservabilityIfNeeded(deps, properties, 
false, catalog);
+
+        assertThat(deps).contains("camel:ai-observability");
+    }
+
+    @Test
+    void skipsAiObservabilityWithoutGenAiArtifacts() {
+        List<String> deps = new ArrayList<>(List.of("camel:timer"));
+
+        GenAiDependencyHelper.addAiObservabilityIfNeeded(deps, new 
Properties(), true, catalog);
+
+        assertThat(deps).doesNotContain("camel:ai-observability");
+    }
+
+    @Test
+    void skipsAiObservabilityWhenExplicitlyDisabled() {
+        List<String> deps = new ArrayList<>(List.of("camel:langchain4j-chat"));
+        Properties properties = new Properties();
+        properties.setProperty(GenAiDependencyHelper.AI_OBSERVABILITY_ENABLED, 
"false");
+
+        GenAiDependencyHelper.addAiObservabilityIfNeeded(deps, properties, 
true, catalog);
+
+        assertThat(deps).doesNotContain("camel:ai-observability");
+    }

Review Comment:
   This test mocks the catalog to return a `ComponentModel` with label `"ai"` 
for `"openai"` — but the real catalog already has `openai` with label `"ai"` 
(verified in `components/camel-ai/camel-openai/src/generated/resources/`). 
Using the real `DefaultCamelCatalog` here (like the other tests) would be more 
resilient to future catalog changes and wouldn't require Mockito.



##########
dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/common/GenAiDependencyHelper.java:
##########
@@ -0,0 +1,131 @@
+/*
+ * 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.camel.dsl.jbang.core.common;
+
+import java.util.Collection;
+import java.util.Properties;
+
+import org.apache.camel.catalog.CamelCatalog;
+import org.apache.camel.catalog.DefaultCamelCatalog;
+import org.apache.camel.tooling.model.ComponentModel;
+
+/**
+ * Adds optional GenAI observability dependencies using the same 
settings-driven approach as OpenTelemetry and LRA.
+ * <p>
+ * GenAI component and LangChain4j provider JARs are resolved by the existing 
silent-run download pipeline
+ * ({@code DependencyDownloaderComponentResolver}, {@code 
KnownDependenciesResolver}) — not by scanning route source.
+ * </p>
+ */
+public final class GenAiDependencyHelper {
+

Review Comment:
   Nit: the constant `AI_OBSERVABILITY_ENABLED` is `public` but is only used 
within this class and in tests (which access package-private methods anyway). 
Consider narrowing to package-private if there's no external consumer planned.



##########
dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/common/GenAiDependencyHelper.java:
##########
@@ -0,0 +1,131 @@
+/*
+ * 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.camel.dsl.jbang.core.common;
+
+import java.util.Collection;
+import java.util.Properties;
+
+import org.apache.camel.catalog.CamelCatalog;
+import org.apache.camel.catalog.DefaultCamelCatalog;
+import org.apache.camel.tooling.model.ComponentModel;
+
+/**
+ * Adds optional GenAI observability dependencies using the same 
settings-driven approach as OpenTelemetry and LRA.
+ * <p>
+ * GenAI component and LangChain4j provider JARs are resolved by the existing 
silent-run download pipeline
+ * ({@code DependencyDownloaderComponentResolver}, {@code 
KnownDependenciesResolver}) — not by scanning route source.
+ * </p>
+ */
+public final class GenAiDependencyHelper {
+
+    public static final String AI_OBSERVABILITY_ENABLED = 
"camel.aiObservability.enabled";
+
+    private GenAiDependencyHelper() {
+    }
+
+    /**
+     * Adds {@code camel:ai-observability} when GenAI artifacts are already in 
the dependency set and observability is
+     * requested via {@code --observe} or {@code 
camel.aiObservability.enabled=true}.
+     */
+    public static void addAiObservabilityIfNeeded(Collection<String> deps, 
Properties properties, boolean observe) {
+        addAiObservabilityIfNeeded(deps, properties, observe, new 
DefaultCamelCatalog());
+    }
+
+    static void addAiObservabilityIfNeeded(
+            Collection<String> deps, Properties properties, boolean observe, 
CamelCatalog catalog) {
+        if (!includeAiObservability(properties, observe)) {
+            return;
+        }
+        if (!hasGenAiDependency(deps, catalog)) {
+            return;
+        }
+        if (catalog.otherModel("ai-observability") != null) {
+            deps.add("camel:ai-observability");
+        }
+    }
+
+    static boolean includeAiObservability(Properties properties, boolean 
observe) {
+        String enabled = properties != null ? 
properties.getProperty(AI_OBSERVABILITY_ENABLED) : null;
+        if ("false".equalsIgnoreCase(enabled)) {
+            return false;
+        }
+        return observe || "true".equalsIgnoreCase(enabled);
+    }
+
+    static boolean hasGenAiDependency(Collection<String> deps, CamelCatalog 
catalog) {
+        for (String dep : deps) {
+            if (dep == null || dep.isBlank()) {
+                continue;
+            }
+            if (isGenAiCamelComponent(dep, catalog)) {
+                return true;
+            }
+            if (isLangChain4jProviderJar(dep)) {
+                return true;
+            }
+        }
+        return false;
+    }
+
+    private static boolean isGenAiCamelComponent(String dep, CamelCatalog 
catalog) {
+        if (dep.startsWith("camel:")) {
+            String scheme = dep.substring("camel:".length());
+            int query = scheme.indexOf('?');
+            if (query > 0) {
+                scheme = scheme.substring(0, query);
+            }
+            ComponentModel model = catalog.componentModel(scheme);
+            return model != null && isAiLabel(model.getLabel());
+        }
+        return dep.contains(":camel-") && isAiArtifactId(dep);
+    }
+
+    private static boolean isAiArtifactId(String dep) {
+        int idx = dep.indexOf(":camel-");
+        if (idx < 0) {
+            return false;

Review Comment:
   `isAiArtifactId` uses `artifact.startsWith("camel-ai-")` which matches 
`camel-ai-observability` itself. If a user adds 
`mvn:org.apache.camel:camel-ai-observability:...` as an explicit `--dep`, 
`hasGenAiDependency` returns true, and the method would add 
`camel:ai-observability` again as a duplicate.
   
   In `ExportBaseCommand` this is harmless (TreeSet dedup), but in `Run.java` 
`dependencies` is an `ArrayList`, so it results in a duplicate entry. Not a 
crash-level issue — the downstream dependency resolution likely handles it — 
but it's an unintended self-reference.
   
   Consider excluding `camel-ai-observability` explicitly, or using the catalog 
label check as the primary filter for `mvn:` deps too.



-- 
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]

Reply via email to