Skip to content

feat(traceloop-sdk): add 'trace_content' flag and relevant test cases. Fixes #137 #3433

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.

Sign up for GitHub

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

feat(traceloop-sdk): add 'trace_content' flag and relevant test cases. Fixes #137 #3433

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8d1f250
feature: add 'trace_content' flag and relevant test cases
visheshdvivedi Nov 2, 2025
136655e
update test case
visheshdvivedi Nov 2, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 27 additions & 0 deletions packages/traceloop-sdk/tests/conftest.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,33 @@ def on_start(self, span, parent_context=None):
TracerWrapper.instance = _trace_wrapper_instance


@pytest.fixture
def exporter_with_trace_content_override():
# Clear singleton if existed
Copy link
Contributor

@ellipsis-dev ellipsis-dev bot Nov 2, 2025

Choose a reason for hiding this comment

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

Initialize _trace_wrapper_instance to None (e.g. before the if-check) so that the teardown code doesn’t raise a NameError if the attribute isn’t present.

Suggested change
# Clear singleton if existed
_trace_wrapper_instance = None

if hasattr(TracerWrapper, "instance"):
_trace_wrapper_instance = TracerWrapper.instance
del TracerWrapper.instance

# Set environment variable to false
os.environ["TRACELOOP_TRACE_CONTENT"] = "false"
Copy link

@coderabbitai coderabbitai bot Nov 2, 2025

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major

Preserve the original environment variable value.

The fixture unconditionally deletes TRACELOOP_TRACE_CONTENT in cleanup, which could raise KeyError if the test deletes it, or could lose a pre-existing value set outside the test.

Apply this diff:

+    _original_trace_content = os.environ.get("TRACELOOP_TRACE_CONTENT")
     # Set environment variable to false
     os.environ["TRACELOOP_TRACE_CONTENT"] = "false"
     
     ...
     
     # Clean up environment
-    del os.environ["TRACELOOP_TRACE_CONTENT"]
+    if _original_trace_content is not None:
+        os.environ["TRACELOOP_TRACE_CONTENT"] = _original_trace_content
+    else:
+        os.environ.pop("TRACELOOP_TRACE_CONTENT", None)

Also applies to: 250-250

🤖 Prompt for AI Agents 
In packages/traceloop-sdk/tests/conftest.py around lines 234 and 250, the
fixture unconditionally deletes TRACELOOP_TRACE_CONTENT in cleanup which can
raise KeyError or clobber an existing external value; capture the original
os.environ.get("TRACELOOP_TRACE_CONTENT") before setting it, and in teardown
restore it (if original is None remove safely using
os.environ.pop("TRACELOOP_TRACE_CONTENT", None) or if original had a value set
it back) so the environment is restored without raising errors or losing
pre-existing values.


# Create exporter with trace_content=True to override env var
exporter = InMemorySpanExporter()
Traceloop.init(
exporter=exporter,
disable_batch=True,
trace_content=True # This should override the env var
)

yield exporter

# Restore singleton if any
if _trace_wrapper_instance:
TracerWrapper.instance = _trace_wrapper_instance
# Clean up environment
del os.environ["TRACELOOP_TRACE_CONTENT"]


@pytest.fixture
def datasets():
"""Create a Datasets instance with HTTP client for VCR recording/playback"""
Expand Down
3 changes: 2 additions & 1 deletion packages/traceloop-sdk/traceloop/sdk/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ def init(
sampler: Optional[Sampler] = None,
traceloop_sync_enabled: bool = False,
should_enrich_metrics: bool = True,
trace_content: bool = True,
Copy link

@coderabbitai coderabbitai bot Nov 2, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Breaking change: default trace_content=True overrides environment variable.

The default value of True breaks backward compatibility. Previously, is_content_tracing_enabled() controlled content tracing based on the TRACELOOP_TRACE_CONTENT environment variable. Now, the OR logic at line 98 means content tracing is always enabled when callers don't pass the parameter, ignoring TRACELOOP_TRACE_CONTENT=false.

Additionally, the OR semantics prevent true programmatic override: trace_content=False cannot disable content tracing if the environment variable is set to "true", contradicting the PR objective of "explicit programmatic control."

To maintain backward compatibility and enable true override, use None as the default:

-        trace_content: bool = True,
+        trace_content: Optional[bool] = None,
         resource_attributes: dict = {},

And update the logic at line 98:

-        enable_content_tracing = trace_content or is_content_tracing_enabled()
+        enable_content_tracing = (
+            trace_content if trace_content is not None else is_content_tracing_enabled()
+        )

This ensures:

  • trace_content=True → force enable (override env var)
  • trace_content=False → force disable (override env var)
  • trace_content=None (default) → defer to env var (backward compatible)

Also applies to: 98-98

🤖 Prompt for AI Agents 
In packages/traceloop-sdk/traceloop/sdk/__init__.py at line 67 and affecting
logic at line 98, the default trace_content=True both breaks backwards
compatibility and prevents programmatic overrides; change the parameter default
from True to None, and update the decision at line 98 to use the explicit value
when trace_content is not None, otherwise fall back to
is_content_tracing_enabled() (i.e., use trace_content if provided, else consult
the env-based helper), so True forces enable, False forces disable, and None
defers to the environment.

resource_attributes: dict = {},
instruments: Optional[Set[Instruments]] = None,
block_instruments: Optional[Set[Instruments]] = None,
Expand Down Expand Up @@ -94,7 +95,7 @@ def init(
print(Fore.YELLOW + "Tracing is disabled" + Fore.RESET)
return

enable_content_tracing = is_content_tracing_enabled()
enable_content_tracing = trace_content or is_content_tracing_enabled()

if exporter or processor:
print(Fore.GREEN + "Traceloop exporting traces to a custom exporter")
Expand Down