feat(tracing): Add general AssociationProperty #3484
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds a Gemini-based chatbot sample with simulated tool integrations and Traceloop workflow observability, and introduces an associations API plus SDK integration to attach and propagate tracing associations through the Traceloop SDK and OpenTelemetry. Changes
Sequence Diagram(s)sequenceDiagram
participant User as CLI User
participant Main as main()
participant Process as process_message()
participant Gemini as Gemini GenAI
participant Router as execute_function()
participant Tools as Helpers (weather/time/knowledge)
participant Traces as Traceloop/Tracing
participant History as Conversation History
User->>Main: enter user text
Main->>Process: process_message(conversation_id,user_message,history)
activate Process
Process->>History: append user message
Process->>Traces: start workflow span (associations in context)
Process->>Gemini: request generation (may include tool_call)
Gemini-->>Process: response (text or tool_call)
alt tool_call present
loop tool call loop
Process->>Router: execute_function(name,args)
Router->>Tools: invoke helper
Tools-->>Router: return simulated result
Router-->>Process: tool result
Process->>History: append tool result
Process->>Gemini: continue generation with tool result
end
end
Process-->>Main: assistant text + updated history
deactivate Process
Main->>User: display assistant response
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (1)**/*.py📄 CodeRabbit inference engine (CLAUDE.md)
Files:
🧠 Learnings (1)📚 Learning: 2025-08-04T15:23:44.799ZApplied to files:
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
Comment |
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed a7ae838 in 37 seconds. Click for details.
- Reviewed
235lines of code in1files - Skipped
0files when reviewing. - Skipped posting
1draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. packages/sample-app/sample_app/gemini_chatbot_with_tools.py:1
- Draft comment:
The entire file is being deleted. The PR title mentions adding a 'conversation_id' attribute to span attributes, but the diff removes a sample file that used 'chat_id'. Please verify if this deletion was intentional and that the intended changes (i.e. adding 'conversation_id') are applied in the correct location. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
Workflow ID: wflow_2vwhVXTQV0RtiWcn
You can customize 
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed everything up to cd37f9a in 2 minutes and 11 seconds. Click for details.
- Reviewed
586lines of code in5files - Skipped
0files when reviewing. - Skipped posting
5draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. packages/sample-app/sample_app/chats/gemini_chatbot.py:128
- Draft comment:
Good use of Traceloop.set_conversation_id to attach the conversation identifier to spans. This ensures observability for each conversation. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
2. packages/sample-app/sample_app/gemini_chatbot_with_tools.py:128
- Draft comment:
Consider using 'conversation_id' instead of 'chat_id' for consistency. Currently, the attribute is set as 'chat_id' while tests and other modules use 'conversation_id'. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% This comment is making a claim about consistency with other parts of the codebase (tests and other modules) that I cannot verify from the diff. The rules explicitly state "Ignore cross-file issues. Only think about the file you are reviewing." This is clearly a cross-file consistency issue. Additionally, since this is a new file, there's no "change" being made to an existing naming convention - the author chosechat_idfrom the start. The comment would require me to look at other files to verify ifconversation_idis indeed the standard, which violates the review guidelines. Could the comment be valid if there's a clear codebase-wide convention that should be followed? Perhaps the automated tool has access to the full codebase and identified a legitimate inconsistency that would cause confusion or integration issues. Even if there is a codebase-wide convention, the rules explicitly state to ignore cross-file issues and only focus on the file being reviewed. Without being able to see the other files mentioned (tests and other modules), I cannot verify this claim, and per the instructions, I should delete comments that require more context from other files. The most important rule is that I need STRONG EVIDENCE the comment is correct, and I don't have that evidence in this diff. This comment should be deleted because it's a cross-file consistency issue that cannot be verified from the diff alone. The rules explicitly state to ignore cross-file issues and only review the file at hand. Without access to the tests and other modules mentioned, there's no strong evidence this comment is correct.
3. packages/traceloop-sdk/tests/test_conversation_id.py:15
- Draft comment:
Tests for conversation_id propagation look comprehensive and validate that conversation_id is correctly applied to both workflow and task spans. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
4. packages/traceloop-sdk/traceloop/sdk/__init__.py:206
- Draft comment:
The new static method set_conversation_id properly delegates to the underlying tracing function, ensuring central handling of the conversation id. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
5. packages/traceloop-sdk/traceloop/sdk/tracing/tracing.py:245
- Draft comment:
The set_conversation_id function attaches the conversation_id to the current span as intended. Ensure that the attribute name 'conversation_id' is used uniformly across all modules. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
Workflow ID: wflow_XPhRxGaTkLPG9bKW
You can customize
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/sample-app/sample_app/chats/gemini_chatbot.py (1)
215-222: Broad exception handling is acceptable for sample code.While the static analysis tool flags the broad
Exceptioncatch, this is reasonable for a sample CLI application where preventing crashes and providing a good user experience is prioritized over granular error handling.For production code, consider catching more specific exceptions (e.g., API-related errors, network errors).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
packages/sample-app/sample_app/chats/gemini_chatbot.py(1 hunks)packages/traceloop-sdk/tests/test_conversation_id.py(1 hunks)packages/traceloop-sdk/traceloop/sdk/__init__.py(2 hunks)packages/traceloop-sdk/traceloop/sdk/tracing/tracing.py(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Store API keys only in environment variables/secure vaults; never hardcode secrets in code
Use Flake8 for code linting and adhere to its rules
Files:
packages/traceloop-sdk/traceloop/sdk/__init__.pypackages/sample-app/sample_app/chats/gemini_chatbot.pypackages/traceloop-sdk/tests/test_conversation_id.pypackages/traceloop-sdk/traceloop/sdk/tracing/tracing.py
🧬 Code graph analysis (3)
packages/traceloop-sdk/traceloop/sdk/__init__.py (1)
packages/traceloop-sdk/traceloop/sdk/tracing/tracing.py (1)
set_conversation_id(245-252)
packages/traceloop-sdk/tests/test_conversation_id.py (2)
packages/traceloop-sdk/traceloop/sdk/__init__.py (1)
Traceloop(37-272)packages/traceloop-sdk/traceloop/sdk/tracing/tracing.py (1)
set_conversation_id(245-252)
packages/traceloop-sdk/traceloop/sdk/tracing/tracing.py (1)
packages/traceloop-sdk/traceloop/sdk/__init__.py (1)
set_conversation_id(206-207)
🪛 Ruff (0.14.6)
packages/sample-app/sample_app/chats/gemini_chatbot.py
220-220: Do not catch blind exception: Exception
(BLE001)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Test Packages (3.12)
- GitHub Check: Test Packages (3.10)
- GitHub Check: Test Packages (3.11)
- GitHub Check: Build Packages (3.11)
- GitHub Check: Lint
🔇 Additional comments (11)
packages/traceloop-sdk/tests/test_conversation_id.py (2)
5-29: LGTM!The test correctly validates that conversation_id propagates to spans when set before workflow execution. The assertions verify both span order and attribute values.
32-56: LGTM!The test correctly validates conversation_id propagation when set within a workflow. This complements the first test by covering the scenario where the ID is set during workflow execution.
packages/traceloop-sdk/traceloop/sdk/__init__.py (2)
30-30: LGTM!The import is correctly placed alongside related tracing functions.
205-207: LGTM!The method follows the same delegation pattern as
set_association_properties, maintaining API consistency.packages/traceloop-sdk/traceloop/sdk/tracing/tracing.py (2)
245-252: LGTM!The implementation correctly stores the conversation_id in both the context (for propagation to child spans) and the current span (for immediate attribution). The dual approach ensures coverage for both scenarios tested.
326-328: LGTM!The propagation logic correctly retrieves the conversation_id from context and sets it on new spans, consistent with other context-propagated attributes in the same function.
packages/sample-app/sample_app/chats/gemini_chatbot.py (5)
1-14: LGTM!The initialization correctly sources the API key from environment variables, adhering to the coding guideline that prohibits hardcoding secrets.
17-49: LGTM!The tool implementations use appropriate mock data for a sample application, making it easy to understand the chatbot's capabilities without external dependencies.
52-107: LGTM!The tool declarations are well-structured with proper schema definitions and clear descriptions for the Gemini model.
111-120: LGTM!The function execution router is straightforward and handles unknown functions gracefully.
123-186: LGTM!The workflow correctly demonstrates the use of
Traceloop.set_conversation_id()and implements a proper tool-call handling loop with conversation history management.
nina-kollman
changed the title
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed f6f8d60 in 1 minute and 0 seconds. Click for details.
- Reviewed
22lines of code in1files - Skipped
0files when reviewing. - Skipped posting
2draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. packages/traceloop-sdk/traceloop/sdk/__init__.py:29
- Draft comment:
Exposing 'associations' and 'AssociationProperty' in the top-level module is useful, but please add a brief docstring to clarify their intended use in the public API. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
2. packages/traceloop-sdk/traceloop/sdk/__init__.py:207
- Draft comment:
Assigning 'associations = associations' as a class attribute exposes the module; consider adding a comment or docstring to clarify its usage to avoid potential confusion with local variables. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
Workflow ID: wflow_xENdnKzeIcq6TXun
You can customize
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
packages/sample-app/sample_app/chats/gemini_chatbot.py (3)
136-186: Potential infinite loop if model repeatedly returns function calls.The
while Trueloop relies on the model eventually returning a text response. If the model continuously returns function calls (e.g., due to a malformed prompt or API issue), this loop will run indefinitely.Consider adding a maximum iteration limit:
+ MAX_TOOL_CALLS = 10 + tool_call_count = 0 + # Keep trying until we get a final response (handle tool calls) while True: + if tool_call_count >= MAX_TOOL_CALLS: + return "I apologize, but I'm having trouble completing this request.", conversation_history + # Generate content with tools response = client.models.generate_content( ... ) # Check if the model wants to use a tool if response.candidates[0].content.parts[0].function_call: + tool_call_count += 1 function_call = response.candidates[0].content.parts[0].function_call
148-152: Add defensive checks for response structure.Direct indexing into
response.candidates[0].content.parts[0]can raiseIndexErrororAttributeErrorif the response structure is unexpected (e.g., empty candidates or parts).- if response.candidates[0].content.parts[0].function_call: + if (response.candidates + and response.candidates[0].content + and response.candidates[0].content.parts + and response.candidates[0].content.parts[0].function_call): function_call = response.candidates[0].content.parts[0].function_call
220-222: Consider catching more specific exceptions.The static analysis tool flags catching the broad
Exceptionclass. For a sample app this is acceptable, but catching specific exceptions (e.g.,google.api_core.exceptions.GoogleAPIError) would provide better error handling.packages/traceloop-sdk/traceloop/sdk/__init__.py (1)
32-33: Add__all__to clarify public exports and silence linter.The
AssociationPropertyimport is for re-export purposes, which is valid. However, Flake8 flags it as unused (F401). Consider adding an__all__declaration to explicitly document the public API and silence the linter:__all__ = ["Traceloop", "AssociationProperty"]Alternatively, the sample app (
gemini_chatbot.py) correctly imports it, confirming the re-export is used.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
packages/sample-app/sample_app/chats/gemini_chatbot.py(1 hunks)packages/traceloop-sdk/tests/test_associations.py(1 hunks)packages/traceloop-sdk/traceloop/sdk/__init__.py(2 hunks)packages/traceloop-sdk/traceloop/sdk/tracing/associations.py(1 hunks)packages/traceloop-sdk/traceloop/sdk/tracing/tracing.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Store API keys only in environment variables/secure vaults; never hardcode secrets in code
Use Flake8 for code linting and adhere to its rules
Files:
packages/traceloop-sdk/traceloop/sdk/tracing/tracing.pypackages/traceloop-sdk/traceloop/sdk/tracing/associations.pypackages/sample-app/sample_app/chats/gemini_chatbot.pypackages/traceloop-sdk/tests/test_associations.pypackages/traceloop-sdk/traceloop/sdk/__init__.py
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
Repo: traceloop/openllmetry PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-08-17T15:06:48.109Z
Learning: For debugging OpenTelemetry spans, use ConsoleSpanExporter with Traceloop to print spans to console
🧬 Code graph analysis (3)
packages/sample-app/sample_app/chats/gemini_chatbot.py (4)
packages/traceloop-sdk/traceloop/sdk/__init__.py (3)
Traceloop(38-272)init(50-200)get(256-272)packages/traceloop-sdk/traceloop/sdk/tracing/associations.py (1)
AssociationProperty(7-14)packages/traceloop-sdk/traceloop/sdk/client/client.py (1)
Client(12-67)packages/traceloop-sdk/traceloop/sdk/prompts/model.py (1)
Tool(47-49)
packages/traceloop-sdk/tests/test_associations.py (3)
packages/traceloop-sdk/traceloop/sdk/__init__.py (1)
Traceloop(38-272)packages/traceloop-sdk/traceloop/sdk/tracing/associations.py (1)
AssociationProperty(7-14)packages/traceloop-sdk/traceloop/sdk/utils/in_memory_span_exporter.py (1)
get_finished_spans(40-43)
packages/traceloop-sdk/traceloop/sdk/__init__.py (1)
packages/traceloop-sdk/traceloop/sdk/tracing/associations.py (1)
AssociationProperty(7-14)
🪛 Flake8 (7.3.0)
packages/traceloop-sdk/traceloop/sdk/__init__.py
[error] 33-33: 'traceloop.sdk.tracing.associations.AssociationProperty' imported but unused
(F401)
🪛 Ruff (0.14.7)
packages/sample-app/sample_app/chats/gemini_chatbot.py
220-220: Do not catch blind exception: Exception
(BLE001)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Test Packages (3.11)
- GitHub Check: Test Packages (3.12)
- GitHub Check: Build Packages (3.11)
- GitHub Check: Test Packages (3.10)
- GitHub Check: Lint
🔇 Additional comments (12)
packages/sample-app/sample_app/chats/gemini_chatbot.py (2)
1-14: LGTM on initialization and API key handling.The API key is correctly retrieved from an environment variable (
os.environ.get("GENAI_API_KEY")), which aligns with the coding guidelines for secure secret handling. Traceloop initialization is properly placed at module level.
123-128: Good use of the new associations API.The
process_messagefunction correctly uses the newTraceloop.associations.set()API to associate theconversation_idwith the current trace context.packages/traceloop-sdk/traceloop/sdk/tracing/tracing.py (1)
316-320: LGTM on associations handling in span processor.The implementation correctly retrieves associations from context, validates it's a dictionary, and sets each key-value pair as span attributes. The
str(value)coercion ensures type safety for span attributes.Note: The associations are set as top-level attributes (e.g.,
conversation_id) whereas the existingassociation_propertiesuses a prefixed format (traceloop.association.properties.{key}). This appears intentional for cleaner attribute names, but verify this distinction is documented.packages/traceloop-sdk/traceloop/sdk/__init__.py (1)
206-207: LGTM on associations namespace exposure.The
associations = associationspattern correctly exposes the module as a class attribute, enabling the cleanTraceloop.associations.set(...)API usage demonstrated in the sample app and tests.packages/traceloop-sdk/tests/test_associations.py (5)
1-4: LGTM on imports.The imports correctly reference the new public API (
Traceloop,AssociationProperty) and existing decorators.
6-29: Good test for single association.The test properly verifies that a single association is propagated to both task and workflow spans. The assertion on span names ensures correct span ordering.
32-65: Good test for multiple associations.Comprehensive verification that all three associations are present on both span types.
68-98: Good test for associations set within workflow.This test validates an important use case where associations are set after the workflow span has started, ensuring the associations still propagate to child spans.
101-123: Test isolation is already properly handled. Theclear_exporterfixture (lines 31-37 in conftest.py) is marked asautouse=Truewith function scope, which means it automatically runs before each test. It both clears the exporter's span collection viaexporter.clear()and resets the OpenTelemetry context by callingattach(Context()). This prevents any state leakage between tests.packages/traceloop-sdk/traceloop/sdk/tracing/associations.py (3)
1-14: LGTM on enum definition.The
AssociationPropertyenum correctly inherits fromstrfor convenient string usage in span attributes. The standard properties cover common tracing use cases.
21-53: Function correctly implements dual storage pattern.The implementation properly:
- Stores associations in context for propagation to child spans via the span processor
- Sets attributes directly on the current span (if recording) for immediate association
One consideration: The function name
setshadows Python's builtinsettype within importing modules. Since it's accessed viaTraceloop.associations.set(), this is acceptable.
17-18: No action needed — Python 3.10+ is the minimum requirement and fully supports thetuple[...]syntax.The SDK requires Python 3.10 or higher, which natively supports PEP 585 syntax (
tuple[...]). Usingtyping.Tupleis unnecessary and would be a step backward.Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed 36dc637 in 1 minute and 22 seconds. Click for details.
- Reviewed
239lines of code in5files - Skipped
0files when reviewing. - Skipped posting
7draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. packages/sample-app/sample_app/chats/gemini_chatbot.py:10
- Draft comment:
Good assignment of the Traceloop client to 'traceloop'. Consider adding a check to ensure that Traceloop.init() doesn't return None before using its associations. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
2. packages/sample-app/sample_app/chats/gemini_chatbot.py:128
- Draft comment:
Using the client instance's associations.set() is consistent with the new design. Ensure that associations are correctly propagated to the spans. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
3. packages/traceloop-sdk/tests/test_associations.py:60
- Draft comment:
Switching to client.associations.set() throughout tests aligns with the updated SDK design. The removal of THREAD_ID in tests now matches the updated AssociationProperty enum. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
4. packages/traceloop-sdk/tests/test_associations.py:121
- Draft comment:
Replacing THREAD_ID with USER_ID in associations (within workflow) is a breaking change; ensure this change is clearly documented for downstream users. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%The comment is asking the PR author to ensure that a change is documented for downstream users. This falls under the rule of not asking the author to ensure behavior is intended or to update documentation. The comment does not provide a specific code suggestion or ask for a test to be written.
5. packages/traceloop-sdk/traceloop/sdk/__init__.py:37
- Draft comment:
Defining all clarifies the public API. Note that removing the associations namespace from the Traceloop class directs users to access associations via the Client instance. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
6. packages/traceloop-sdk/traceloop/sdk/client/client.py:72
- Draft comment:
Attaching the associations module to the Client instance (self.associations) is clear. Consider using a more specific type hint than ModuleType if a defined interface is available. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
7. packages/traceloop-sdk/traceloop/sdk/tracing/associations.py:14
- Draft comment:
Removal of the THREAD_ID property from AssociationProperty is a breaking change. Ensure that documentation and downstream integrations are updated accordingly. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =40%<= threshold50%The comment is asking the PR author to ensure documentation and downstream integrations are updated, which is similar to asking them to ensure the change is tested or verified. This violates the rule against asking the author to ensure certain actions are taken. However, it does point out a breaking change, which is useful information. The comment could be rephrased to focus on confirming the intention of the breaking change rather than ensuring updates.
Workflow ID: wflow_kXUxaHfyCIadrN1s
You can customize
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (5)
packages/traceloop-sdk/tests/test_associations.py (1)
48-171: Avoid relying on span ordering; select spans by name insteadAll four tests assume:
- Exactly two spans are exported in a specific order (task first, then workflow), and
- Index into
spans[0]/spans[1]based on that order.If the processor/exporter ever changes how spans are flushed, or additional spans are emitted in the same test context, these assertions can start failing for ordering reasons even when associations are correct.
To make the tests more robust, consider selecting spans by name:
- spans = exporter.get_finished_spans() - assert [span.name for span in spans] == [ - "test_single_task.task", - "test_single_association.workflow", - ] - - task_span = spans[0] - workflow_span = spans[1] + spans = exporter.get_finished_spans() + names = [span.name for span in spans] + assert "test_single_task.task" in names + assert "test_single_association.workflow" in names + + spans_by_name = {span.name: span for span in spans} + task_span = spans_by_name["test_single_task.task"] + workflow_span = spans_by_name["test_single_association.workflow"]and apply the same pattern in the other tests.
packages/traceloop-sdk/traceloop/sdk/__init__.py (1)
32-38: Optional: sort__all__to align with Ruff/style expectationsRuff flags
__all__here as unsorted (RUF022). Not a functional issue, but if you want to keep the repo’s style/linting fully green, you can reorder it, e.g.:-__all__ = ["Traceloop", "Client", "Instruments", "associations", "AssociationProperty"] +__all__ = ["AssociationProperty", "Client", "Instruments", "Traceloop", "associations"]packages/traceloop-sdk/traceloop/sdk/tracing/associations.py (1)
7-52: Align type alias and docs with the actual API (and consider avoidingsetbuiltin shadowing)A few small inconsistencies/nits in this module:
Type alias vs implementation
Associationis defined astuple[AssociationProperty, str], butset()accepts any key type and handles non‑AssociationPropertyvalues in theelsebranch.- Either:
- Widen the alias to reflect accepted keys (e.g.
Union[AssociationProperty, str]), or- Drop the
elsebranch and enforceAssociationPropertykeys only.That keeps type checkers and runtime behavior aligned.
Docstring example doesn’t match the public surface
- The example uses
Traceloop.associations.set(...), but the API exposed in this PR is:
- module-level
associationsviafrom traceloop.sdk import associations, andclient.associationsonClient.- Consider updating the example to something like:
from traceloop.sdk import associations, AssociationProperty associations.set([(AssociationProperty.CONVERSATION_ID, "conv-123")])Name
setshadows the builtin
- Within this module
setnow refers to the association helper rather than the builtin set type. It’s not currently causing issues (you don’t callset()inside), but it may be mildly confusing.- If you’re open to a rename before this becomes widely used, something like
set_associationswould avoid the shadowing.None of these are blockers, but tightening them up would make the API clearer and easier to use correctly.
packages/sample-app/sample_app/chats/gemini_chatbot.py (2)
29-33: Fix UTC/timezone inconsistency inget_current_timeThe comment says “just return current UTC time”, but the implementation uses
datetime.now(), which is local time, while the string labels it as({timezone}).To avoid confusion, either:
- Actually return UTC:
-from datetime import datetime +from datetime import datetime, timezone @@ -def get_current_time(timezone: str = "UTC") -> str: - """Get the current time in a specific timezone.""" - # Simplified - just return current UTC time - return f"Current time ({timezone}): {datetime.now().strftime('%Y-%m-%d %H:%M:%S')}" +def get_current_time(timezone: str = "UTC") -> str: + """Get the current time in a specific timezone.""" + # Simplified - just return current UTC time + utc_now = datetime.now(timezone.utc) + return f"Current time ({timezone}): {utc_now.strftime('%Y-%m-%d %H:%M:%S')} UTC"
- Or adjust the comment/label to clearly state it’s using local time.
215-223: Consider narrowing the broadexcept ExceptioninmainFor an interactive sample, catching
Exceptionand printing a friendly message is understandable, but it also hides useful details (and may mask programming errors).If you want to keep the UX but reduce risk:
- Catch expected runtime errors explicitly (e.g., API errors, misconfiguration), and
- Let unexpected exceptions surface (or at least re‑raise after logging).
For example:
from google.genai import errors as genai_errors try: assistant_message, conversation_history = process_message( chat_id, user_message, conversation_history ) except (genai_errors.APIError, RuntimeError) as e: print(f"\nError: {e}") print("Please try again.")This keeps the sample friendly while avoiding swallowing unrelated bugs.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
packages/sample-app/sample_app/chats/gemini_chatbot.py(1 hunks)packages/traceloop-sdk/tests/test_associations.py(1 hunks)packages/traceloop-sdk/traceloop/sdk/__init__.py(1 hunks)packages/traceloop-sdk/traceloop/sdk/client/client.py(3 hunks)packages/traceloop-sdk/traceloop/sdk/tracing/associations.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Store API keys only in environment variables/secure vaults; never hardcode secrets in code
Use Flake8 for code linting and adhere to its rules
Files:
packages/traceloop-sdk/tests/test_associations.pypackages/traceloop-sdk/traceloop/sdk/client/client.pypackages/traceloop-sdk/traceloop/sdk/tracing/associations.pypackages/sample-app/sample_app/chats/gemini_chatbot.pypackages/traceloop-sdk/traceloop/sdk/__init__.py
🧠 Learnings (1)
📓 Common learnings
Learnt from: nina-kollman
Repo: traceloop/openllmetry PR: 3219
File: packages/traceloop-sdk/tests/datasets/mock_objects.py:105-106
Timestamp: 2025-08-04T15:23:44.799Z
Learning: In the traceloop-sdk project, directly accessing private attributes like _client is acceptable in test mock objects and utilities, even though it breaks encapsulation principles, because it's isolated to test code and not production code.
🧬 Code graph analysis (2)
packages/traceloop-sdk/tests/test_associations.py (3)
packages/traceloop-sdk/traceloop/sdk/__init__.py (2)
Traceloop(40-271)init(52-202)packages/traceloop-sdk/traceloop/sdk/tracing/associations.py (1)
AssociationProperty(7-13)packages/traceloop-sdk/traceloop/sdk/utils/in_memory_span_exporter.py (4)
export(45-51)InMemorySpanExporter(22-61)clear(35-38)get_finished_spans(40-43)
packages/traceloop-sdk/traceloop/sdk/__init__.py (2)
packages/traceloop-sdk/traceloop/sdk/tracing/associations.py (1)
AssociationProperty(7-13)packages/traceloop-sdk/traceloop/sdk/client/client.py (1)
Client(15-72)
🪛 Flake8 (7.3.0)
packages/traceloop-sdk/traceloop/sdk/client/client.py
[error] 3-3: 'typing.TYPE_CHECKING' imported but unused
(F401)
🪛 Ruff (0.14.7)
packages/sample-app/sample_app/chats/gemini_chatbot.py
220-220: Do not catch blind exception: Exception
(BLE001)
packages/traceloop-sdk/traceloop/sdk/__init__.py
37-37: __all__ is not sorted
Apply an isort-style sorting to __all__
(RUF022)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Build Packages (3.11)
- GitHub Check: Lint
- GitHub Check: Test Packages (3.11)
- GitHub Check: Test Packages (3.10)
- GitHub Check: Test Packages (3.12)
| else: | ||
| current_associations[str(prop)] = value |
There was a problem hiding this comment.
Caution
Changes requested ❌
Reviewed c0b2818 in 1 minute and 53 seconds. Click for details.
- Reviewed
171lines of code in4files - Skipped
0files when reviewing. - Skipped posting
2draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. packages/traceloop-sdk/traceloop/sdk/__init__.py:32
- Draft comment:
Refactored associations import and all export looks correct. Ensure that documentation and client usage are updated accordingly. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
2. packages/traceloop-sdk/traceloop/sdk/client/client.py:69
- Draft comment:
Instantiating Associations() here is acceptable; however, if Associations only provides static methods consider exposing it as a class-level reference to avoid redundant instantiation. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
Workflow ID: wflow_n0KZYDF5RloKkr0O
You can customize
packages/traceloop-sdk/traceloop/sdk/associations/associations.py
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/traceloop-sdk/traceloop/sdk/client/client.py (1)
9-70: Associations integration onClientlooks goodThe new
associationsattribute is wired correctly and exposes a clearclient.associations.set(...)entry point. SinceAssociationsonly has static behavior today, you could also expose the class directly (self.associations = Associations) if you prefer to avoid constructing an instance, but the current approach is perfectly fine and future‑proof if you ever add instance state.packages/traceloop-sdk/traceloop/sdk/associations/associations.py (1)
7-54: Refine context key usage, dict handling, and typing for associationsThe implementation matches the desired behavior, but a few small refinements would make it more robust and self‑consistent:
Use a namespaced context key instead of raw
"associations"Using a plain string risks collisions with other code using OpenTelemetry context. Consider defining a dedicated context key:
- from typing import Sequence
- from opentelemetry import trace
- from opentelemetry.context import attach, set_value, get_value
- from typing import Sequence, Union
- from opentelemetry import trace
- from opentelemetry.context import attach, set_value, get_value, create_key
+ASSOCIATIONS_KEY = create_key("traceloop.associations")
2. **Avoid mutating the associations dict from context in place** Right now `current_associations = get_value("associations") or {}` and then updating it mutates the same dict that may already be stored in context. Copying first keeps contexts logically immutable: ```diff - # Store all associations in context - current_associations = get_value("associations") or {} + # Store all associations in context + current_associations = dict(get_value(ASSOCIATIONS_KEY) or {}) for prop, value in associations: if isinstance(prop, AssociationProperty): current_associations[prop.value] = value - attach(set_value("associations", current_associations)) + attach(set_value(ASSOCIATIONS_KEY, current_associations))
Align the
Associationtype alias with the runtime behaviorThe implementation explicitly supports non‑enum properties via
else str(prop), but the alias restricts toAssociationProperty. To keep typing and behavior in sync, either drop theelsebranch or broaden the type:-# Type alias for a single association
-Association = tuple[AssociationProperty, str]
+# Type alias for a single association
+Association = tuple[Union[AssociationProperty, str], str]If you intend to *only* support `AssociationProperty`, you can simplify by removing the `else` path in the span loop instead: ```diff - if span and span.is_recording(): - for prop, value in associations: - prop_name = prop.value if isinstance(prop, AssociationProperty) else str(prop) - span.set_attribute(prop_name, value) + if span and span.is_recording(): + for prop, value in associations: + # prop is always an AssociationProperty if you keep the stricter type alias + span.set_attribute(prop.value, value)These changes are not strictly required for correctness in the current code, but they tighten the API and reduce chances of subtle context or typing surprises as the SDK evolves.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
packages/traceloop-sdk/traceloop/sdk/__init__.py(1 hunks)packages/traceloop-sdk/traceloop/sdk/associations/associations.py(1 hunks)packages/traceloop-sdk/traceloop/sdk/client/client.py(3 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Store API keys only in environment variables/secure vaults; never hardcode secrets in code
Use Flake8 for code linting and adhere to its rules
Files:
packages/traceloop-sdk/traceloop/sdk/client/client.pypackages/traceloop-sdk/traceloop/sdk/associations/associations.pypackages/traceloop-sdk/traceloop/sdk/__init__.py
🧬 Code graph analysis (2)
packages/traceloop-sdk/traceloop/sdk/client/client.py (1)
packages/traceloop-sdk/traceloop/sdk/associations/associations.py (1)
Associations(20-54)
packages/traceloop-sdk/traceloop/sdk/__init__.py (2)
packages/traceloop-sdk/traceloop/sdk/associations/associations.py (2)
Associations(20-54)AssociationProperty(7-13)packages/traceloop-sdk/traceloop/sdk/client/client.py (1)
Client(13-70)
🪛 Ruff (0.14.7)
packages/traceloop-sdk/traceloop/sdk/__init__.py
36-36: __all__ is not sorted
Apply an isort-style sorting to __all__
(RUF022)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Test Packages (3.11)
- GitHub Check: Test Packages (3.10)
- GitHub Check: Test Packages (3.12)
- GitHub Check: Lint
- GitHub Check: Build Packages (3.11)
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed 0e8ac3e in 1 minute and 44 seconds. Click for details.
- Reviewed
44lines of code in3files - Skipped
0files when reviewing. - Skipped posting
3draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. packages/sample-app/sample_app/chats/gemini_chatbot.py:7
- Draft comment:
Ensure 'traceloop.associations.set' is valid post-refactor; if 'associations' is no longer an instance property, use Traceloop.set_association_properties instead. - Reason this comment was not posted:
Comment was on unchanged code.
2. packages/traceloop-sdk/traceloop/sdk/__init__.py:29
- Draft comment:
Removing AssociationProperty from top-level exports is breaking; update docs and inform users of new import path. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =20%<= threshold50%The comment is suggesting a documentation update and informing users about a breaking change. It doesn't ask for confirmation or testing, but it does suggest an action related to documentation, which is not allowed by the rules.
3. packages/traceloop-sdk/traceloop/sdk/associations/__init__.py:1
- Draft comment:
Consider adding a module-level docstring to explain the purpose and usage of the associations re-exports. - Reason this comment was not posted:
Confidence changes required:50%<= threshold50%None
Workflow ID: wflow_bmOrcnwGmSvCaXDf
You can customize
There was a problem hiding this comment.
Caution
Changes requested ❌
Reviewed 23d088f in 1 minute and 44 seconds. Click for details.
- Reviewed
16lines of code in1files - Skipped
0files when reviewing. - Skipped posting
0draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
Workflow ID: wflow_emu8Y314x6VTuzFs
You can customize
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
packages/sample-app/sample_app/chats/gemini_chatbot.py (1)
6-14: Avoid depending onTraceloop.initreturning a client and validateGENAI_API_KEYupfront
Traceloop.init(...)returnsOptional[Client]and will returnNonewhen tracing is disabled or misconfigured (e.g., missingTRACELOOP_API_KEY). In that casetraceloop.associations.set(...)on Line 129 will raiseAttributeError, and the genericexcept Exceptioninmain()will turn every request into a vague error.Separately,
genai.Client(api_key=os.environ.get("GENAI_API_KEY"))will passNoneif the env var is missing, which leads to confusing failures later instead of a clear configuration error.You can make the sample more robust and decouple it from the optional return value of
Traceloop.initby:
- Calling
Traceloop.initfor side effects only (not storing its return), and- Using the public
associationsAPI directly, and- Validating
GENAI_API_KEYbefore constructing the Gemini client.-import os -import uuid -from datetime import datetime -import google.genai as genai -from google.genai import types -from traceloop.sdk import Traceloop -from traceloop.sdk.associations import AssociationProperty -from traceloop.sdk.decorators import workflow +import os +import uuid +from datetime import datetime +import google.genai as genai +from google.genai import types +from traceloop.sdk import Traceloop, associations +from traceloop.sdk.associations import AssociationProperty +from traceloop.sdk.decorators import workflow @@ -# Initialize Traceloop for observability -traceloop = Traceloop.init(app_name="gemini_chatbot") - -# Initialize Gemini client -client = genai.Client(api_key=os.environ.get("GENAI_API_KEY")) +# Initialize Traceloop for observability (may no-op if tracing is disabled) +Traceloop.init(app_name="gemini_chatbot") + +# Initialize Gemini client +genai_api_key = os.environ.get("GENAI_API_KEY") +if not genai_api_key: + raise RuntimeError( + "GENAI_API_KEY environment variable is required to run gemini_chatbot." + ) +client = genai.Client(api_key=genai_api_key) @@ - # Set a conversation_id to identify the conversation using the associations API - traceloop.associations.set([(AssociationProperty.CONVERSATION_ID, conversation_id)]) + # Set a conversation_id to identify the conversation using the associations API + associations.set([(AssociationProperty.CONVERSATION_ID, conversation_id)])This way, missing tracing config only disables observability, while a missing Gemini key fails fast with a clear message.
Also applies to: 124-129
🧹 Nitpick comments (3)
packages/traceloop-sdk/traceloop/sdk/associations/__init__.py (1)
1-7: Sort__all__to satisfy Ruff RUF022Ruff flags the export list as unsorted. Alphabetizing it keeps things tidy and clears the warning.
-__all__ = ["Associations", "AssociationProperty", "Association"] +__all__ = ["Association", "AssociationProperty", "Associations"]packages/sample-app/sample_app/chats/gemini_chatbot.py (2)
30-33:get_current_timeclaims UTC but uses local time and ignores thetimezoneargumentThe docstring and comment say “current time in a specific timezone” / “current UTC time”, but
datetime.now()returns local time and thetimezoneparameter is unused. For a sample, at least returning real UTC avoids misleading output:def get_current_time(timezone: str = "UTC") -> str: """Get the current time in a specific timezone.""" - # Simplified - just return current UTC time - return f"Current time ({timezone}): {datetime.now().strftime('%Y-%m-%d %H:%M:%S')}" + # Simplified - just return current UTC time + return f"Current time ({timezone}): {datetime.utcnow().strftime('%Y-%m-%d %H:%M:%S')}"(If you actually want timezone-aware behavior, consider wiring the
timezoneparameter through to a real tz implementation later.)
215-223: Broadexcept Exceptionin CLI loop hides root causes (Ruff BLE001)Catching bare
Exceptionhere can mask programming errors and makes debugging harder, which Ruff flags as BLE001. For this sample app, either narrow the exception types you expect (e.g., config/runtime errors) or explicitly mark the catch as intentional:- # Process the message - try: - assistant_message, conversation_history = process_message( - chat_id, user_message, conversation_history - ) - print(f"\nAssistant: {assistant_message}") - except Exception as e: - print(f"\nError: {e}") - print("Please try again.") + # Process the message + try: + assistant_message, conversation_history = process_message( + chat_id, user_message, conversation_history + ) + print(f"\nAssistant: {assistant_message}") + except Exception as e: # noqa: BLE001 + print(f"\nError: {e}") + print("Please try again.")If you know the concrete exception types from
process_message/google.genai, prefer listing those instead ofException.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
packages/sample-app/sample_app/chats/gemini_chatbot.py(1 hunks)packages/traceloop-sdk/traceloop/sdk/associations/__init__.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Store API keys only in environment variables/secure vaults; never hardcode secrets in code
Use Flake8 for code linting and adhere to its rules
Files:
packages/sample-app/sample_app/chats/gemini_chatbot.pypackages/traceloop-sdk/traceloop/sdk/associations/__init__.py
🧬 Code graph analysis (2)
packages/sample-app/sample_app/chats/gemini_chatbot.py (4)
packages/traceloop-sdk/traceloop/sdk/__init__.py (3)
Traceloop(36-267)init(48-198)get(251-267)packages/traceloop-sdk/traceloop/sdk/associations/associations.py (1)
AssociationProperty(7-13)packages/traceloop-sdk/traceloop/sdk/client/client.py (1)
Client(13-70)packages/traceloop-sdk/traceloop/sdk/prompts/model.py (1)
Tool(47-49)
packages/traceloop-sdk/traceloop/sdk/associations/__init__.py (1)
packages/traceloop-sdk/traceloop/sdk/associations/associations.py (2)
Associations(20-54)AssociationProperty(7-13)
🪛 Ruff (0.14.7)
packages/sample-app/sample_app/chats/gemini_chatbot.py
221-221: Do not catch blind exception: Exception
(BLE001)
packages/traceloop-sdk/traceloop/sdk/associations/__init__.py
7-7: __all__ is not sorted
Apply an isort-style sorting to __all__
(RUF022)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Test Packages (3.12)
- GitHub Check: Lint
- GitHub Check: Test Packages (3.10)
- GitHub Check: Build Packages (3.11)
- GitHub Check: Test Packages (3.11)
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed 645269c in 2 minutes and 9 seconds. Click for details.
- Reviewed
41lines of code in2files - Skipped
0files when reviewing. - Skipped posting
4draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. packages/traceloop-sdk/traceloop/sdk/__init__.py:29
- Draft comment:
Imported Associations and AssociationProperty to expose the new associations API. Ensure that any related documentation and tests are updated accordingly. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%The comment is purely informative and asks the PR author to ensure that documentation and tests are updated, which violates the rules. It doesn't provide a specific suggestion or point out a specific issue with the code.
2. packages/traceloop-sdk/traceloop/sdk/__init__.py:36
- Draft comment:
Updated all to include Associations and AssociationProperty. This explicit export is good practice for a clear public API. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%This comment is purely informative, as it praises the update without providing any actionable feedback or suggestions. It doesn't align with the rules since it doesn't offer a specific code suggestion or ask for a test to be written.
3. packages/traceloop-sdk/traceloop/sdk/associations/associations.py:42
- Draft comment:
Added explicit type annotation for 'current_associations'. Removing the isinstance check assumes all association keys are of type AssociationProperty. Ensure that all callers comply with the defined Association type alias to avoid runtime issues. - Reason this comment was not posted:
Comment looked like it was already resolved.
4. packages/traceloop-sdk/traceloop/sdk/associations/associations.py:52
- Draft comment:
Simplified span attribute setting by directly using prop.value. This assumes that all keys are AssociationProperty instances. If backward compatibility with raw strings is needed, consider retaining type checking. - Reason this comment was not posted:
Comment looked like it was already resolved.
Workflow ID: wflow_1dhUDIqgWpsnO6Ke
You can customize
| ]) | ||
| """ | ||
| # Store all associations in context | ||
| current_associations: dict[str, str] = get_value("associations") or {} # type: ignore |
There was a problem hiding this comment.
Make associations a const in this file
You don't need to import in tracing.py to save from circular imports
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed 505c403 in 2 minutes and 11 seconds. Click for details.
- Reviewed
16lines of code in1files - Skipped
0files when reviewing. - Skipped posting
2draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. packages/traceloop-sdk/traceloop/sdk/__init__.py:29
- Draft comment:
Removing the import for Associations and AssociationProperty can break backward compatibility if users relied on them. Please ensure their deprecation is documented or provide alternative access. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 15% vs. threshold = 50% The comment is asking the PR author to "ensure deprecation is documented" or "provide alternative access" - this is essentially asking them to verify/confirm something rather than pointing out a definite code issue. The rules state "Do NOT ask the PR author to confirm their intention, to explain, to double-check things, to ensure the behavior is intended" and "Do NOT make speculative comments, such as 'If X, then Y is an issue'". This comment is speculative because it assumes users relied on these imports without evidence, and it's asking the author to ensure/verify rather than pointing to a concrete problem. The comment doesn't show that there's definitely a problem - it's a "can break" not "will break" statement. However, removing public API exports is a legitimate backward compatibility concern in library development. If these classes were previously exported in__all__, they were part of the public API. This could be a valid architectural concern rather than just speculation. While backward compatibility is important, the comment violates the rule about asking authors to "ensure" or verify things. It's phrased as a request for the author to check/document rather than pointing out a definite issue. Without evidence that these classes are actually used by external consumers, this is speculative. The comment should either show concrete evidence of usage or not be made at all. This comment should be deleted. It asks the PR author to "ensure" documentation and "provide alternative access" which violates the rules against asking authors to verify or confirm things. The comment is speculative ("can break") without showing concrete evidence of the issue.
2. packages/traceloop-sdk/traceloop/sdk/__init__.py:31
- Draft comment:
The all list was updated by removing Associations and AssociationProperty. Confirm this change is intentional and update documentation if it introduces breaking changes. - Reason this comment was not posted:
Comment looked like it was already resolved.
Workflow ID: wflow_p6UyWaxZLNWLhAX6
You can customize
| from traceloop.sdk.client.client import Client | ||
|
|
||
|
|
||
|
|
| class AssociationProperty(str, Enum): | ||
| """Standard association properties for tracing.""" | ||
|
|
||
| CONVERSATION_ID = "conversation_id" |
There was a problem hiding this comment.
whats the differnece between session_id and conversation_id
will we aggregate by both, when to use each
There was a problem hiding this comment.
conversation is a type of session. I do need to recognize that this is a conversation because it has different characteristics then other sessions. Such us first/last messages.
The term session as a stand alone is not yet determined
feat(instrumentation): ...orfix(instrumentation): ....Important
Introduces trace association management in Traceloop SDK and a Gemini-based chatbot with integrated tools.
AssociationPropertyenum andAssociationsclass inassociations.pyfor managing trace associations.gemini_chatbot.pywith tools for weather, time, and knowledge lookups.Associations.set()method allows setting associations on spans in the current context.traceloop.associations.set()to associate conversation IDs with traces.test_associations.pyto validate single and multiple association creation and propagation.Clientclass inclient.pyto includeassociationsattribute.This description was created by
for 505c403. You can customize this summary. It will automatically update as commits are pushed.
Summary by CodeRabbit
New Features
Bug Fixes / Improvements
Tests
✏️ Tip: You can customize this high-level summary in your review settings.