Skip to content

Fix/crewAI wrong version skips instrumentation, fix instrumentation error fails customer call - #953

Open
haneric00 wants to merge 21 commits into
mainfrom
fix/crewai-cr-01-version-gate
Open

haneric00 wants to merge 21 commits into
mainfrom
fix/crewai-cr-01-version-gate

Conversation

@haneric00

@haneric00 haneric00 commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

fix: the CrewAI instrumentor now skips instrumentation, with a warning, when the installed crewai is outside its
supported range (>= 1.10.0, < 2). Under auto-instrumentation the range was not enforced, so Crew.kickoff()
raised AttributeError on crewai 0.177.0-1.9.x.

  • fix in aws_opentelemetry_distro.py, previously upstream layer was explicitly setting skip dependency checks to true, so for the entrypoint into the ADOT layer, we flip this back. This resolves the above issue with crewai, and any related issues in the other instrumentation libraries.

fix: CrewAI instrumentation errors no longer fail the customer's call. The crewai_event_bus.emit wrapper now
catches and debug-logs exceptions from the ADOT event handler, so the event is still emitted and kickoff()
returns normally; only telemetry for that event is lost. LLM events emitted from a source other than a CrewAI
BaseLLM now produce a chat span without token usage instead of raising AttributeError.

  • fix in event_handler.py

fix: token-usage lookups no longer assume the event source is a baseLLM. For other sources the chat span is now recorded without token counts instead of being dropped.

  • New helper method _get_cumulative_token_usage to replace get_token_usage_summary safely. Now we get either the actual token values or the ability to input default values in case get_token_usage_summary is not callable on the object.

Tests added:

  • test_llm_events_from_non_base_llm_source
    • proves the token usage fix works: LLM events from a non-BaseLLM source still get a chat span, with no made-up token counts
  • test_handler_error_does_not_fail_kickoff
    • runs a real crew.kickoff() with a custom LLM whose input makes our handler raise, and asserts the error is caught and kickoff still returns its result

test_aws_opentelemetry_distro.py:

  • test_load_instrumentor_enforces_genai_dependency_ranges
    • for all five AWS GenAI instrumentors, out-of-range or missing framework versions are skipped and supported versions get instrumented
  • test_load_instrumentor_keeps_skip_dep_check_for_other_entry_points
    • non-AWS instrumentors are loaded exactly as before
  • updated test_load_native_when_no_third_party, test_load_native_when_mode_enabled and test_mode_value_is_case_insensitive to expect the new dependency-check arguments

Removed:

  • TestCrewAIInstrumentorVersionGate, replaced by the distro-level test
  • TestEventBusEmitWrapperFailOpen, replaced by test_handler_error_does_not_fail_kickoff

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

… range

Auto-instrumentation loads instrumentors with skip_dep_check=True, so the
declared crewai >= 1.10.0, < 2 range was never enforced. On crewai
0.177.0-1.9.x the event handler reads event_id/started_event_id, which
those versions lack, and Crew.kickoff() raised AttributeError.

_instrument now runs the dependency check itself and, on a conflict, logs
a warning and leaves crewai_event_bus.emit unwrapped.
@haneric00
haneric00 requested a review from a team as a code owner October 23, 2026 22:28
@haneric00 haneric00 changed the title Fix/crewai CR01, CR02 Fix/crewAI wrong version skips instrumentation, fix instrumentation error fails customer call Oct 23, 2026
@aws-observability aws-observability deleted a comment from github-actions Bot Oct 30, 2026
@aws-observability aws-observability deleted a comment from github-actions Bot Oct 30, 2026
@aws-observability aws-observability deleted a comment from github-actions Bot Oct 30, 2026
@aws-observability aws-observability deleted a comment from github-actions Bot Oct 30, 2026
@aws-observability aws-observability deleted a comment from github-actions Bot Oct 30, 2026
@aws-observability aws-observability deleted a comment from github-actions Bot Oct 30, 2026
@aws-observability aws-observability deleted a comment from github-actions Bot Oct 30, 2026
@aws-observability aws-observability deleted a comment from github-actions Bot Oct 30, 2026
@aws-observability aws-observability deleted a comment from github-actions Bot Oct 30, 2026
@aws-observability aws-observability deleted a comment from github-actions Bot Oct 30, 2026
@aws-observability aws-observability deleted a comment from github-actions Bot Oct 30, 2026
@github-actions

