Skip to content

feat/Add liteAgent support - #955

Open
haneric00 wants to merge 25 commits into
mainfrom
fix/crewAI-cr-09-liteAgentExecution
Open

haneric00 wants to merge 25 commits into
mainfrom
fix/crewAI-cr-09-liteAgentExecution

Conversation

@haneric00

Copy link
Copy Markdown
Contributor

Previously liteAgent executions would create malformed spans, the reason being that liteAgent execution handlers were not implemented in ADOT yet.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

- fix gen_ai_input_message in start function
- rewrite complete function for code quality
- formatting for error function
@haneric00
haneric00 requested a review from a team as a code owner October 27, 2026 20:33
@aws-observability aws-observability deleted a comment from github-actions Bot Oct 28, 2026
@aws-observability aws-observability deleted a comment from github-actions Bot Oct 28, 2026
@aws-observability aws-observability deleted a comment from github-actions Bot Oct 28, 2026
@aws-observability aws-observability deleted a comment from github-actions Bot Oct 28, 2026
@github-actions

github-actions Bot commented Nov 4, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @haneric00's task in 2m 13s —— View job


Claude Code Review

Todo List:

  • Check prior review comments to avoid duplicates
  • Read the PR diff (vs. origin/main)
  • Review current state of changed files in pr-head/
  • Post review feedback

