-
Notifications
You must be signed in to change notification settings - Fork 860
fix(openai-agents): apply content tracing flag to content #3487
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -33,6 +33,7 @@ npm-debug.log | |
| yarn-error.log | ||
| testem.log | ||
| /typings | ||
| .tool-versions | ||
|
|
||
| # System Files | ||
| .DS_Store | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -174,10 +174,18 @@ def run_async(method): | |
| asyncio.run(method) | ||
|
|
||
|
|
||
| def should_send_prompts(): | ||
| return ( | ||
| os.getenv(TRACELOOP_TRACE_CONTENT) or "true" | ||
| ).lower() == "true" or context_api.get_value("override_enable_content_tracing") | ||
| def _is_truthy(value): | ||
| return str(value).strip().lower() in ("true", "1", "yes", "on") | ||
duanyutong marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
|
|
||
| def should_send_prompts() -> bool: | ||
| """Determine if LLM content tracing should be enabled. | ||
|
|
||
| Content includes not only prompts, but also responses. | ||
| """ | ||
| env_setting = os.getenv(TRACELOOP_TRACE_CONTENT, "true") | ||
| override = context_api.get_value("override_enable_content_tracing") | ||
| return _is_truthy(env_setting) or _is_truthy(override) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same here - we set the override flag so it will always be a boolean
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I see that The |
||
|
|
||
|
|
||
| def should_emit_events() -> bool: | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why did you change this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
context_apiis not typed. This flag isn't documented and users could set it to any value1,True,"true","yes","on"etc. Having a single implementation of_is_truthyreduces surprises and provides better compatibility with a range of possible values. The rationale is the same as that for the environment variable.