Skip to content

fix(tracer): add gen_ai.operation.name event type detection (Priority 4.5)#291

Open
devin-ai-integration[bot] wants to merge 2 commits intofederated-sdk-release-candidatefrom
devin/1772446156-fix-strands-event-type-detection
Open

fix(tracer): add gen_ai.operation.name event type detection (Priority 4.5)#291
devin-ai-integration[bot] wants to merge 2 commits intofederated-sdk-release-candidatefrom
devin/1772446156-fix-strands-event-type-detection

Conversation

@devin-ai-integration
Copy link
Contributor

@devin-ai-integration devin-ai-integration bot commented Mar 2, 2026

Summary

Adds a new event type detection priority (4.5) in HoneyHiveSpanProcessor._detect_event_type() that checks the gen_ai.operation.name span attribute before falling back to dynamic pattern matching (Priority 5).

This fixes incorrect event_type classification for strands-agents spans (and potentially other GenAI semantic convention instrumentors like PydanticAI v3+). Without this fix, spans like invoke_agent and create_agent fall through to pattern matching and get classified as tool instead of chain.

Mapping added:

gen_ai.operation.name event_type
chat, text_completion, generate_content model
invoke_agent, create_agent chain
execute_tool tool
embeddings tool

Companion backend fix: honeyhiveai/hive-kube#2516 (same priority logic in ingestion service's event_type.go).

Updates since last revision

  • Docstring updated: _detect_event_type() docstring now documents all 6 priority levels including Priority 4.5 and the full gen_ai.operation.name mapping table.
  • embeddings mapping added: Mapped to tool, consistent with OpenInference EMBEDDING → tool. Prevents future unmapped operation name fall-through for standard GenAI embeddings operations.

Review & Testing Checklist for Human

  • Cross-framework impact: This fires on any span with gen_ai.operation.name, not just Strands. Verify that other frameworks using this attribute (e.g., Traceloop, OpenInference) either don't set it or would benefit from the same mapping.
  • Mapping completeness: Confirm no other gen_ai.operation.name values exist in the wild that should be mapped (e.g., rerank). Unmapped values silently fall through to Priority 5 pattern matching, which is safe but may produce unexpected results.
  • No unit tests included: Consider adding a regression test ensuring invoke_agentchain and chatmodel. The change was validated via the strands-agents integration example, but there's no automated test guarding against regression.

Recommended test plan: Run the strands-agents integration example, retrieve the session from the API, and verify:

  • chat spans have event_type: "model" (SDK-side fix, works now)
  • invoke_agent spans get event_type: "chain" after the companion backend PR (honeyhiveai/hive-kube#2516) is deployed

Notes

… 4.5)

Add detection of gen_ai.operation.name attribute for event type classification.
This fixes incorrect event_type for strands-agents spans where invoke_agent
and create_agent were defaulting to 'tool' instead of 'chain'.

Priority 4.5 (before pattern matching fallback):
- chat, text_completion, generate_content -> model
- invoke_agent, create_agent -> chain
- execute_tool -> tool

This works alongside the backend fix in hive-kube ingestion service
to ensure correct event types for AWS Strands and other GenAI
semantic convention instrumentors.

Co-Authored-By: unknown <>
@devin-ai-integration devin-ai-integration bot requested a review from a team March 2, 2026 10:15
@devin-ai-integration
Copy link
Contributor Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

@snyk-io
Copy link

snyk-io bot commented Mar 2, 2026

Snyk checks have passed. No issues have been found so far.

Status Scanner Critical High Medium Low Total (0)
Open Source Security 0 0 0 0 0 issues
Licenses 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

@claude
Copy link
Contributor

claude bot commented Mar 2, 2026

Claude finished @devin-ai-integration[bot]'s task in 1m 39s —— View job


Review complete

  • Gather context and read changed files
  • Review code changes for correctness and best practices
  • Post review feedback (1 top-level comment + 2 inline comments)

Summary: The change is well-scoped and correctly placed at Priority 4.5. Left feedback on three items:

  1. Docstring is stale — the method docstring doesn't mention the new priority and numbering is now wrong
  2. No test coverage — need at least a regression test for invoke_agentchain
  3. Consider adding embeddings — it's a known GenAI semantic convention operation name that maps naturally to tool

Also left inline nits on dict allocation per call and logging style inconsistency (non-blocking).