New commits since my last review (5323b00 "fix test"): 0affd28 — a merge from main that only brings in the post-release 0.22.0 version bump (#964). The two PR files (_event_handler.py and test_crewai_instrumentor.py) are byte-identical to the previously reviewed state (git diff 5323b00..HEAD touches only version.py, CHANGELOG.md, and daily-scan.yml).

Re: @haneric00's inline comment on line 344 — "find evidence for why liteAgent spans must be its own trace"

This is a reasonable thing to double-check — and worth being precise about, because what the code actually does differs from what "its own trace" suggests.

The call is:

self._start_span(
    span_name, event.event_id, attributes, self._get_parent_event_id(event), root_event_id=event.event_id
)

root_event_id=event.event_id does not force a new trace. Looking at _start_span (_event_handler.py:619-659):

  • Trace parentage is determined by parent_event_id → parent_entry. If _get_parent_event_id(event) resolves to a known span, the lite-agent span continues under it. If no parent is resolvable, the branch elif root_event_id is not None or parent_event_id is None: parent_ctx = context.get_current() runs, so the span joins the active OTEL context (e.g. an incoming HTTP request trace).
  • root_event_id is purely a cleanup grouping key — it determines which entries _end_spans_for_root_event sweeps when the lite agent completes. Setting it to event.event_id makes the lite-agent span the top of its own cleanup scope, so a lone Agent.kickoff() can end cleanly without an enclosing crew/task event ever having been seen.

So the WHY is: LiteAgent events have no enclosing CrewKickoffStarted/TaskStarted to anchor a cleanup root, unlike regular agent events which inherit a root from a crew/task that was already registered in _event_id_to_span. root_event_id=event.event_id fills that gap for the Crew-less Agent.kickoff() entrypoint. Trace parentage still falls through to whatever parent context is available (parent_event_id → active OTEL context → nothing).

If you want this in the code, the existing comment block at lines 297–303 could be extended with one line, e.g.:

# root_event_id=event.event_id anchors cleanup since LiteAgent has no enclosing Crew/Task root; trace parentage still falls through to the active context.

Status of prior review items (still open; handler bytes unchanged)

  • ⚠️ (1) Misplaced pylint: disable=unused-argument on _on_lite_agent_started (_event_handler.py:305). Both source (line 307: agent = source) and event (line 335: getattr(event, "messages", None)) are used; the disable is unnecessary and should be removed. Sibling _on_lite_agent_error (line 363) places it correctly.
  • ⚠️ (2) _on_lite_agent_completed writes GEN_AI_OUTPUT_MESSAGES directly on the span (_event_handler.py:350–358) rather than flowing it through _end_span. Works correctly; consistency item only.
  • ⚠️ (3) _get_task_or_agent_id → agent_entry mapping not mirrored for lite agents. _on_agent_start adds this at _event_handler.py:270–271; _on_lite_agent_started does not. Tests pass via the active-context fallback — only a bug if any Agent.kickoff() path emits an orphan event keyed only by agent_id.
  • ⚠️ Agent.Kickoff() capitalization in comment (_event_handler.py:301). Method is Agent.kickoff() (lowercase k). Trivial.
  • ℹ️ Reduced test coverage (from 9d08038) — non-blocking; flagged previously.

Nothing blocking from my side.
· Branch

haneric00 and others added 2 commits November 5, 2026 16:01
…umentation/crewai/_event_handler.py

Co-authored-by: Steve Liu <liustve@amazon.com>
…umentation/crewai/_event_handler.py

Co-authored-by: Steve Liu <liustve@amazon.com>
@github-actions

github-actions Bot commented Nov 6, 2026 •

Copy link
Copy Markdown
Contributor

Todo List:

  • Check prior review comments to avoid duplicates
  • Read the PR diff (vs. origin/main)
  • Review current state of changed files in pr-head/
  • Post review feedback

View job run · Branch

haneric00 and others added 2 commits November 5, 2026 16:05
…umentation/crewai/_event_handler.py

Co-authored-by: Steve Liu <liustve@amazon.com>
Co-authored-by: Steve Liu <liustve@amazon.com>
@github-actions

github-actions Bot commented Nov 6, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @haneric00's task in 3m 9s —— View job


Todo List:

  • Check prior review comments to avoid duplicates
  • Read the PR diff (vs. origin/main)
  • Review current state of changed files in pr-head/
  • Post review feedback

Claude Code Review

New commits since my last review (5323b00 "fix test"):

  • f8d173f — adds the WHY block above the TYPE_CHECKING/runtime import at _event_handler.py:123–127 describing when LiteAgentExecution* events are emitted (addresses @liustve's line‑135 suggestion).
  • d8d0ad5 — in _on_lite_agent_started, extracts and serializes GEN_AI_TOOL_DEFINITIONS from event.tools or agent.tools at _event_handler.py:348–351 (addresses @liustve's line‑346 suggestion, partially).
  • d52d6c5 — adds the WHY block above _start_span(..., root_event_id=event.event_id) at _event_handler.py:353–357 explaining the cleanup‑root choice (addresses @liustve's line‑360 suggestion and @haneric00's own inline question).
  • a24094d — mirrors the _get_task_or_agent_id → agent_entry fallback mapping for lite agents at _event_handler.py:361–363 (addresses @liustve's line‑364 suggestion and my prior item 3).

New finding

  • test_lite_agent_kickoff will fail against the new handler. Commit d8d0ad5 now sets GEN_AI_TOOL_DEFINITIONS on the lite‑agent span whenever event.tools / agent.tools is non‑empty. The test at test_crewai_instrumentor.py:581 constructs the agent with tools=[tool_a] (line 590) and then compares attributes to an exact dict at line 631–642 after popping only GEN_AI_INPUT_MESSAGES, GEN_AI_OUTPUT_MESSAGES, and GEN_AI_REQUEST_STOP_SEQUENCES. GEN_AI_TOOL_DEFINITIONS will remain in attributes and the equality assertion will fail. @liustve's original suggestion included the matching test change (pop + assert the deserialized tool definitions); only the handler half was applied. Please add the companion pop in the test (and ideally the structural assertion from the review suggestion), or constrain the handler to only set the attribute when tools are user‑supplied on the event.

Status of prior review items (still open; _on_lite_agent_started body is otherwise unchanged)

  • ⚠️ (1) Misplaced pylint: disable=unused-argument on _on_lite_agent_started (_event_handler.py:310). Both source (line 312: agent = source) and event (line 340: getattr(event, "messages", None)) are used; the disable comment is unnecessary and should be removed. Sibling _on_lite_agent_error (line 381) places it correctly.
  • ⚠️ (2) _on_lite_agent_completed writes GEN_AI_OUTPUT_MESSAGES directly on the span (_event_handler.py:368–377) rather than flowing it through _end_span as _on_agent_completed does. Works correctly; purely a consistency item.
  • ✅ (3) _get_task_or_agent_id → agent_entry mapping — resolved by a24094d.
  • ⚠️ Agent.Kickoff() capitalization in comment (_event_handler.py:306). Method is Agent.kickoff() (lowercase k). Trivial.
  • ℹ️ @liustve's validate_otel_genai_schema suggestion for test_lite_agent_kickoff / test_lite_agent_kickoff_with_message_list (test file lines 616, 620, 656) — not applied. The current exact-dict compares verify we emit the JSON written here but don't verify it matches the OTel GenAI message schemas. Non‑blocking, but worth considering alongside the test fix above since you'll be touching the same block.
  • ℹ️ Reduced test coverage (from 9d08038) — non-blocking; flagged previously.

The test regression from d8d0ad5 is the only concern I'd call blocking. Everything else is unchanged from prior rounds.
· Branch

@github-actions

github-actions Bot commented Nov 6, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @haneric00's task in 1m 4s —— View job


Claude Code Review

New commits since my last review (f8d173f):

  • d8d0ad5 — adds GEN_AI_TOOL_DEFINITIONS extraction in _on_lite_agent_started (_event_handler.py:348–351)
  • d52d6c5 — adds the WHY block above _start_span(..., root_event_id=event.event_id) (_event_handler.py:353–357)
  • a24094d — mirrors the _get_task_or_agent_id → agent_entry fallback mapping for lite agents (_event_handler.py:361–363)
  • 67e148c — "fix lint issue and apply validate otel genai schema function call to testing": adds too-many-locals to the pylint disable on _on_lite_agent_started (line 309), pops GEN_AI_TOOL_DEFINITIONS with the structural assertion in test_lite_agent_kickoff, and routes input/output messages through validate_otel_genai_schema in test_lite_agent_kickoff and test_lite_agent_kickoff_with_message_list.

Status of prior review items

  • ✅ Test regression from d8d0ad5 — resolved by 67e148c. test_lite_agent_kickoff now pops GEN_AI_TOOL_DEFINITIONS with the structural assertion suggested by @liustve, and the pop happens before the final attributes equality check.
  • ✅ @liustve's validate_otel_genai_schema suggestion — applied in 67e148c for test_lite_agent_kickoff (:616, :619) and test_lite_agent_kickoff_with_message_list (:673).
  • ✅ (3) _get_task_or_agent_id → agent_entry mapping for lite agents — resolved by a24094d.
  • ⚠️ (1) Misplaced pylint: disable=unused-argument on _on_lite_agent_started (_event_handler.py:310) — still open. Both source (line 312: agent = source) and event (line 340: getattr(event, "messages", None)) are used in this method; the disable is unnecessary and should be removed. Sibling _on_lite_agent_error (line 381) places it correctly next to the actually-unused parameter.
  • ⚠️ (2) _on_lite_agent_completed writes GEN_AI_OUTPUT_MESSAGES directly on the span (_event_handler.py:371–376) rather than flowing it through _end_span as _on_agent_completed does. Works correctly; purely a consistency item.
  • ⚠️ Agent.Kickoff() capitalization in comment (_event_handler.py:306). Method is Agent.kickoff() (lowercase k). Trivial.
  • ℹ️ Reduced test coverage (from 9d08038) — non-blocking; flagged previously.

Nothing blocking from my side.
· Branch

@github-actions

github-actions Bot commented Nov 7, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @haneric00's task in 1m 48s —— View job


Claude Code Review

New commits since my last review (67e148c):

  • 5b103bc "fix error leak and add tests (fix error leak and add tests #962)" — merged into this branch via ec57a1b. Rewrites _end_span / _end_spans_for_root_event to classify error.type as a low-cardinality exception class name (new _get_error_type helper), keeps the full message in the status description, and adds tests to prove the message doesn't leak into error.type. Reviewed and merged independently to main; not a change authored by this PR.
  • ec57a1b — the merge commit itself.

The three lite-agent handler methods (_event_handler.py:303–383) and the lite-agent tests are byte-identical to the previously reviewed state (git diff 67e148c..HEAD -- <PR files> shows only the #962 changes). No new findings on the lite-agent additions.

Status of prior review items (still open)

  • ⚠️ (1) Misplaced pylint: disable=unused-argument on _on_lite_agent_started (_event_handler.py:311). Both source (line 313: agent = source) and event (line 341: getattr(event, "messages", None)) are used in this method; the disable is unnecessary and should be removed. Sibling _on_lite_agent_error (line 382) places it correctly next to the actually-unused parameter.
  • ⚠️ (2) _on_lite_agent_completed writes GEN_AI_OUTPUT_MESSAGES directly on the span (_event_handler.py:371–377) rather than flowing it through _end_span as _on_agent_completed does. Works correctly; purely a consistency item.
  • ⚠️ Agent.Kickoff() capitalization in comment (_event_handler.py:307). Method is Agent.kickoff() (lowercase k). Trivial.
  • ℹ️ Reduced test coverage (from 9d08038) — non-blocking; flagged previously.

Note on the merge interaction

_get_error_type (new in 5b103bc) uses sys.exc_info()[1] as a fallback when the failure event's error is a string. _on_lite_agent_error at _event_handler.py:382 passes getattr(event, "error", None) straight through to _end_spans_for_root_event, which is exactly what the other *_error handlers do, so the lite-agent error path inherits the new behavior correctly — the LiteAgent exception class ends up on error.type when the handler runs synchronously inside CrewAI's except block (which is where CrewAI emits LiteAgentExecutionErrorEvent from). No action needed, just confirming the two land cleanly together.

Nothing blocking from my side.
· · Branch

@aws-observability aws-observability deleted a comment from github-actions Bot Nov 7, 2026
@aws-observability aws-observability deleted a comment from github-actions Bot Nov 7, 2026
@aws-observability aws-observability deleted a comment from github-actions Bot Nov 7, 2026
@aws-observability aws-observability deleted a comment from github-actions Bot Nov 7, 2026
@aws-observability aws-observability deleted a comment from github-actions Bot Nov 7, 2026
@aws-observability aws-observability deleted a comment from github-actions Bot Nov 7, 2026
@aws-observability aws-observability deleted a comment from github-actions Bot Nov 7, 2026
@aws-observability aws-observability deleted a comment from github-actions Bot Nov 7, 2026
@aws-observability aws-observability deleted a comment from github-actions Bot Nov 7, 2026
@aws-observability aws-observability deleted a comment from github-actions Bot Nov 7, 2026
1. fix import randomly removed during merge
2. fix comment
3. fix unnecessary pylint disable
4. fix test to reflect recent changes to how errors are captured
@github-actions

github-actions Bot commented Nov 7, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @haneric00's task in 1m 30s —— View job


Claude Code Review

New commits since my last review (67e148c):

  • 5b103bc "fix error leak and add tests (fix error leak and add tests #962)" — merged in via ec57a1b; this is upstream main content (not authored on this PR).
  • 2f20609 "fix claude comments" — the author's follow-up on this PR, addressing three of my prior items:
    • Agent.Kickoff() → Agent.kickoff() at _event_handler.py:307 ✅
    • Removed the misplaced # pylint: disable=unused-argument on _on_lite_agent_started at _event_handler.py:311 ✅
    • Restored LLMCallFailedEvent import in the test file (test_crewai_instrumentor.py:25) that was dropped during the merge ✅
    • Updated test_lite_agent_llm_call_failure (test_crewai_instrumentor.py:747–748) to the new error.type contract from fix error leak and add tests #962 — asserts error.type == "RuntimeError" and that "Service unavailable" is in status.description ✅

Status of prior review items

  • ✅ (1) Misplaced pylint: disable=unused-argument — resolved by 2f20609.
  • ✅ Agent.Kickoff() capitalization — resolved by 2f20609.
  • ✅ Test alignment with the new _get_error_type behavior from fix error leak and add tests #962 — resolved by 2f20609.
  • ⚠️ (2) _on_lite_agent_completed writes GEN_AI_OUTPUT_MESSAGES directly on the span (_event_handler.py:371–377) rather than flowing it through _end_span as _on_agent_completed does. Still open; works correctly, purely a consistency item.
  • ℹ️ Reduced test coverage (from 9d08038) — non-blocking; flagged previously.

New findings

None. The lite-agent handler and tests behave as intended on the current tip.

Nothing blocking from my side.
· Branch

@haneric00

Copy link
Copy Markdown
Contributor Author

To address the AI generated comment about writing directly to the span on lite_agent_completed vs. using end_span, the reason to keep this is that lite_agent_completed needs to end not just its own span but all children spans. Using end_span would only end itself, and the alternative adds complexity that increases the risk of getting the span ordering wrong (based on when each span completes).

The current system works so I think its best to keep it this way for now.

@github-actions

github-actions Bot commented Nov 10, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @haneric00's task in 1m 44s —— View job


Claude Code Review

New commits since my last review (2f20609 "fix claude comments"):

git diff 2f20609..HEAD -- <PR files> is empty — the lite-agent handler and tests are byte-identical to the previously reviewed state.

Status of prior review items

  • ✅ (1) Misplaced pylint: disable=unused-argument — resolved by 2f20609.
  • 🗣️ (2) _on_lite_agent_completed writes GEN_AI_OUTPUT_MESSAGES directly on the span — closed by the author (2026-11-07): routing through _end_span would only end the lite-agent span itself, not its children, and would add ordering risk. Keeping the direct write.
  • ✅ Agent.Kickoff() capitalization — resolved by 2f20609.
  • ✅ Test alignment with _get_error_type from fix error leak and add tests #962 — resolved by 2f20609.
  • ℹ️ Reduced test coverage from 9d08038 — non-blocking; flagged previously.

Note on the merge interaction with #971

#971 tightens the nested-CLIENT-span processor to require a local, non-remote parent and uses (trace_id, span_id) keys. The lite-agent additions don't touch that processor, and the test_lite_agent_llm_call_failure / test_lite_agent_kickoff tests assert that chat gpt-4 child spans sit under the invoke_agent parent in the same trace — which is exactly what the stricter processor needs to still demote the outer CLIENT span to INTERNAL. No action needed; just confirming the two land cleanly together.

Nothing blocking from my side.
· Branch

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants