feat: Python otel support, enriching spans, OpenAI Agents SDK example #1839
Conversation
|
WalkthroughThis pull request introduces a new event enrichment utility and integrates it into the OTLPExporter’s trace and log export methods. It also refines resource span filtering logic. In addition, several new helper functions and tests are added to support enriched event data formatting. Changes in core modules add a new carrier-from-context function and an execution environment attribute. Updates in the Python package improve tracing context handling and error reporting. Finally, major modifications and new files in the references (d3-demo) section add support for a chat-based workflow, CSV data enrichment, and build extensions for Playwright and Python agents. Changes
Sequence Diagram(s)sequenceDiagram
participant E as OTLPExporter
participant U as EnrichCreatableEvents
participant L as Logger
participant R as Event Repo
E->>+U: Call enrichCreatableEvents(events)
U->>U: Process and enrich events
U-->>-E: Return enrichedEvents
E->>L: Log enrichedEvents
E->>R: Insert enrichedEvents
sequenceDiagram
participant D as d3Demo Task
participant H as handleCSVRow
participant C as rowAgentCoordinator
participant R as rowAgentRunner
participant E as enrichmentResultsEvaluator
D->>+H: Process CSV chunk
H->>+C: Trigger enrichment for row
C->>+R: Execute Python enrichment
R-->>-C: Return enrichment result
C-->>-H: Aggregate results
H->>E: Evaluate and compile outcomes
E-->>D: Return final enriched data
Possibly related PRs
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (10)
references/d3-demo/src/extensions/playwright.ts (1)
1-36: Well-implemented Playwright extension with good containerization practices.The extension correctly installs Playwright with Chromium and necessary dependencies. The implementation follows good container build practices such as combining RUN statements, cleaning up temporary files, and properly configuring environment variables.
Consider pinning the Playwright version in line 17 for better build reproducibility:
- `RUN npm install -g playwright`, + `RUN npm install -g [email protected]`, // Use specific versionreferences/d3-demo/src/trigger/d3Demo.ts (2)
30-32: Possible large CSV memory considerations.
Loading the entire CSV into memory can be problematic for very large files. A streaming solution or chunked ingestion approach could help mitigate out-of-memory issues.
177-183: Optimize reduce pattern to avoid accumulating spreads.
Using{ ...acc, ...result }in a.reduce()can cause O(n^2) complexity. For large arrays, this becomes costly. You could directly mutate a single object or push into arrays instead.-const combinedResult = results.reduce( - (acc, result) => ({ - ...acc, - ...result, - }), - {} as RowEnrichmentResult -); +const combinedResult = Object.assign({}, ...results);🧰 Tools
🪛 Biome (1.9.4)
[error] 179-179: Avoid the use of spread (
...) syntax on accumulators.Spread syntax should be avoided on accumulators (like those in
.reduce) because it causes a time complexity ofO(n^2).
Consider methods such as .splice or .push instead.(lint/performance/noAccumulatingSpread)
apps/webapp/app/v3/utils/enrichCreatableEvents.server.ts (2)
38-43: Potential lack of string escaping inrepr.
reprencloses strings with single quotes without escaping internal quotes or special characters. This might cause confusion if strings contain'or newline characters.-function repr(value: any): string { if (typeof value === "string") { - return `'${value}'`; + // Consider escaping internal single quotes, e.g.: + const escaped = value.replace(/'/g, "\\'"); + return `'${escaped}'`; } return String(value); }
56-67: Tiny redundancy in capturingactualKey.
Line 58 reassignsactualKeywith the samekeyfield, even whenhasRepris false.You could remove this extra variable assignment unless you plan to differentiating the key differently for
!rplaceholders.references/d3-demo/src/trigger/python/agent.py (2)
57-68: Handle potential missing environment variable and request exceptions.
os.environ['TRIGGER_API_URL']may raise aKeyErrorif the variable isn’t defined, andrequests.postcan raise network or HTTP-related exceptions beyondJSONDecodeError. Consider wrapping this with try-except or validating environment variables before use to prevent uncaught errors.
125-157: Strengthen error handling for request failures.Inside the
mainfunction, onlyJSONDecodeErroris caught. For more robust resilience, consider catching other exceptions (e.g.,HTTPError,Timeout,ConnectionError) when callingcomplete_wait_tokento handle potential network or server issues gracefully.packages/python/src/index.ts (3)
22-22: Validate carrier contents before injecting trace info.
carrierFromContext()may occasionally return an empty or undefinedtraceparent. Consider adding a fallback to ensure environment variables are set correctly. This helps avoid accidental undefined injections.Also applies to: 83-83, 147-147, 197-197, 245-245, 290-290
34-39: Caution when injecting taskContext attributes.Directly injecting all fields as
OTEL_RESOURCE_ATTRIBUTEScan inadvertently expose sensitive or personal data. If there's any risk thattaskContext.attributescontains PII or confidential information, consider sanitizing or filtering these attributes before passing them as environment variables.Also applies to: 94-101, 156-161, 206-211, 253-258, 298-303
113-116: Potential risk of sensitive data exposure in error messages.Revealing both
stdoutandstderrin the thrown error text can simplify debugging but might leak sensitive data. Consider carefully reviewing or sanitizing these outputs if security or privacy is a concern.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (7)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yamlreferences/d3-demo/public/file.svgis excluded by!**/*.svgreferences/d3-demo/public/globe.svgis excluded by!**/*.svgreferences/d3-demo/public/next.svgis excluded by!**/*.svgreferences/d3-demo/public/vercel.svgis excluded by!**/*.svgreferences/d3-demo/public/window.svgis excluded by!**/*.svgreferences/d3-demo/src/app/favicon.icois excluded by!**/*.ico
📒 Files selected for processing (20)
apps/webapp/app/v3/otlpExporter.server.ts(4 hunks)apps/webapp/app/v3/utils/enrichCreatableEvents.server.ts(1 hunks)apps/webapp/test/otlpExporter.test.ts(1 hunks)packages/core/src/v3/otel/utils.ts(2 hunks)packages/core/src/v3/semanticInternalAttributes.ts(1 hunks)packages/core/src/v3/workers/index.ts(1 hunks)packages/python/src/index.ts(17 hunks)references/agent-loop/src/trigger/schemas.ts(0 hunks)references/agent-loop/trigger.config.ts(0 hunks)references/d3-demo/package.json(3 hunks)references/d3-demo/requirements.in(1 hunks)references/d3-demo/requirements.txt(1 hunks)references/d3-demo/src/app/page.tsx(1 hunks)references/d3-demo/src/components/main-app.tsx(3 hunks)references/d3-demo/src/extensions/playwright.ts(1 hunks)references/d3-demo/src/trigger/chat.ts(1 hunks)references/d3-demo/src/trigger/d3Demo.ts(1 hunks)references/d3-demo/src/trigger/python/agent.py(1 hunks)references/d3-demo/src/trigger/schemas.ts(1 hunks)references/d3-demo/trigger.config.ts(1 hunks)
💤 Files with no reviewable changes (2)
- references/agent-loop/src/trigger/schemas.ts
- references/agent-loop/trigger.config.ts
🧰 Additional context used
🧬 Code Definitions (6)
references/d3-demo/trigger.config.ts (1)
references/d3-demo/src/extensions/playwright.ts (1)
installPlaywrightChromium(4-36)
references/d3-demo/src/components/main-app.tsx (1)
references/d3-demo/src/trigger/chat.ts (1)
chatExample(8-98)
references/d3-demo/src/trigger/d3Demo.ts (2)
references/d3-demo/src/trigger/python/agent.py (2)
CSVRowPayload(20-23)RowEnrichmentResult(52-55)references/d3-demo/src/trigger/schemas.ts (4)
CSVRowPayload(12-16)CSVRowPayload(18-18)RowEnrichmentResult(20-38)RowEnrichmentResult(40-40)
references/d3-demo/src/trigger/schemas.ts (1)
references/d3-demo/src/trigger/python/agent.py (2)
CSVRowPayload(20-23)RowEnrichmentResult(52-55)
apps/webapp/app/v3/utils/enrichCreatableEvents.server.ts (1)
apps/webapp/app/v3/otlpExporter.server.ts (1)
events(101-107)
packages/python/src/index.ts (4)
packages/core/src/v3/otel/utils.ts (1)
carrierFromContext(24-29)packages/core/src/v3/workers/index.ts (1)
carrierFromContext(11-11)packages/core/src/v3/semanticInternalAttributes.ts (1)
SemanticInternalAttributes(1-61)packages/core/src/v3/index.ts (1)
SemanticInternalAttributes(21-21)
🪛 Biome (1.9.4)
references/d3-demo/src/trigger/d3Demo.ts
[error] 179-179: Avoid the use of spread (...) syntax on accumulators.
Spread syntax should be avoided on accumulators (like those in .reduce) because it causes a time complexity of O(n^2).
Consider methods such as .splice or .push instead.
(lint/performance/noAccumulatingSpread)
🪛 Ruff (0.8.2)
references/d3-demo/src/trigger/python/agent.py
71-72: f-string without any placeholders
Remove extraneous f prefix
(F541)
79-80: f-string without any placeholders
Remove extraneous f prefix
(F541)
87-88: f-string without any placeholders
Remove extraneous f prefix
(F541)
95-107: f-string without any placeholders
Remove extraneous f prefix
(F541)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: typecheck / typecheck
- GitHub Check: units / 🧪 Unit Tests
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (42)
references/d3-demo/src/app/page.tsx (1)
5-5: Updated token generation to use "chat-example" instead of "agent-loop-example"This change aligns with the broader refactoring in the PR, where the agent loop functionality has been renamed to chat functionality, making the codebase more consistent.
packages/core/src/v3/semanticInternalAttributes.ts (1)
60-60: Added new EXECUTION_ENVIRONMENT semantic attributeThis new attribute will allow for tracking or filtering spans based on execution environment, which enhances observability capabilities. The naming convention is consistent with other attributes in this file.
packages/core/src/v3/otel/utils.ts (2)
1-1: Updated imports to include OpenTelemetry context and propagationThe import has been correctly updated to include the new dependencies needed for the
carrierFromContextfunction.
24-29: Well-implemented carrier function for propagating trace contextThe
carrierFromContextfunction correctly creates a carrier object and injects the active context using OpenTelemetry's propagation API. This is an important addition for distributed tracing across service boundaries, particularly for the Python integration.references/d3-demo/trigger.config.ts (1)
1-20: Well-structured configuration with good Python and Playwright integration.The configuration file properly sets up the Trigger.dev project with Python and Playwright support. The 3600-second maximum duration provides ample time for longer-running tasks, and the integration of both the Python extension and custom Playwright extension is done correctly.
references/d3-demo/src/trigger/chat.ts (1)
8-10: Appropriate renaming from agent-loop to chat.The renaming from "agent-loop-example" to "chat-example" better describes the actual functionality, which is handling a chat interaction rather than a more complex agent loop concept. The changes are consistent across the exported constant name, id, and description.
packages/core/src/v3/workers/index.ts (1)
7-12: Improved export format and added carrier context functionality.The changes improve code readability by using a multi-line format for exports, which is consistent with other export statements in the file. The addition of
carrierFromContextis valuable for OpenTelemetry trace context propagation across services or components.references/d3-demo/src/trigger/schemas.ts (6)
1-8: Well-structured schema definition forAgentLoopMetadataThe schema is properly defined using Zod with clear object structure. The
waitTokenobject contains the necessary identifiers for token-based authentication.
10-10: Type export matches schema definitionGood practice to export the inferred type from the Zod schema, maintaining type safety throughout the application.
12-16: Clear and descriptive schema forCSVRowPayloadThe schema correctly validates CSV data with proper descriptions for each field. Using the
url()method for the URL field ensures valid URL formats.
18-18: Type export forCSVRowPayloadmatches schemaConsistent pattern with the previous type export, maintaining type consistency.
20-38: Well-structured nested object schema forRowEnrichmentResultThe schema defines a comprehensive data structure with proper nesting for different categories of information. Each field has clear descriptions and appropriate validation rules (like email validation and optional fields).
40-40: Type export forRowEnrichmentResultmatches schemaConsistent pattern with the previous type exports.
apps/webapp/app/v3/otlpExporter.server.ts (7)
30-30: Good addition of enrichment utility importThe import for
enrichCreatableEventsis properly added at the file level.
58-63: Proper implementation of event enrichment in exportTracesThe code now enriches events before logging and storing them, maintaining a consistent enrichment pattern. The span attribute for event count is correctly updated to use the enriched events.
65-68: Consistent usage of enriched events for repository operationsBoth immediate and standard insertion methods now use the enriched events.
85-90: Consistent implementation of event enrichment in exportLogsThe same enrichment pattern is properly applied to the
exportLogsmethod, maintaining code consistency between trace and log handling.
92-95: Consistent usage of enriched events in repository operations for logsSimilar to the trace handling, both immediate and standard insertion methods for logs now use the enriched events.
143-162: Enhanced resource span filtering logicThe filtering logic has been improved to handle execution environment attributes. The changes correctly:
- Allow resource spans without either trigger or execution environment attributes
- Allow resource spans with execution environment set to "trigger"
- Maintain backward compatibility with existing filter logic
164-164: Fixed conditional check for trigger attributeThe return statement now correctly handles the case where
triggerAttribute?.valuemight be undefined by returning false as the default.references/d3-demo/src/components/main-app.tsx (3)
10-10: Updated import type to reflect chat functionalityThe import type has been properly updated from
agentLoopExampletochatExample, aligning with the refactoring from agent loop to chat paradigm.
20-27: Renamed function and updated task identifierThe function has been appropriately renamed from
useAgentLooptouseChatto better reflect its purpose. The task identifier inuseRealtimeTaskTriggerWithStreamshas been updated to "chat-example" to match the changes in the backend.
89-89: Updated function usage in componentThe component correctly uses the renamed
useChatfunction while maintaining the same destructuring pattern for the returned values.apps/webapp/test/otlpExporter.test.ts (11)
1-10: Well-structured test setup with proper importsThe test file correctly imports testing utilities from Vitest and the function under test. The necessary types from related modules are also properly imported.
11-89: Comprehensive test for template string enrichment with !r modifierThis test thoroughly verifies the template string enrichment functionality with the
!rmodifier, which appears to format the value as a quoted string. The test data is detailed and realistic, and the assertions properly verify both the message enrichment and style changes.
91-167: Thorough test for template string enrichment without !r modifierSimilar to the previous test, but validating the behavior without the
!rmodifier. This correctly tests that values are inserted without quotes in this case.
169-192: Important edge case handling for missing propertiesThis test validates that the enrichment function gracefully handles missing properties, keeping the original message unchanged when a referenced property doesn't exist.
194-223: Multiple template variable substitution testGood test coverage for scenarios with multiple variables in a single message, including different variable types (string and number).
225-254: Non-string property value handling testTests the ability to handle numeric and boolean values correctly in template substitutions, validating the type conversion logic.
256-283: Test for messages without template variablesValidates that messages without variables remain unchanged while still applying style enrichment.
285-309: Style enrichment without gen_ai.system property testConfirms that the function preserves existing styles when there's no matching style enricher.
311-339: Priority handling in style enrichers testTests that the first matching style enricher is applied when multiple enrichers could match.
341-369: Fallback behavior for style enrichers testVerifies that the function falls back to secondary enrichers when the primary one doesn't match.
371-395: Style preservation testConfirms that existing styles are preserved when no enrichers match.
references/d3-demo/src/trigger/d3Demo.ts (2)
15-17: Consider handling potential fetch or network errors.
Failure to handle non-200 responses (e.g.,response.okchecks) or network issues could lead to unhandled exceptions or undefined behavior.Would you like me to create a follow-up script to confirm how often
fetchfails or returns error codes, or if any retry logic is needed?
154-160: You might be discarding the Python script results.
Theresultlogged at line 157 isn't returned to the caller. Instead, you return an empty object cast toRowEnrichmentResultat line 160, causing the subsequent workflow to see empty data.Do you intend to pass actual script outputs downstream? If so, consider returning the relevant data from
result.apps/webapp/app/v3/utils/enrichCreatableEvents.server.ts (1)
3-7: Functions are well-structured and easy to follow.references/d3-demo/package.json (3)
11-15: Python dependency management scripts look good.
The new scripts (python:compile-requirements, etc.) provide a clear workflow for environment setup. Ensure developers installing locally haveuvinstalled or instruct them to do so.
19-22: Pinned workspace dependencies and new “zod-to-json-schema”.
Pinning workspace dependencies can help with stability, but verify that other packages stay in sync. The addition ofzod-to-json-schemais consistent with usage ind3Demo.ts.Would you like a script to cross-check for conflicting versions across monorepo packages?
Also applies to: 31-32
40-42: New dev dependencies “trigger.dev” & “@trigger.dev/build”.
Including them as workspace dependencies centralizes version control. This helps maintain consistent build output and shared triggers.references/d3-demo/src/trigger/python/agent.py (1)
158-171: Guard against missing TRACEPARENT environment variable.If
TRACEPARENTis not set in the environment,os.environ['TRACEPARENT']will raise aKeyError. Replace with.get()or add a fallback to prevent runtime errors.Do you want to verify the presence of
TRACEPARENTbefore usage? You could add a conditional check or run a script to confirm it’s set.references/d3-demo/requirements.txt (1)
1-310: Pinned dependencies look consistent.All dependencies are pinned, supporting reproducible builds. Ensure you periodically update them and check for security advisories (especially for any pinned library with potential vulnerabilities).
⧓ Review in Butler Review
#4XfLF2XAifeat: Python otel support, enriching spans, OpenAI Agents SDK example
4 commit series (version 1)
Please leave review feedback in the Butler Review
Summary by CodeRabbit