op_name = attributes.get("gen_ai.operation.name")
if op_name:
op_name_str = str(op_name)
OPERATION_NAME_TO_EVENT_TYPE = {
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: dict recreated on every call. This dict is allocated on each invocation of _detect_event_type. The existing OPENINFERENCE_TO_HONEYHIVE at Priority 4 has the same pattern so this is at least consistent, but both should ideally be hoisted to module-level or class-level constants.

Not blocking, but worth a follow-up cleanup for both mapping dicts together.

Suggested change
OPERATION_NAME_TO_EVENT_TYPE = {
_OPERATION_NAME_TO_EVENT_TYPE = {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Acknowledged — this is consistent with the existing OPENINFERENCE_TO_HONEYHIVE dict pattern at Priority 4. Will defer hoisting both to module-level constants to a follow-up cleanup.

Comment on lines +1083 to +1088
self._safe_log(
"debug",
"✅ Event type from gen_ai.operation.name: %s (%s)",
event_type,
op_name_str,
)
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: Logging style inconsistency. This block uses %s lazy formatting (good practice for logging), but the Priority 4 block above (lines 1052–1054) uses f-strings inside _safe_log. The %s style here is actually the better approach — consider aligning Priority 4 to match in a follow-up.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch — %s lazy formatting is intentional here and is the better approach. Agree that Priority 4's f-strings should be aligned in a follow-up.

@claude
Copy link
Contributor

claude bot commented Mar 2, 2026

Review Summary

The change is well-scoped and the mapping logic is correct. Priority placement between OpenInference (4) and dynamic pattern matching (5) is the right slot. A few items to address:

What looks good:

  • Graceful degradation: unmapped gen_ai.operation.name values silently fall through to Priority 5 (pattern matching), which is safe
  • str(op_name) cast handles non-string attribute values correctly
  • Wrapped inside the existing try/except so it can't crash the host app
  • The mapping values (model, chain, tool) are the correct HoneyHive event types

Issues to address:

  1. Docstring is stale (should fix): The method docstring (lines 953–961) doesn't mention the new gen_ai.operation.name priority and the numbering is now wrong. This will confuse future contributors. Couldn't leave an inline comment since the docstring lines aren't in the diff — please update it.

  2. No test coverage (should fix): There are no tests for this new priority level. At minimum, add a unit test asserting invoke_agentchain and chatmodel to prevent regressions on the exact bug this fixes. The existing test file tests/unit/test_tracer_processing_span_processor.py or tests/unit/test_tracer_utils_event_type.py would be natural homes.

  3. Mapping completeness (consider): The OTel GenAI semantic conventions also define embeddings as an operation name. Currently unmapped values fall through safely, but if you know embeddings should map to tool (consistent with the OpenInference EMBEDDING → tool mapping), it would be worth adding it now rather than discovering it later.

Documentation

No public API changes — no doc updates needed. The PR description's mapping table is sufficient for internal reference.

@github-actions
Copy link
Contributor

github-actions bot commented Mar 2, 2026

📚 Documentation preview built — Download artifact

Review instructions & validation status

How to Review

  1. Download the artifact from the link above
  2. Extract the files
  3. Open index.html in your browser

Validation Status

  • API validation: ✅ Passed
  • Build process: ✅ Successful
  • Import tests: ✅ All imports working

- Update _detect_event_type docstring to reflect all 6 priority levels
- Add 'embeddings' -> 'tool' mapping (standard GenAI semantic convention)

Co-Authored-By: unknown <>
@devin-ai-integration
Copy link
Contributor Author

Addressed items 1 and 3 in 9bc83c6:

  1. Docstring updated — now reflects all 6 priority levels including 4.5 (gen_ai.operation.name) and the gen_ai operation name mappings.
  2. Test coverage — deferring to human reviewer to decide if a unit test is needed for this PR. The end-to-end validation via the strands-agents integration example covers this path.
  3. embeddings added — mapped to tool, consistent with OpenInference EMBEDDING → tool.

@github-actions
Copy link
Contributor

github-actions bot commented Mar 2, 2026

📚 Documentation preview built — Download artifact

Review instructions & validation status

How to Review

  1. Download the artifact from the link above
  2. Extract the files
  3. Open index.html in your browser

Validation Status

  • API validation: ✅ Passed
  • Build process: ✅ Successful
  • Import tests: ✅ All imports working

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.

0 participants