mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
Emit analytics for remote plugin installs (#20267)
## Summary - emit `codex_plugin_installed` after a remote plugin install succeeds - keep local installs unchanged, but let remote installs override the analytics `plugin_id` with the backend remote plugin id (`plugins~Plugin_...`) - preserve the local/display identity in `plugin_name` and `marketplace_name`, plus capability metadata from the installed bundle - add regression coverage for local install analytics, remote install analytics, and analytics id override serialization ## Testing - `just fmt` - `cargo test -p codex-analytics` - `cargo test -p codex-app-server`
This commit is contained in:
committed by
GitHub
Unverified
parent
b6f81257f8
commit
acdf908268
@@ -1496,6 +1496,25 @@ fn plugin_management_event_serializes_expected_shape() {
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn plugin_management_event_can_use_remote_plugin_id_override() {
|
||||
let mut plugin = sample_plugin_metadata();
|
||||
plugin.remote_plugin_id = Some("plugins~Plugin_remote".to_string());
|
||||
let event = TrackEventRequest::PluginInstalled(CodexPluginEventRequest {
|
||||
event_type: "codex_plugin_installed",
|
||||
event_params: codex_plugin_metadata(plugin),
|
||||
});
|
||||
|
||||
let payload = serde_json::to_value(&event).expect("serialize plugin installed event");
|
||||
|
||||
assert_eq!(
|
||||
payload["event_params"]["plugin_id"],
|
||||
"plugins~Plugin_remote"
|
||||
);
|
||||
assert_eq!(payload["event_params"]["plugin_name"], "sample");
|
||||
assert_eq!(payload["event_params"]["marketplace_name"], "test");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn hook_run_event_serializes_expected_shape() {
|
||||
let tracking = TrackEventsContext {
|
||||
@@ -2482,6 +2501,7 @@ async fn turn_completed_without_started_notification_emits_null_started_at() {
|
||||
fn sample_plugin_metadata() -> PluginTelemetryMetadata {
|
||||
PluginTelemetryMetadata {
|
||||
plugin_id: PluginId::parse("sample@test").expect("valid plugin id"),
|
||||
remote_plugin_id: None,
|
||||
capability_summary: Some(PluginCapabilitySummary {
|
||||
config_name: "sample@test".to_string(),
|
||||
display_name: "sample".to_string(),
|
||||
|
||||
@@ -587,11 +587,16 @@ pub(crate) fn codex_app_metadata(
|
||||
}
|
||||
|
||||
pub(crate) fn codex_plugin_metadata(plugin: PluginTelemetryMetadata) -> CodexPluginMetadata {
|
||||
let capability_summary = plugin.capability_summary;
|
||||
let PluginTelemetryMetadata {
|
||||
plugin_id,
|
||||
remote_plugin_id,
|
||||
capability_summary,
|
||||
} = plugin;
|
||||
let event_plugin_id = remote_plugin_id.unwrap_or_else(|| plugin_id.as_key());
|
||||
CodexPluginMetadata {
|
||||
plugin_id: Some(plugin.plugin_id.as_key()),
|
||||
plugin_name: Some(plugin.plugin_id.plugin_name),
|
||||
marketplace_name: Some(plugin.plugin_id.marketplace_name),
|
||||
plugin_id: Some(event_plugin_id),
|
||||
plugin_name: Some(plugin_id.plugin_name),
|
||||
marketplace_name: Some(plugin_id.marketplace_name),
|
||||
has_skills: capability_summary
|
||||
.as_ref()
|
||||
.map(|summary| summary.has_skills),
|
||||
|
||||
@@ -284,6 +284,7 @@ use codex_core_plugins::PluginReadRequest;
|
||||
use codex_core_plugins::PluginUninstallError as CorePluginUninstallError;
|
||||
use codex_core_plugins::loader::load_plugin_apps;
|
||||
use codex_core_plugins::loader::load_plugin_mcp_servers;
|
||||
use codex_core_plugins::loader::plugin_telemetry_metadata_from_root;
|
||||
use codex_core_plugins::manifest::PluginManifestInterface;
|
||||
use codex_core_plugins::marketplace::MarketplaceError;
|
||||
use codex_core_plugins::marketplace::MarketplacePluginSource;
|
||||
|
||||
@@ -505,7 +505,7 @@ impl CodexMessageProcessor {
|
||||
async fn remote_plugin_install_response(
|
||||
&self,
|
||||
remote_marketplace_name: String,
|
||||
plugin_name: String,
|
||||
remote_plugin_id: String,
|
||||
) -> Result<PluginInstallResponse, JSONRPCErrorError> {
|
||||
let config = self.load_latest_config(/*fallback_cwd*/ None).await?;
|
||||
if !config.features.enabled(Feature::Plugins)
|
||||
@@ -513,8 +513,10 @@ impl CodexMessageProcessor {
|
||||
{
|
||||
return Err(invalid_request("remote plugin install is not enabled"));
|
||||
}
|
||||
if plugin_name.is_empty() || !is_valid_remote_plugin_id(&plugin_name) {
|
||||
return Err(invalid_request("invalid remote plugin id"));
|
||||
if remote_plugin_id.is_empty() || !is_valid_remote_plugin_id(&remote_plugin_id) {
|
||||
return Err(invalid_request(
|
||||
"invalid remote plugin id: only ASCII letters, digits, `_`, `-`, and `~` are allowed",
|
||||
));
|
||||
}
|
||||
|
||||
let auth = self.auth_manager.auth().await;
|
||||
@@ -526,7 +528,7 @@ impl CodexMessageProcessor {
|
||||
&remote_plugin_service_config,
|
||||
auth.as_ref(),
|
||||
&remote_marketplace_name,
|
||||
&plugin_name,
|
||||
&remote_plugin_id,
|
||||
)
|
||||
.await
|
||||
.map_err(|err| {
|
||||
@@ -537,7 +539,7 @@ impl CodexMessageProcessor {
|
||||
})?;
|
||||
if remote_detail.summary.install_policy == PluginInstallPolicy::NotAvailable {
|
||||
return Err(invalid_request(format!(
|
||||
"remote plugin {plugin_name} is not available for install"
|
||||
"remote plugin {remote_plugin_id} is not available for install"
|
||||
)));
|
||||
}
|
||||
let actual_remote_marketplace_name = remote_detail.marketplace_name.clone();
|
||||
@@ -550,7 +552,7 @@ impl CodexMessageProcessor {
|
||||
&remote_detail.summary.name,
|
||||
);
|
||||
let validated_bundle = codex_core_plugins::remote_bundle::validate_remote_plugin_bundle(
|
||||
&plugin_name,
|
||||
&remote_plugin_id,
|
||||
&actual_remote_marketplace_name,
|
||||
&remote_detail.summary.name,
|
||||
remote_detail.release_version.as_deref(),
|
||||
@@ -572,7 +574,7 @@ impl CodexMessageProcessor {
|
||||
&remote_plugin_service_config,
|
||||
auth.as_ref(),
|
||||
&actual_remote_marketplace_name,
|
||||
&plugin_name,
|
||||
&remote_plugin_id,
|
||||
)
|
||||
.await
|
||||
.map_err(|err| remote_plugin_catalog_error_to_jsonrpc(err, "install remote plugin"))?;
|
||||
@@ -585,6 +587,12 @@ impl CodexMessageProcessor {
|
||||
Some(self.effective_plugins_changed_callback(config.clone())),
|
||||
);
|
||||
|
||||
let mut plugin_metadata =
|
||||
plugin_telemetry_metadata_from_root(&result.plugin_id, &result.installed_path).await;
|
||||
plugin_metadata.remote_plugin_id = Some(remote_plugin_id);
|
||||
self.analytics_events_client
|
||||
.track_plugin_installed(plugin_metadata);
|
||||
|
||||
let plugin_mcp_servers = load_plugin_mcp_servers(result.installed_path.as_path()).await;
|
||||
if !plugin_mcp_servers.is_empty() {
|
||||
self.start_plugin_mcp_oauth_logins(&config, plugin_mcp_servers)
|
||||
|
||||
@@ -635,22 +635,7 @@ async fn plugin_install_tracks_analytics_event() -> Result<()> {
|
||||
let response: PluginInstallResponse = to_response(response)?;
|
||||
assert_eq!(response.apps_needing_auth, Vec::<AppSummary>::new());
|
||||
|
||||
let payload = timeout(DEFAULT_TIMEOUT, async {
|
||||
loop {
|
||||
let Some(requests) = analytics_server.received_requests().await else {
|
||||
tokio::time::sleep(Duration::from_millis(25)).await;
|
||||
continue;
|
||||
};
|
||||
if let Some(request) = requests.iter().find(|request| {
|
||||
request.method == "POST" && request.url.path() == "/codex/analytics-events/events"
|
||||
}) {
|
||||
break request.body.clone();
|
||||
}
|
||||
tokio::time::sleep(Duration::from_millis(25)).await;
|
||||
}
|
||||
})
|
||||
.await?;
|
||||
let payload: serde_json::Value = serde_json::from_slice(&payload).expect("analytics payload");
|
||||
let payload = wait_for_plugin_analytics_payload(&analytics_server).await?;
|
||||
assert_eq!(
|
||||
payload,
|
||||
json!({
|
||||
@@ -671,6 +656,59 @@ async fn plugin_install_tracks_analytics_event() -> Result<()> {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn plugin_install_tracks_remote_plugin_analytics_event() -> Result<()> {
|
||||
let codex_home = TempDir::new()?;
|
||||
let server = MockServer::start().await;
|
||||
let bundle_url = mount_remote_plugin_bundle(
|
||||
&server,
|
||||
/*status_code*/ 200,
|
||||
remote_plugin_bundle_tar_gz_bytes("linear")?,
|
||||
)
|
||||
.await;
|
||||
configure_remote_plugin_test(codex_home.path(), &server)?;
|
||||
mount_remote_plugin_detail(&server, REMOTE_PLUGIN_ID, "1.2.3", Some(&bundle_url)).await;
|
||||
mount_empty_remote_installed_plugins(&server).await;
|
||||
mount_remote_plugin_install(&server, REMOTE_PLUGIN_ID).await;
|
||||
mount_backend_analytics_events(&server).await;
|
||||
|
||||
let mut mcp = McpProcess::new_with_env(
|
||||
codex_home.path(),
|
||||
&[(TEST_ALLOW_HTTP_REMOTE_PLUGIN_BUNDLE_DOWNLOADS, Some("1"))],
|
||||
)
|
||||
.await?;
|
||||
timeout(DEFAULT_TIMEOUT, mcp.initialize()).await??;
|
||||
|
||||
let request_id = send_remote_plugin_install_request(&mut mcp, REMOTE_PLUGIN_ID).await?;
|
||||
let response: JSONRPCResponse = timeout(
|
||||
DEFAULT_TIMEOUT,
|
||||
mcp.read_stream_until_response_message(RequestId::Integer(request_id)),
|
||||
)
|
||||
.await??;
|
||||
let response: PluginInstallResponse = to_response(response)?;
|
||||
assert_eq!(response.apps_needing_auth, Vec::<AppSummary>::new());
|
||||
|
||||
let payload = wait_for_plugin_analytics_payload(&server).await?;
|
||||
assert_eq!(
|
||||
payload,
|
||||
json!({
|
||||
"events": [{
|
||||
"event_type": "codex_plugin_installed",
|
||||
"event_params": {
|
||||
"plugin_id": REMOTE_PLUGIN_ID,
|
||||
"plugin_name": "linear",
|
||||
"marketplace_name": "chatgpt-global",
|
||||
"has_skills": true,
|
||||
"mcp_server_count": 0,
|
||||
"connector_ids": [],
|
||||
"product_client_id": DEFAULT_CLIENT_NAME,
|
||||
}
|
||||
}]
|
||||
})
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn plugin_install_errors_when_remote_bundle_download_fails() -> Result<()> {
|
||||
let codex_home = TempDir::new()?;
|
||||
@@ -1150,6 +1188,37 @@ fn write_analytics_config(codex_home: &std::path::Path, base_url: &str) -> std::
|
||||
)
|
||||
}
|
||||
|
||||
async fn mount_backend_analytics_events(server: &MockServer) {
|
||||
Mock::given(method("POST"))
|
||||
.and(path("/backend-api/codex/analytics-events/events"))
|
||||
.respond_with(ResponseTemplate::new(200).set_body_string(r#"{"status":"ok"}"#))
|
||||
.mount(server)
|
||||
.await;
|
||||
}
|
||||
|
||||
async fn wait_for_plugin_analytics_payload(server: &MockServer) -> Result<serde_json::Value> {
|
||||
timeout(DEFAULT_TIMEOUT, async {
|
||||
loop {
|
||||
let Some(requests) = server.received_requests().await else {
|
||||
tokio::time::sleep(Duration::from_millis(25)).await;
|
||||
continue;
|
||||
};
|
||||
if let Some(request) = requests.iter().find(|request| {
|
||||
request.method == "POST"
|
||||
&& request
|
||||
.url
|
||||
.path()
|
||||
.ends_with("/codex/analytics-events/events")
|
||||
}) {
|
||||
return serde_json::from_slice(&request.body)
|
||||
.map_err(|err| anyhow::anyhow!("invalid analytics payload: {err}"));
|
||||
}
|
||||
tokio::time::sleep(Duration::from_millis(25)).await;
|
||||
}
|
||||
})
|
||||
.await?
|
||||
}
|
||||
|
||||
fn write_remote_plugin_catalog_config(
|
||||
codex_home: &std::path::Path,
|
||||
base_url: &str,
|
||||
|
||||
@@ -927,6 +927,7 @@ pub async fn plugin_telemetry_metadata_from_root(
|
||||
|
||||
PluginTelemetryMetadata {
|
||||
plugin_id: plugin_id.clone(),
|
||||
remote_plugin_id: None,
|
||||
capability_summary: Some(PluginCapabilitySummary {
|
||||
config_name: plugin_id.as_key(),
|
||||
display_name: plugin_id.plugin_name.clone(),
|
||||
|
||||
@@ -42,6 +42,9 @@ pub struct PluginHookSource {
|
||||
#[derive(Debug, Clone, PartialEq, Eq)]
|
||||
pub struct PluginTelemetryMetadata {
|
||||
pub plugin_id: PluginId,
|
||||
/// Optional backend identifier for remote plugins, used when analytics
|
||||
/// should report the remote id instead of the local plugin cache id.
|
||||
pub remote_plugin_id: Option<String>,
|
||||
pub capability_summary: Option<PluginCapabilitySummary>,
|
||||
}
|
||||
|
||||
@@ -49,6 +52,7 @@ impl PluginTelemetryMetadata {
|
||||
pub fn from_plugin_id(plugin_id: &PluginId) -> Self {
|
||||
Self {
|
||||
plugin_id: plugin_id.clone(),
|
||||
remote_plugin_id: None,
|
||||
capability_summary: None,
|
||||
}
|
||||
}
|
||||
@@ -60,6 +64,7 @@ impl PluginCapabilitySummary {
|
||||
.ok()
|
||||
.map(|plugin_id| PluginTelemetryMetadata {
|
||||
plugin_id,
|
||||
remote_plugin_id: None,
|
||||
capability_summary: Some(self.clone()),
|
||||
})
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user