github-actions Bot commented Nov 2, 2026 •

Copy link
Copy Markdown
Contributor

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


Claude PR Review

  • Read prior reviews on this PR
  • Review full PR diff
  • Dedupe findings against prior reviews
  • Post findings

All 4 prior review items from @liustve have been addressed in this PR:

  1. Dependency range enforcement moved to AwsOpenTelemetryDistro.load_instrumentor for all 5 AWS GenAI instrumentors with skip_dep_check=False, raise_exception_on_conflict=True (aws_opentelemetry_distro.py:271-278)
  2. Tuple unpacked to prompt_tokens, completion_tokens in _event_handler.py
  3. test_handler_error_does_not_fail_kickoff drives the regression through crew.kickoff() with mocking at the provider boundary only
  4. TestCrewAIInstrumentorVersionGate removed and replaced by test_load_instrumentor_enforces_genai_dependency_ranges in the distro tests, covering all 5 frameworks with below-lower-bound, at/above-upper-bound, missing, and supported versions. The _installed_framework_versions helper patches importlib.metadata.distribution and keeps the real parent loader and dependency check unmocked, as requested.
  5. The "why" comment next to the skip_dep_check=False override explains the instruments extras vs instrumentation_dependencies() gap.

No new substantive issues to flag.
· Branch: fix/crewai-cr-01-version-gate


@staticmethod
def _get_cumulative_token_usage(llm: Any) -> Optional[Tuple[int, int]]:
# only BaseLLM exposes a usage summary; LLM events can also be emitted from other sources (e.g. customer code)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

find example of external sources that don't have token count, also why

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

if necessary change tests to reflect external sources: either no token count or token count available from these sources

@github-actions

github-actions Bot commented Nov 5, 2026 •

Copy link
Copy Markdown
Contributor

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


Claude PR Review

  • Read prior reviews on this PR
  • Review full PR diff
  • Dedupe findings against prior reviews
  • Post findings

All prior review items from @liustve (4 items on commit abfb2e0) and the author self-review threads from @haneric00 remain addressed in head 34f20aa:

  1. Dependency range enforcement lives in AwsOpenTelemetryDistro.load_instrumentor for all 5 AWS GenAI entry points with skip_dep_check=False, raise_exception_on_conflict=True (aws_opentelemetry_distro.py:271-278); the "why" comment next to the override explains the instruments extras vs instrumentation_dependencies() gap.
  2. Token-usage tuple is unpacked into prompt_tokens, completion_tokens in _event_handler.py:461-466, and the shared _get_cumulative_token_usage helper at _event_handler.py:530-537 returns None for non-BaseLLM sources so callers fall through to (0, 0) / previous — no fabricated token counts.
  3. test_handler_error_does_not_fail_kickoff drives the regression through Crew.kickoff() with mocking only at the custom-LLM boundary, asserts on the exported spans and the kickoff result, and verifies no agent retry fires.
  4. TestCrewAIInstrumentorVersionGate is gone, replaced by test_load_instrumentor_enforces_genai_dependency_ranges (test_aws_opentelemetry_distro.py:778-786) covering all 5 frameworks with below-lower-bound, at/above-upper-bound, missing, and supported versions. The _installed_framework_versions helper patches importlib.metadata.distribution only and keeps the real parent loader and dependency check unmocked.
  5. The latest commit (5630025) adds test_llm_events_from_non_base_llm_source_with_reported_usage, which complements the existing non-BaseLLM test by proving that when a non-BaseLLM source does report usage on the completed event, token counts are recorded — directly addressing the "find example of external sources that don't have token count, also why" / "change tests to reflect external sources" thread.

No new substantive issues to flag.
· Branch: fix/crewai-cr-01-version-gate ·

@github-actions

github-actions Bot commented Nov 6, 2026 •

Copy link
Copy Markdown
Contributor

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


Claude PR Review

  • Read prior reviews on this PR
  • Review full PR diff
  • Dedupe findings against prior reviews
  • Post findings

