Connector and Model Tracing #4004

chriswlai · 2025-07-25T20:50:02Z

Description

Adds tracing to all CRUD operations of the connector component as well as model prediction and execution.

Related Issues

Resolves #3971

Check List

New functionality includes testing.
New functionality has been documented.
API changes companion pull request created.
Commits are signed per the DCO using --signoff.
Public documentation issue/PR created.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

pyek-bot · 2025-08-06T04:13:42Z

common/src/main/resources/index-mappings/ml_agent_trace.json

@@ -0,0 +1,175 @@
+{


we don't need this anymore?

Signed-off-by: chrislai <chrlaii@amazon.com>

pyek-bot

Changes look good, left a few comments

pyek-bot · 2025-08-06T21:34:52Z

...in/src/test/java/org/opensearch/ml/action/connector/UpdateConnectorTransportActionTests.java

+        MLConnectorTracer.resetForTest();
+        MLConnectorTracer.initialize(NoopTracer.INSTANCE, mlFeatureEnabledSetting);
+


is this used?

Yes, it is required to avoid IllegalStateException from MLConnectorTracer.getInstance calls

But in this file I don't see any usage of this tracer?

pyek-bot · 2025-08-06T21:37:14Z

...ithms/src/main/java/org/opensearch/ml/engine/algorithms/agent/tracing/MLConnectorTracer.java

+ * </ul>
+ */
+@Log4j2
+public class MLConnectorTracer extends MLTracer {


is it possible to separate MLConnectorTracer and MLModelTracer?

MLModelTracer added

pyek-bot · 2025-08-06T21:38:53Z

...ithms/src/main/java/org/opensearch/ml/engine/algorithms/agent/tracing/MLConnectorTracer.java

+    public static void handleSpanError(Span span, String errorMessage, Exception e) {
+        log.error(errorMessage, e);
+        span.setError(e);
+        getInstance().endSpan(span);
+    }


nit: i see lot of these methods handleSpanError() and they do the same thing, do u think it's possible to move this to the MLTracer itself?

Unfortunately no, since getInstance only exists in specific classes (MLAgentTracer / MLConnectorTracer) so the method can't be moved to MLTracer

pyek-bot · 2025-08-06T21:43:05Z

plugin/src/main/java/org/opensearch/ml/action/prediction/TransportPredictionTaskAction.java

                                                RestStatus.FORBIDDEN
                                            )
                                        );
+                                    predictSpan.setError(e);


does this need to be set before the listener.onFailure()?

Good catch! addressed

Signed-off-by: chrislai <chrlaii@amazon.com>

pyek-bot · 2025-08-07T19:43:15Z

...ithms/src/main/java/org/opensearch/ml/engine/algorithms/agent/tracing/MLConnectorTracer.java

+     *                                If null, tracing will be disabled.
+     */
+    public static synchronized void initialize(Tracer tracer, MLFeatureEnabledSetting mlFeatureEnabledSetting) {
+        initialize(tracer, mlFeatureEnabledSetting, null);


is this ever null?

pyek-bot

LGTM

chriswlai requested review from b4sjoo, dhrubo-os, mingshl, jngz-es, model-collapse, rbhavna, ylwu-amzn, zane-neo, Zhangxunmt, austintlee, HenryL27 and xinyual as code owners July 25, 2025 20:50

chriswlai had a problem deploying to ml-commons-cicd-env-require-approval July 25, 2025 20:51 — with GitHub Actions Failure

chriswlai force-pushed the feature/conntracer branch from 22b888d to 2ddfc2d Compare July 25, 2025 21:11

chriswlai had a problem deploying to ml-commons-cicd-env-require-approval July 25, 2025 21:13 — with GitHub Actions Failure

chriswlai had a problem deploying to ml-commons-cicd-env-require-approval July 25, 2025 22:12 — with GitHub Actions Failure

chriswlai temporarily deployed to ml-commons-cicd-env-require-approval July 29, 2025 18:37 — with GitHub Actions Inactive

pyek-bot reviewed Aug 6, 2025

View reviewed changes

common/src/main/resources/index-mappings/ml_agent_trace.json Outdated

@@ -0,0 +1,175 @@

{

Copy link

Collaborator

pyek-bot Aug 6, 2025

Choose a reason for hiding this comment

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

we don't need this anymore?

chriswlai had a problem deploying to ml-commons-cicd-env-require-approval August 6, 2025 20:11 — with GitHub Actions Failure

more refactoring

8022fe4

Signed-off-by: chrislai <chrlaii@amazon.com>

chriswlai had a problem deploying to ml-commons-cicd-env-require-approval August 6, 2025 21:22 — with GitHub Actions Failure

more refactoring

db2f515

Signed-off-by: chrislai <chrlaii@amazon.com>

chriswlai had a problem deploying to ml-commons-cicd-env-require-approval August 6, 2025 21:26 — with GitHub Actions Failure

pyek-bot reviewed Aug 6, 2025

View reviewed changes

add MLModelTracer

ce2f479

Signed-off-by: chrislai <chrlaii@amazon.com>

chriswlai changed the title ~~Connector Tracing~~ Connector and Model Tracing Aug 6, 2025

chriswlai had a problem deploying to ml-commons-cicd-env-require-approval August 6, 2025 23:15 — with GitHub Actions Failure

chriswlai had a problem deploying to ml-commons-cicd-env-require-approval August 6, 2025 23:15 — with GitHub Actions Error

use MLModelTracer

4e3a8aa

Signed-off-by: chrislai <chrlaii@amazon.com>

chriswlai had a problem deploying to ml-commons-cicd-env-require-approval August 6, 2025 23:21 — with GitHub Actions Failure

chriswlai had a problem deploying to ml-commons-cicd-env-require-approval August 6, 2025 23:21 — with GitHub Actions Error

pyek-bot reviewed Aug 7, 2025

View reviewed changes

pyek-bot approved these changes Aug 7, 2025

View reviewed changes

mingshl merged commit d7d0781 into opensearch-project:feature/agent-tracing Aug 7, 2025
6 of 10 checks passed

chriswlai added a commit to chriswlai/ml-commons that referenced this pull request Aug 8, 2025

Connector and Model Tracing (opensearch-project#4004)

e9ff3cd

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Connector and Model Tracing #4004

Connector and Model Tracing #4004

Uh oh!

chriswlai commented Jul 25, 2025 •

edited

Loading

Uh oh!

pyek-bot Aug 6, 2025

Uh oh!

pyek-bot left a comment

Uh oh!

pyek-bot Aug 6, 2025

Uh oh!

chriswlai Aug 6, 2025

Uh oh!

pyek-bot Aug 7, 2025

Uh oh!

pyek-bot Aug 6, 2025

Uh oh!

chriswlai Aug 6, 2025

Uh oh!

pyek-bot Aug 6, 2025

Uh oh!

chriswlai Aug 6, 2025

Uh oh!

pyek-bot Aug 6, 2025

Uh oh!

chriswlai Aug 6, 2025

Uh oh!

pyek-bot Aug 7, 2025

Uh oh!

pyek-bot left a comment

Uh oh!

Uh oh!

Uh oh!

		MLConnectorTracer.resetForTest();
		MLConnectorTracer.initialize(NoopTracer.INSTANCE, mlFeatureEnabledSetting);

Connector and Model Tracing #4004

Connector and Model Tracing #4004

Uh oh!

Conversation

chriswlai commented Jul 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Related Issues

Check List

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pyek-bot left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pyek-bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

chriswlai commented Jul 25, 2025 •

edited

Loading