-
Notifications
You must be signed in to change notification settings - Fork 2.2k
Feature: Make trace_include_sensitive_data configurable via environment variable #1192
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?
Feature: Make trace_include_sensitive_data configurable via environment variable #1192
Conversation
This PR is stale because it has been open for 10 days with no activity. |
Thank you for the PR @Kunmeer-SyedMohamedHyder , any blocking points before approving it @seratch? |
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.
Pull Request Overview
This PR adds environment variable configuration for the trace_include_sensitive_data
field in RunConfig
. Previously, this field was hardcoded to True
, but now it can be controlled via the OPENAI_AGENTS_TRACE_INCLUDE_SENSITIVE_DATA
environment variable while maintaining backward compatibility.
- Introduces a new helper function
_default_trace_include_sensitive_data()
to parse the environment variable - Changes
trace_include_sensitive_data
from a simple default value to usefield(default_factory=...)
- Adds comprehensive test coverage for various boolean string formats and precedence behavior
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
File | Description |
---|---|
src/agents/run.py | Implements environment variable parsing logic and updates RunConfig field definition |
tests/test_run_config.py | Adds test cases for environment variable parsing, default behavior, and explicit override precedence |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
def _default_trace_include_sensitive_data() -> bool: | ||
"""Returns the default value for trace_include_sensitive_data based on environment variable.""" | ||
val = os.getenv("OPENAI_AGENTS_TRACE_INCLUDE_SENSITIVE_DATA", "true") | ||
return val.strip().lower() in ("1", "true", "yes", "on") |
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.
The boolean parsing logic is incomplete and will incorrectly handle invalid values. For example, setting the environment variable to "invalid" will return False, but it should either raise an error or have documented fallback behavior. Consider adding validation or explicit handling for unrecognized values.
return val.strip().lower() in ("1", "true", "yes", "on") | |
val = os.getenv("OPENAI_AGENTS_TRACE_INCLUDE_SENSITIVE_DATA", "true").strip().lower() | |
truthy = ("1", "true", "yes", "on") | |
falsy = ("0", "false", "no", "off") | |
if val in truthy: | |
return True | |
elif val in falsy: | |
return False | |
else: | |
raise ValueError( | |
f"Invalid value for OPENAI_AGENTS_TRACE_INCLUDE_SENSITIVE_DATA: '{val}'. " | |
f"Expected one of {truthy + falsy}." | |
) |
Copilot uses AI. Check for mistakes.
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.
I think having this enhancement is good; @rm-openai any thoughts?
Description:
This PR adds support for configuring
trace_include_sensitive_data
inRunConfig
using an environment variable:OPENAI_AGENTS_TRACE_INCLUDE_SENSITIVE_DATA
.Previously, this field always defaulted to
True
, and there was no way to control it without explicitly passing it intoRunConfig
. With this change, if the field is not set explicitly, its value will be pulled from the environment. This provides more flexibility, especially in environments where trace sensitivity and data privacy are important.The environment variable supports common boolean values like
true
,false
,1
,0
,yes
, andno
(case-insensitive).Fixes #1183
Behavior Summary:
trace_include_sensitive_data
is passed explicitly, that value is used.OPENAI_AGENTS_TRACE_INCLUDE_SENSITIVE_DATA
environment variable.True
(preserving current behavior).Example usage:
export OPENAI_AGENTS_TRACE_INCLUDE_SENSITIVE_DATA=false