All prior review items from @liustve (4 items on commit abfb2e0) and the author self-review threads from @haneric00 remain addressed in head 68fca64. Since the previous review (34f20aa), the only new commit is a merge from main — no changes to the PR's substantive code:

  1. Dependency range enforcement lives in AwsOpenTelemetryDistro.load_instrumentor for all 5 AWS GenAI entry points with skip_dep_check=False, raise_exception_on_conflict=True (aws_opentelemetry_distro.py:271-278); the "why" comment next to the override explains the instruments extras vs instrumentation_dependencies() gap.
  2. Token-usage tuple is unpacked into prompt_tokens, completion_tokens in _event_handler.py:461-466, and the shared _get_cumulative_token_usage helper at _event_handler.py:530-537 returns None for non-BaseLLM sources so callers fall through to (0, 0) / previous — no fabricated token counts.
  3. test_handler_error_does_not_fail_kickoff drives the regression through Crew.kickoff() with mocking only at the custom-LLM boundary, asserts on the exported spans and the kickoff result, and verifies no agent retry fires.
  4. TestCrewAIInstrumentorVersionGate is gone, replaced by test_load_instrumentor_enforces_genai_dependency_ranges (test_aws_opentelemetry_distro.py:778-786) covering all 5 frameworks with below-lower-bound, at/above-upper-bound, missing, and supported versions. The _installed_framework_versions helper patches importlib.metadata.distribution only and keeps the real parent loader and dependency check unmocked.
  5. test_llm_events_from_non_base_llm_source_with_reported_usage complements the existing non-BaseLLM test by proving that when a non-BaseLLM source does report usage on the completed event, token counts are recorded — directly addressing the "find example of external sources that don't have token count, also why" / "change tests to reflect external sources" thread.

No new substantive issues to flag.
· Branch: fix/crewai-cr-01-version-gate

@github-actions

github-actions Bot commented Nov 7, 2026 •

Copy link
Copy Markdown
Contributor

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


Claude PR Review

  • Read prior reviews on this PR
  • Review full PR diff
  • Dedupe findings against prior reviews
  • Post findings

