-
Notifications
You must be signed in to change notification settings - Fork 1
feat: add optional PostHog instrumentation for agent tracing #1
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
Conversation
Add opt-in PostHog instrumentation that tracks agent workflows, LLM generations, tool calls, and handoffs. Enabled by setting POSTHOG_API_KEY environment variable. - Add instrumentation.py with setup_posthog() using posthog-python SDK - Integrate with OpenAI Agents SDK via posthog.ai.openai_agents.instrument() - Add python-dotenv for .env file loading in CLI - Add posthog as optional dependency group - Document PostHog env vars in .env.example and CLAUDE.md
📝 WalkthroughWalkthroughThe pull request introduces optional PostHog instrumentation to the project. A new instrumentation module provides setup and shutdown functions, runtime and optional dependencies are added, environment variables are loaded at startup, and instrumentation is initialized in key workflows with sensitive data tracing enabled. Changes
Sequence DiagramsequenceDiagram
participant CLI
participant Env as Environment
participant PostHog as PostHog Setup
participant Workflow as Workflow Runner
participant Client as PostHog Client
participant Tracer as Instrumentation Processor
CLI->>Env: load_dotenv()
Env-->>CLI: environment variables loaded
Workflow->>PostHog: setup_posthog(github_username)
PostHog->>Env: check POSTHOG_API_KEY
alt API Key Present
PostHog->>Client: initialize PostHog client
Client-->>PostHog: client ready
PostHog->>Tracer: create instrumentation processor
Tracer-->>PostHog: processor ready
PostHog-->>Workflow: True (enabled)
else API Key Missing
PostHog-->>Workflow: False (disabled)
end
Workflow->>Workflow: execute with tracing (if enabled)
Workflow->>PostHog: shutdown_posthog()
PostHog->>Client: flush and close
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the 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. Comment |
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.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@pyproject.toml`:
- Around line 40-42: The optional dependency entry posthog = ["posthog>=7.5.0",
...] in pyproject.toml conflicts with the version documented in CLAUDE.md (line
91) which requires posthog>=7.6.0; update the pyproject.toml posthog spec to
"posthog>=7.6.0" (or alternatively update CLAUDE.md to 7.5.0) so both sources
reference the same minimum version, and run dependency resolution/tests to
confirm compatibility.
In `@src/github_standup_agent/instrumentation.py`:
- Around line 67-76: shutdown_posthog currently clears _posthog_client but does
not reset the module-level flag _instrumentation_enabled, so is_enabled() can
return True after shutdown; update shutdown_posthog to also set
_instrumentation_enabled = False (referencing the same global as used by
enable/disable functions) inside the function (ensure you declare it global if
needed) so the module state accurately reflects that instrumentation is disabled
after shutdown.
In `@src/github_standup_agent/runner.py`:
- Around line 53-54: The code calls setup_posthog(distinct_id=github_username)
but never calls shutdown_posthog(), so buffered PostHog events may not be
flushed; fix by invoking shutdown_posthog() at the end of each workflow where
setup_posthog() is used (e.g., at the end of the runner functions that
initialize PostHog) or alternatively register atexit handler inside the
instrumentation module (in the setup_posthog implementation) to call
shutdown_posthog() on process exit; locate the setup_posthog and add matching
shutdown_posthog calls or add an atexit.register(shutdown_posthog) in the
instrumentation setup.
🧹 Nitpick comments (1)
CLAUDE.md (1)
82-94: Consider documenting the debug mode.Based on the AI summary,
instrumentation.pysupports aPOSTHOG_DEBUGenvironment variable for debug logging. Consider adding this to the documentation for completeness:- `POSTHOG_API_KEY` - Enables PostHog agent tracing when set - `POSTHOG_HOST` - PostHog host (default: https://us.posthog.com) - `POSTHOG_DISTINCT_ID` - User identifier (defaults to github_username) +- `POSTHOG_DEBUG` - Enable debug logging for PostHog (optional)
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (6)
.env.exampleCLAUDE.mdpyproject.tomlsrc/github_standup_agent/cli.pysrc/github_standup_agent/instrumentation.pysrc/github_standup_agent/runner.py
🔇 Additional comments (8)
pyproject.toml (1)
28-28: LGTM!Adding
python-dotenvas a runtime dependency is appropriate since the CLI relies on it to load environment variables early in the startup process.src/github_standup_agent/cli.py (1)
7-14: Unconventional but intentional import ordering.Calling
load_dotenv()between imports violates PEP 8 ordering but is necessary here to ensure environment variables are available before other modules (likeinstrumentation.py) read them at import time. The explanatory comment helps clarify this intent.Consider adding a
# noqa: E402comment on line 14 if linters flag the import ordering, or suppress at the file level if needed..env.example (1)
11-16: LGTM!Clear documentation of the optional PostHog environment variables with appropriate comments indicating they are opt-in.
src/github_standup_agent/runner.py (2)
14-14: Verify the instrumentation module implementation and PostHog lifecycle management.Ensure the
instrumentation.pymodule:
- Implements idempotent setup (safe to call multiple times on lines 53-54 and 147-148 in runner.py)
- Handles graceful degradation if the
posthogpackage is not installed- Includes proper error handling if PostHog initialization fails
- Provides a
shutdown_posthog()function for cleanup on application exit (currently no shutdown calls detected in runner.py)Additionally, review the
trace_include_sensitive_data=Trueconfiguration on lines 79 and 159 to ensure sensitive user inputs and GitHub data are not inadvertently exposed in PostHog traces.
77-80: Evaluate the security implications oftrace_include_sensitive_data=True.This setting will include LLM inputs/outputs and tool call payloads in traces. Depending on where traces are sent (PostHog, APM, logging systems, etc.), this could expose sensitive data including GitHub usernames, PR/issue content, and user inputs.
Confirm:
- Where traces are being routed (PostHog, APM, etc.)
- Whether this aligns with your privacy policy and data handling requirements
- Whether a separate opt-in environment variable should control this behavior, rather than being enabled by default
src/github_standup_agent/instrumentation.py (3)
1-10: LGTM!Module setup with lazy optional dependency pattern is appropriate. The
Anytype for the optional client and module-level debug flag evaluation are reasonable choices.
79-81: LGTM!Simple and clear accessor function.
46-51: Verify thatprivacy_mode=Falsealigns with data handling requirements.With
privacy_mode=False, LLM inputs and outputs (including user prompts and generated content) will be sent to PostHog. This may include sensitive information depending on usage. Consider:
- Documenting this behavior clearly for users enabling PostHog instrumentation.
- Offering a
POSTHOG_PRIVACY_MODEenvironment variable to let users opt into privacy mode.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| posthog = [ | ||
| "posthog>=7.5.0", # For local dev with openai_agents, install ../posthog-python | ||
| ] |
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.
Version mismatch with documentation.
The optional dependency specifies posthog>=7.5.0, but CLAUDE.md (line 91) documents posthog>=7.6.0. Align the versions to avoid confusion.
🤖 Prompt for AI Agents
In `@pyproject.toml` around lines 40 - 42, The optional dependency entry posthog =
["posthog>=7.5.0", ...] in pyproject.toml conflicts with the version documented
in CLAUDE.md (line 91) which requires posthog>=7.6.0; update the pyproject.toml
posthog spec to "posthog>=7.6.0" (or alternatively update CLAUDE.md to 7.5.0) so
both sources reference the same minimum version, and run dependency
resolution/tests to confirm compatibility.
| def shutdown_posthog() -> None: | ||
| """Flush and shutdown PostHog client.""" | ||
| global _posthog_client | ||
| if _posthog_client: | ||
| try: | ||
| _posthog_client.flush() | ||
| _posthog_client.shutdown() | ||
| except Exception: | ||
| pass | ||
| _posthog_client = None |
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.
Reset _instrumentation_enabled flag on shutdown.
After shutdown_posthog() is called, is_enabled() will still return True because _instrumentation_enabled is never reset. This creates inconsistent state where the module reports instrumentation as enabled when the client has been shut down.
Proposed fix
def shutdown_posthog() -> None:
"""Flush and shutdown PostHog client."""
- global _posthog_client
+ global _posthog_client, _instrumentation_enabled
if _posthog_client:
try:
_posthog_client.flush()
_posthog_client.shutdown()
except Exception:
pass
_posthog_client = None
+ _instrumentation_enabled = False📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def shutdown_posthog() -> None: | |
| """Flush and shutdown PostHog client.""" | |
| global _posthog_client | |
| if _posthog_client: | |
| try: | |
| _posthog_client.flush() | |
| _posthog_client.shutdown() | |
| except Exception: | |
| pass | |
| _posthog_client = None | |
| def shutdown_posthog() -> None: | |
| """Flush and shutdown PostHog client.""" | |
| global _posthog_client, _instrumentation_enabled | |
| if _posthog_client: | |
| try: | |
| _posthog_client.flush() | |
| _posthog_client.shutdown() | |
| except Exception: | |
| pass | |
| _posthog_client = None | |
| _instrumentation_enabled = False |
🤖 Prompt for AI Agents
In `@src/github_standup_agent/instrumentation.py` around lines 67 - 76,
shutdown_posthog currently clears _posthog_client but does not reset the
module-level flag _instrumentation_enabled, so is_enabled() can return True
after shutdown; update shutdown_posthog to also set _instrumentation_enabled =
False (referencing the same global as used by enable/disable functions) inside
the function (ensure you declare it global if needed) so the module state
accurately reflects that instrumentation is disabled after shutdown.
| # Initialize PostHog instrumentation (if configured) | ||
| setup_posthog(distinct_id=github_username) |
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.
Missing cleanup/shutdown for PostHog.
setup_posthog() is called in both workflows, but there's no corresponding shutdown_posthog() call. According to the AI summary, the instrumentation module provides this function. Without proper shutdown, buffered events may not be flushed before the process exits.
Consider adding cleanup at the end of each workflow or registering an atexit handler in the instrumentation module.
Also applies to: 147-148
🤖 Prompt for AI Agents
In `@src/github_standup_agent/runner.py` around lines 53 - 54, The code calls
setup_posthog(distinct_id=github_username) but never calls shutdown_posthog(),
so buffered PostHog events may not be flushed; fix by invoking
shutdown_posthog() at the end of each workflow where setup_posthog() is used
(e.g., at the end of the runner functions that initialize PostHog) or
alternatively register atexit handler inside the instrumentation module (in the
setup_posthog implementation) to call shutdown_posthog() on process exit; locate
the setup_posthog and add matching shutdown_posthog calls or add an
atexit.register(shutdown_posthog) in the instrumentation setup.
Summary
POSTHOG_API_KEYis setposthog.ai.openai_agents.instrument()for automatic tracingpython-dotenvfor.envfile loading in CLIWhat's Tracked
When enabled, PostHog captures:
$ai_trace- Full agent workflow traces$ai_generation- LLM API calls with model, tokens, input/output$ai_span- Agent execution, tool calls, and handoffsConfiguration
Test plan
make check(lint, type-check, tests pass)Summary by CodeRabbit
New Features
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.