feat(cli): add OpenAI Codex OTEL integration support#183
Conversation
Add Codex CLI commands for installing/uninstalling OTEL configuration: - `shelltime codex install` - configures ~/.codex/config.toml with OTEL settings - `shelltime codex uninstall` - removes OTEL configuration Improvements to AICodeOtel processor: - Use substring matching for source detection (supports claude-code variants) - Add clientType field to metrics and events for better tracking - Fix timestamp fallback to observed time for log records Add debug logging for daemon configuration and config file discovery. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Summary of ChangesHello @AnnatarHe, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly expands the CLI's observability capabilities by integrating OpenAI Codex with the existing OpenTelemetry system. It provides users with convenient commands to configure and manage Codex telemetry, enhances the daemon's ability to accurately identify and categorize different AI code tool sources, and adds a new field to telemetry data for more granular client tracking. These changes aim to improve the collection and analysis of usage data from various AI-powered coding assistants. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
Codecov Report❌ Patch coverage is
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 2 files with indirect coverage changes 🚀 New features to boost your workflow:
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces support for OpenAI Codex OTEL integration by adding shelltime codex install/uninstall commands. It also improves the OTEL data processing by using substring matching for source detection and adding a clientType field to telemetry data for better tracking. The changes are well-structured, but there are a few critical and high-severity issues related to error handling that need to be addressed. Specifically, a potential issue from ignoring an error when resolving the user's home directory, and inconsistent file read error handling. I've provided suggestions to fix these issues.
| func NewCodexOtelConfigService() CodexOtelConfigService { | ||
| homeDir, _ := os.UserHomeDir() | ||
| configPath := filepath.Join(homeDir, codexConfigDir, codexConfigFile) | ||
| return &codexOtelConfigService{ | ||
| configPath: configPath, | ||
| } |
There was a problem hiding this comment.
os.UserHomeDir() can return an error if the user's home directory cannot be determined. This error is currently being ignored. If an error occurs, homeDir will be an empty string, and configPath will resolve to a path relative to the current working directory (e.g., ./.codex/config.toml). This could lead to unexpected behavior and potential file permission issues. The error should be handled, and NewCodexOtelConfigService should return an error to the caller.
func NewCodexOtelConfigService() (CodexOtelConfigService, error) {
homeDir, err := os.UserHomeDir()
if err != nil {
return nil, fmt.Errorf("could not get user home directory: %w", err)
}
configPath := filepath.Join(homeDir, codexConfigDir, codexConfigFile)
return &codexOtelConfigService{
configPath: configPath,
}, nilReferences
- For platform-independent paths, use
filepath.Jointo combine segments andos.UserHomeDir()to get the home directory, rather than hardcoding path separators or environment variables like$HOME. The error returned byos.UserHomeDir()must be handled to ensure path correctness.
| func commandCodexInstall(c *cli.Context) error { | ||
| color.Yellow.Println("Installing Codex OTEL configuration...") | ||
|
|
||
| service := model.NewCodexOtelConfigService() |
There was a problem hiding this comment.
Following the recommended change to NewCodexOtelConfigService to return an error, this call needs to be updated to handle the potential error. This ensures that if the service cannot be initialized (e.g., because the home directory is not found), the command fails gracefully instead of proceeding with an invalid configuration.
service, err := model.NewCodexOtelConfigService()
if err != nil {
color.Red.Printf("Failed to initialize Codex OTEL config service: %v\n", err)
return err
}| func commandCodexUninstall(c *cli.Context) error { | ||
| color.Yellow.Println("Removing Codex OTEL configuration...") | ||
|
|
||
| service := model.NewCodexOtelConfigService() |
There was a problem hiding this comment.
Similar to the install command, the error from the updated NewCodexOtelConfigService must be handled here to ensure the command fails gracefully if service initialization fails.
service, err := model.NewCodexOtelConfigService()
if err != nil {
color.Red.Printf("Failed to initialize Codex OTEL config service: %v\n", err)
return err
}| if data, err := os.ReadFile(s.configPath); err == nil && len(data) > 0 { | ||
| if err := toml.Unmarshal(data, &config); err != nil { | ||
| return fmt.Errorf("failed to parse existing config: %w", err) | ||
| } | ||
| } |
There was a problem hiding this comment.
The error handling for os.ReadFile is inconsistent. In Install, an error from os.ReadFile (e.g., a permission error) is silently ignored, and the function proceeds to create a new configuration, potentially overwriting a file it couldn't read. In Uninstall and Check, such an error would correctly cause the function to fail. The Install function should also handle file read errors explicitly, only ignoring os.IsNotExist errors.
data, err := os.ReadFile(s.configPath)
if err != nil && !os.IsNotExist(err) {
return fmt.Errorf("failed to read existing config file: %w", err)
}
if err == nil && len(data) > 0 {
if err := toml.Unmarshal(data, &config); err != nil {
return fmt.Errorf("failed to parse existing config: %w", err)
}
}Add support for all Codex-specific OTEL fields that the server expects: - ConversationID, CallID, EventKind, ToolTokens for session/tool events - AuthMode, Slug, ContextWindow, ApprovalPolicy, SandboxPolicy for config - MCPServers, Profile, ReasoningEnabled for conversation_starts - ToolArguments, ToolOutput, PromptEncrypted for tool_result Also adds field aliases for Codex OTEL format compatibility: - http.response.status_code -> statusCode - user.account_id -> userAccountUuid - error.message -> error 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
1 participant
Summary
shelltime codex install/uninstallcommands for configuring OTEL in~/.codex/config.tomlclientTypefield to metrics and events for tracking the originating CLITest plan
shelltime codex installand verify~/.codex/config.tomlis created with correct OTEL settingsshelltime codex uninstalland verify OTEL config is removed🤖 Generated with Claude Code