All prior review items from @liustve (4 items on commit abfb2e0) and the author self-review threads from @haneric00 remain addressed in head 51c1c83. Since the previous Claude review (68fca64), the only new commit is a merge from main (which brought in #962's unrelated error-leak fix) — no changes to this PR's substantive scope:

  1. Dependency range enforcement lives in AwsOpenTelemetryDistro.load_instrumentor for all 5 AWS GenAI entry points with skip_dep_check=False, raise_exception_on_conflict=True (aws_opentelemetry_distro.py:271-278); the "why" comment next to the override explains the instruments extras vs instrumentation_dependencies() gap.
  2. Token-usage tuple is unpacked into prompt_tokens, completion_tokens in _event_handler.py:462-467, and the shared _get_cumulative_token_usage helper at _event_handler.py:531-538 returns None for non-BaseLLM sources so callers fall through to the previous snapshot / (0, 0) — no fabricated token counts.
  3. test_handler_error_does_not_fail_kickoff drives the regression through Crew.kickoff() with mocking only at the custom-LLM boundary, asserts on the exported spans and the kickoff result, and verifies no agent retry fires.
  4. TestCrewAIInstrumentorVersionGate is gone, replaced by test_load_instrumentor_enforces_genai_dependency_ranges (test_aws_opentelemetry_distro.py:778-786) covering all 5 frameworks with below-lower-bound, at/above-upper-bound, missing, and supported versions. The _installed_framework_versions helper patches importlib.metadata.distribution only and keeps the real parent loader and dependency check unmocked.
  5. test_llm_events_from_non_base_llm_source_with_reported_usage complements the existing non-BaseLLM test by proving that when a non-BaseLLM source does report usage on the completed event, token counts are recorded — directly addressing the "find example of external sources that don't have token count, also why" / "change tests to reflect external sources" thread.

No new substantive issues to flag.
· Branch: fix/crewai-cr-01-version-gate

@github-actions

github-actions Bot commented Nov 7, 2026 •

Copy link
Copy Markdown
Contributor

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


Claude PR Review

  • Read prior reviews on this PR
  • Review full PR diff
  • Dedupe findings against prior reviews
  • Post findings

All prior review items from @liustve (4 items on commit abfb2e0) and the author self-review threads from @haneric00 remain addressed in head a01e7dc. The only new commit since the previous Claude review (51c1c83) tightens the baseLLM comment at _event_handler.py:533-534 and removes test_llm_events_from_non_base_llm_source_with_reported_usage as redundant:

  1. Dependency range enforcement lives in AwsOpenTelemetryDistro.load_instrumentor for all 5 AWS GenAI entry points with skip_dep_check=False, raise_exception_on_conflict=True (aws_opentelemetry_distro.py:271-278); the "why" comment next to the override explains the instruments extras vs instrumentation_dependencies() gap.
  2. Token-usage tuple is unpacked into prompt_tokens, completion_tokens in _event_handler.py:462-467, and the shared _get_cumulative_token_usage helper at _event_handler.py:531-539 returns None for non-BaseLLM sources so callers fall through to the previous snapshot / (0, 0) — no fabricated token counts.
  3. test_handler_error_does_not_fail_kickoff drives the regression through Crew.kickoff() with mocking only at the custom-LLM boundary, asserts on the exported spans and the kickoff result, and verifies no agent retry fires.
  4. TestCrewAIInstrumentorVersionGate is gone, replaced by test_load_instrumentor_enforces_genai_dependency_ranges (test_aws_opentelemetry_distro.py:778-786) covering all 5 frameworks with below-lower-bound, at/above-upper-bound, missing, and supported versions. The _installed_framework_versions helper patches importlib.metadata.distribution only and keeps the real parent loader and dependency check unmocked.
  5. The removed test_llm_events_from_non_base_llm_source_with_reported_usage covered a non-BaseLLM source emitting LLMCallCompletedEvent.usage; that path flows through _on_llm_completed's isinstance(event_usage, dict) branch, which is source-agnostic and already covered by test_per_call_token_usage_prefers_event_usage and test_per_call_token_usage_provider_key_variants. No coverage gap introduced. The complementary test_llm_events_from_non_base_llm_source still proves the no-usage fallback.

No new substantive issues to flag.
· Branch: fix/crewai-cr-01-version-gate

@github-actions

github-actions Bot commented Nov 10, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @haneric00's task in 49s —— View job


Claude PR Review

  • Read prior reviews on this PR
  • Review full PR diff
  • Dedupe findings against prior reviews
  • Post findings

All prior review items from @liustve (4 items on commit abfb2e0) and the author self-review threads from @haneric00 remain addressed in head f4a369b. Since the previous Claude review (a01e7dc), the only new commit is a merge from main (which brought in #971's unrelated nested CLIENT span processor fix) — no changes to this PR's substantive scope:

  1. Dependency range enforcement lives in AwsOpenTelemetryDistro.load_instrumentor for all 5 AWS GenAI entry points with skip_dep_check=False, raise_exception_on_conflict=True (aws_opentelemetry_distro.py:271-278); the "why" comment next to the override explains the instruments extras vs instrumentation_dependencies() gap.
  2. Token-usage tuple is unpacked into prompt_tokens, completion_tokens in _event_handler.py:462-467, and the shared _get_cumulative_token_usage helper at _event_handler.py:531-539 returns None for non-BaseLLM sources so callers fall through to the previous snapshot / (0, 0) — no fabricated token counts.
  3. test_handler_error_does_not_fail_kickoff drives the regression through Crew.kickoff() with mocking only at the custom-LLM boundary, asserts on the exported spans and the kickoff result, and verifies no agent retry fires.
  4. TestCrewAIInstrumentorVersionGate is gone, replaced by test_load_instrumentor_enforces_genai_dependency_ranges (test_aws_opentelemetry_distro.py:778-786) covering all 5 frameworks with below-lower-bound, at/above-upper-bound, missing, and supported versions. The _installed_framework_versions helper patches importlib.metadata.distribution only and keeps the real parent loader and dependency check unmocked.
  5. The baseLLM comment tightened in a01e7dc and the removal of the redundant non-BaseLLM-with-reported-usage test remain in place; _on_llm_completed's isinstance(event_usage, dict) branch is source-agnostic and is still covered by test_per_call_token_usage_prefers_event_usage and test_per_call_token_usage_provider_key_variants. The complementary test_llm_events_from_non_base_llm_source still proves the no-usage fallback.

No new substantive issues to flag.
· Branch: fix/crewai-cr-01-version-gate

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