Replace AgentOps mock server with bypassable client #202

acured · 2025-10-23T09:22:52Z

Issue:
agentops mock service and OTL batchprocessor shutdown are not synchronized:
🖇 AgentOps: Network error during span export: HTTPConnectionPool(host='localhost', port=41733): Max retries exceeded with url: /traces (Caused by NewConnectionError('<urllib3.connection.HTTPConnection object at 0x7e0a521ffd10>: Failed to establish a new connection: [Errno 111] Connection refused'))

Bug: https://github.com/microsoft/agent-lightning/issues/51

Fixed:

override batchprocessor exporter.
set auto start session to manual start.

ultmaster · 2025-10-24T04:27:53Z

agentlightning/instrumentation/agentops.py

        return self.server_port
+
+
+class SwitchableAuthenticatedOTLPExporter(AuthenticatedOTLPExporter):


We might need to add tests for the new exporters, to make sure they do not crash or raise more warnings.

I think this should be something like Bypassable...Exporter (switchable is kind of confusing and chinglish...

ultmaster · 2025-10-24T04:28:29Z

agentlightning/tracer/agentops.py

            ctx = self._lightning_span_processor.with_context(store=store, rollout_id=rollout_id, attempt_id=attempt_id)
            with ctx as processor:
+                kwargs: dict[str, Any] = {}
+                if name is not None:


suggest wrap "end_trace" in finally.

Also I remember end_trace can report a job status. Is that supported now?

ultmaster · 2025-10-24T04:28:50Z

agentlightning/tracer/agentops.py


            if not agentops.get_client().initialized:
-                agentops.init()  # type: ignore
+                agentops.init(auto_start_session=False)  # type: ignore


now the agnetops mock server may not be needed any more. Suggest removing them in this PR.

acured · 2025-10-24T07:13:45Z

agentlightning/tracer/agentops.py

+                    yield processor
+                except:
+                    # Need logging or raise here?
+                    status = StatusCode.ERROR  # type: ignore


Do we need logging or raise here?

I'm not sure tracer trace_context can catch errors from runner. Need to test it. Maybe when this except catches something, it's a critical error related to tracer.

Anyway logging doesn't harm. especially logger.debug

ultmaster · 2025-10-24T07:25:58Z

agentlightning/instrumentation/agentops.py

+_switch = False
+
+
+def set_switch(value: bool):


rename it as something like bypass_agentops_service(enabled: bool)

Now I look at it, switch is not a good name. Very confusing.

ultmaster · 2025-10-24T07:26:25Z

agentlightning/instrumentation/agentops.py

-    app = agentops_local_server()
-    app.run(**kwargs)

+class SwitchableOTLPMetricExporter(OTLPMetricExporter):


add docstrings for new classes

ultmaster · 2025-10-24T17:19:55Z

agentlightning/instrumentation/agentops.py

        return self.server_port
+
+
+class SwitchableAuthenticatedOTLPExporter(AuthenticatedOTLPExporter):


I think this should be something like Bypassable...Exporter (switchable is kind of confusing and chinglish...

ultmaster · 2025-10-24T17:20:22Z

agentlightning/tracer/agentops.py

+                finally:
+                    agentops.end_trace(trace, end_state=status)  # type: ignore
        elif store is None and rollout_id is None and attempt_id is None:
            with self._lightning_span_processor:


so you don't care about this path?

what do you think?

Missing, fixed.

ultmaster · 2025-10-24T17:20:58Z

agentlightning/tracer/agentops.py

+            base_url = f"http://dummy"
+            env_vars_to_set = {
+                "AGENTOPS_API_KEY": "dummy",
+                "AGENTOPS_API_ENDPOINT": base_url,


I think api endpoint and app url and exporter endpoint no longer need to be overwritten.

What do you think?

Removed, but we have to keep "AGENTOPS_API_KEY" avoid initialization failed in agentops sdk.

Copilot

Pull Request Overview

This PR addresses synchronization issues between the AgentOps mock service and OTL batch processor shutdown by replacing the local mock server approach with a switchable exporter pattern that can toggle communication with the AgentOps service.

Key Changes:

Replaced the AgentOps local mock server implementation with switchable exporters/clients that can operate in local mode
Modified AgentOps initialization to use manual session start instead of auto-start
Added trace context management with proper status tracking and error handling

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.

File	Description
tests/instrumentation/test_agentops.py	New test file validating switchable exporter behavior when toggling service communication on/off
agentlightning/tracer/agentops.py	Removed server manager, simplified worker initialization to use dummy endpoint, added trace lifecycle management with status tracking
agentlightning/instrumentation/agentops.py	Replaced mock server implementation with switchable exporters/clients that inherit from AgentOps classes and conditionally execute based on service enabled flag

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-10-24T17:23:14Z

agentlightning/instrumentation/agentops.py

+    """
+    global _agentops_service_enabled
+    _agentops_service_enabled = enabled
+    logger.info(f"Switch set to {value} for exporters and clients.")


The log message references an undefined variable value. It should reference enabled instead.

Suggested change

logger.info(f"Switch set to {value} for exporters and clients.")

logger.info(f"Switch set to {enabled} for exporters and clients.")

Copilot · 2025-10-24T17:23:15Z

agentlightning/instrumentation/agentops.py

-            logger.warning(f"AgentOps still not ready after {attempt} attempts. Retrying...")
+    def fetch_auth_token(self, *args: Any, **kwargs: Any) -> AuthTokenResponse:
+        if _agentops_service_enabled:
+            resp = super().post(*args, **kwargs)


Method calls super().post() but the parent class V3Client has a fetch_auth_token method, not a generic post method. This should call super().fetch_auth_token(*args, **kwargs) to properly delegate to the parent implementation.

Suggested change

resp = super().post(*args, **kwargs)

resp = super().fetch_auth_token(*args, **kwargs)

Copilot · 2025-10-24T17:23:15Z

agentlightning/tracer/agentops.py

-                    f"[Worker {worker_id}] AgentOps managed, but local server port is not available. Client may not connect as expected."
-                )
+
+            base_url = f"http://dummy"


[nitpick] Hardcoded dummy URL should be defined as a module-level constant (e.g., DUMMY_AGENTOPS_ENDPOINT = \"http://dummy\") to improve maintainability and make it easier to locate if it needs to be changed.

Skip this improvement.

I think it actually makes sense if we don't need to set API_ENDPOINT any more.

Copilot · 2025-10-24T17:23:16Z

tests/instrumentation/test_agentops.py

+    SwitchableAuthenticatedOTLPExporter,
+    SwitchableOTLPMetricExporter,
+    SwitchableOTLPSpanExporter,
+    set_switch,


Import references set_switch function which does not exist in the module. Based on the implementation in agentops.py, this should be enable_agentops_service instead.

Copilot · 2025-10-24T17:23:16Z

tests/instrumentation/test_agentops.py

+    with patch.object(
+        switchable_authenticated_exporter.__class__.__bases__[0], "export", return_value=SpanExportResult.SUCCESS
+    ) as mock_export:
+        set_switch(True)


Function set_switch is called but does not exist in the imported module. This should call enable_agentops_service(True) based on the actual implementation.

ultmaster · 2025-10-28T04:50:33Z

Please resolve the conflicts

ultmaster · 2025-10-28T06:28:47Z

agentlightning/tracer/agentops.py

            logger.warning("instrument_managed=False. You are responsible for all instrumentation.")

    def __getstate__(self):
        state = self.__dict__.copy()


if that's removed, we don't need getstate and setstate any more.

ultmaster · 2025-10-28T06:29:15Z

agentlightning/tracer/agentops.py

+            base_url = f"http://dummy"
+            env_vars_to_set = {
+                "AGENTOPS_API_KEY": "dummy",
+                "AGENTOPS_API_ENDPOINT": base_url,


What do you think?

ultmaster · 2025-10-28T06:29:35Z

agentlightning/tracer/agentops.py

-                    f"[Worker {worker_id}] AgentOps managed, but local server port is not available. Client may not connect as expected."
-                )
+
+            base_url = f"http://dummy"


I think it actually makes sense if we don't need to set API_ENDPOINT any more.

ultmaster · 2025-10-28T06:30:15Z

agentlightning/tracer/agentops.py

+                    yield processor
+                except Exception as e:
+                    status = StatusCode.ERROR  # type: ignore
+                    logger.debug(f"Trace failed for rollout_id={rollout_id}, attempt_id={attempt_id}, error={e}")


that should be logger.error at least

Yes changed.

ultmaster · 2025-10-28T06:30:37Z

agentlightning/tracer/agentops.py

+                finally:
+                    agentops.end_trace(trace, end_state=status)  # type: ignore
        elif store is None and rollout_id is None and attempt_id is None:
            with self._lightning_span_processor:


what do you think?

ultmaster · 2025-10-28T06:31:36Z

/ci

github-actions · 2025-10-28T06:31:47Z

🚀 CI Watcher for correlation id-3454803530-mha6v8ny triggered by comment 3454803530
🏃‍♀️ Tracking 6 workflow run(s):

🟢 PR #202 - Label ci-all - id-3454803530-mha6v8ny — completed/success
🟢 PR #202 - Label ci-all - id-3454803530-mha6v8ny — completed/success
🟢 PR #202 - Label ci-all - id-3454803530-mha6v8ny — completed/success
🟢 PR #202 - Label ci-all - id-3454803530-mha6v8ny — completed/success
🟢 PR #202 - Label ci-all - id-3454803530-mha6v8ny — completed/success
🟢 PR #202 - Label ci-all - id-3454803530-mha6v8ny — completed/success

✅ All runs completed.

acured · 2025-10-28T07:03:45Z

Please resolve the conflicts

Fixed.

ultmaster · 2025-10-28T13:36:30Z

agentlightning/tracer/agentops.py

+            else:
+                raise ValueError("store, rollout_id, and attempt_id must be either all provided or all None")
+        except Exception as e:
+            status = StatusCode.ERROR  # type: ignore


Have you tested whether the exception inside the trace_context will be caught here?

Yes, unless an exception is handled in def _exit (_, exce_type, exce, tb), it can be caught externally.

please add a test case.

ultmaster · 2025-10-28T13:37:12Z

agentlightning/tracer/agentops.py

+        kwargs: dict[str, Any] = {}
+        if name is not None:
+            kwargs["trace_name"] = str(name)
+        trace = agentops.start_trace(**kwargs)


does start_trace have other kwargs?

Args: trace_name: Name for the trace (e.g., "session", "my_custom_task"). tags: Optional tags to attach to the trace span (list of strings or dict).

maybe we can use: agentops.start_trace(trace_name=str(name))

if name is None, we can use rollout_id?

ultmaster · 2025-10-28T13:38:04Z

/ci

github-actions · 2025-10-28T13:38:18Z

🚀 CI Watcher for correlation id-3456516543-mham3qlk triggered by comment 3456516543
🏃‍♀️ Tracking 6 workflow run(s):

🟢 PR #202 - Label ci-all - id-3456516543-mham3qlk — completed/success
🟢 PR #202 - Label ci-all - id-3456516543-mham3qlk — completed/success
🟢 PR #202 - Label ci-all - id-3456516543-mham3qlk — completed/success
🟢 PR #202 - Label ci-all - id-3456516543-mham3qlk — completed/success
🟢 PR #202 - Label ci-all - id-3456516543-mham3qlk — completed/success
🟢 PR #202 - Label ci-all - id-3456516543-mham3qlk — completed/success

✅ All runs completed.

ultmaster · 2025-10-29T01:45:05Z

@acured Please avoid force push. This will toss away the diff and make review difficult. When merging, we will squash. So the chaotic commit history on the feature branch does not matter anyway.

acured · 2025-10-29T02:53:35Z

@acured Please avoid force push. This will toss away the diff and make review difficult. When merging, we will squash. So the chaotic commit history on the feature branch does not matter anyway.

OK

…plorter_patch

…ally

acured · 2025-10-31T10:18:51Z

/ci

github-actions · 2025-10-31T10:19:03Z

🚀 CI Watcher for correlation id-3472335385-mhepb2dm triggered by comment 3472335385
🏃‍♀️ Tracking 6 workflow run(s):

🟢 PR #202 - Label ci-all - id-3472335385-mhepb2dm — completed/success
🟢 PR #202 - Label ci-all - id-3472335385-mhepb2dm — completed/success
🔴 PR #202 - Label ci-all - id-3472335385-mhepb2dm — completed/failure
🟢 PR #202 - Label ci-all - id-3472335385-mhepb2dm — completed/success
🟢 PR #202 - Label ci-all - id-3472335385-mhepb2dm — completed/success
🟢 PR #202 - Label ci-all - id-3472335385-mhepb2dm — completed/success

✅ All runs completed.

github-actions · 2025-10-31T11:39:25Z

🚀 CI Watcher for correlation id-3472679590-mhes6f6j triggered by comment 3472679590
🏃‍♀️ Tracking 6 workflow run(s):

🟢 PR #202 - Label ci-all - id-3472679590-mhes6f6j — completed/success
🟢 PR #202 - Label ci-all - id-3472679590-mhes6f6j — completed/success
🟢 PR #202 - Label ci-all - id-3472679590-mhes6f6j — completed/success
🟢 PR #202 - Label ci-all - id-3472679590-mhes6f6j — completed/success
🟢 PR #202 - Label ci-all - id-3472679590-mhes6f6j — completed/success
🟢 PR #202 - Label ci-all - id-3472679590-mhes6f6j — completed/success

✅ All runs completed.

--------- Co-authored-by: Hao Ni (CSI Interfusion Co Ltd) <[email protected]>

acured force-pushed the hao/add_agentops_explorter_patch branch from 135849d to 8f52ca1 Compare October 24, 2025 01:30

acured marked this pull request as ready for review October 24, 2025 03:02

ultmaster reviewed Oct 24, 2025

View reviewed changes

acured commented Oct 24, 2025

View reviewed changes

acured closed this Oct 24, 2025

acured reopened this Oct 24, 2025

ultmaster reviewed Oct 24, 2025

View reviewed changes

acured closed this Oct 24, 2025

acured reopened this Oct 24, 2025

ultmaster reviewed Oct 24, 2025

View reviewed changes

ultmaster added the ci-all label Oct 24, 2025

ultmaster marked this pull request as draft October 24, 2025 17:22

ultmaster marked this pull request as ready for review October 24, 2025 17:22

Copilot AI review requested due to automatic review settings October 24, 2025 17:22

Copilot AI reviewed Oct 24, 2025

View reviewed changes

acured force-pushed the hao/add_agentops_explorter_patch branch from 085c604 to 1543515 Compare October 27, 2025 01:27

acured closed this Oct 27, 2025

acured reopened this Oct 27, 2025

acured closed this Oct 27, 2025

acured reopened this Oct 27, 2025

acured force-pushed the hao/add_agentops_explorter_patch branch from e7c6eba to 4a05d89 Compare October 28, 2025 01:51

acured closed this Oct 28, 2025

acured reopened this Oct 28, 2025

acured force-pushed the hao/add_agentops_explorter_patch branch from 4a05d89 to 03212cf Compare October 28, 2025 05:53

ultmaster reviewed Oct 28, 2025

View reviewed changes

add agentops explorter patch

84dea21

Hao Ni (CSI Interfusion Co Ltd) added 3 commits October 28, 2025 15:20

fix StatusCode enum not found issue

11da8f0

fix trace all path and rename exporter.

54558a3

remove __getstate__ and __setstate__ in tracet

a306fc2

acured force-pushed the hao/add_agentops_explorter_patch branch from 9b2efe2 to a306fc2 Compare October 28, 2025 07:21

acured mentioned this pull request Oct 28, 2025

Add upload_every_n_tasks to upload to AgentOps service #30

Closed

ultmaster reviewed Oct 28, 2025

View reviewed changes

Hao Ni (CSI Interfusion Co Ltd) added 4 commits October 29, 2025 10:56

Merge remote-tracking branch 'upstream/main' into hao/add_agentops_ex…

8787b0e

…plorter_patch

Merge remote-tracking branch 'upstream/main' into hao/add_agentops_ex…

71caa9d

…plorter_patch

Merge remote-tracking branch 'upstream/main' into hao/add_agentops_ex…

f4c2fb4

…plorter_patch

fix ut and add test on whether caught contextmanager exception extern…

6891161

…ally

acured closed this Oct 31, 2025

acured reopened this Oct 31, 2025

Hao Ni (CSI Interfusion Co Ltd) added 3 commits October 31, 2025 15:01

remove unnecessary check.

7ef0a00

remove mock server in cli

84d0075

fix lint

b58cff7

acured closed this Oct 31, 2025

acured reopened this Oct 31, 2025

Hao Ni (CSI Interfusion Co Ltd) added 2 commits October 31, 2025 17:55

fix coroutine issue

0a1ce7c

change test to subprocess run

1da7456

ultmaster changed the title ~~Add agentops client(explorter) patch~~ Replace AgentOps mock server with bypassable client Oct 31, 2025

ultmaster merged commit 3f372ff into microsoft:main Oct 31, 2025
8 checks passed

totoluo pushed a commit to totoluo/agent-lightning that referenced this pull request Nov 14, 2025

Replace AgentOps mock server with bypassable client (microsoft#202)

e8d879e

--------- Co-authored-by: Hao Ni (CSI Interfusion Co Ltd) <[email protected]>

		return self.server_port


		class SwitchableAuthenticatedOTLPExporter(AuthenticatedOTLPExporter):

	logger.info(f"Switch set to {value} for exporters and clients.")
	logger.info(f"Switch set to {enabled} for exporters and clients.")

	resp = super().post(args, *kwargs)
	resp = super().fetch_auth_token(args, *kwargs)

Replace AgentOps mock server with bypassable client #202

Replace AgentOps mock server with bypassable client #202

Uh oh!

Conversation

acured commented Oct 23, 2025

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!

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!

acured Oct 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Oct 24, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 24, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 24, 2025

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!

Copilot AI Oct 24, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

acured Oct 28, 2025 •

edited

Loading

github-actions bot commented Oct 28, 2025 •

edited

Loading