From cfbc2d097279d0e70433302af23d8d056b91b5f7 Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Mon, 16 Jun 2025 14:58:37 -0400 Subject: [PATCH] Don't spawn Anthropic telemetry event when API key is missing (#32813) Minor refactor that I'm extracting from a branch because it can stand alone. - Now we no longer spawn an executor for `report_anthropic_event` if it's just going to immediately fail due to API key being missing - `report_anthropic_event` now takes a `String` API key instead of `Option` and the error reporting if the key is missing has been moved to the caller. - `report_anthropic_event` is longer coupled to `AnthropicError`, because all it ever did was generate an `AnthropicEvent::Other`, which in turn was then only used for `log_err` - so, can just be an `anyhow::Result`. Release Notes: - N/A --- Cargo.lock | 1 + crates/language_model/Cargo.toml | 1 + crates/language_model/src/telemetry.rs | 51 ++++++++++++-------------- 3 files changed, 25 insertions(+), 28 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 4880424b7348a369b37e1b022168277eefe1ad9d..e6a0d087fae58582712b8eb2e6705f688c59bf3f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -8944,6 +8944,7 @@ dependencies = [ "http_client", "icons", "image", + "log", "parking_lot", "proto", "schemars", diff --git a/crates/language_model/Cargo.toml b/crates/language_model/Cargo.toml index aa4815e5bbd296d0edbcd7d022ccd9c829bcc01f..b718c530f5cd59f25593fb3c5261bdc706839223 100644 --- a/crates/language_model/Cargo.toml +++ b/crates/language_model/Cargo.toml @@ -26,6 +26,7 @@ gpui.workspace = true http_client.workspace = true icons.workspace = true image.workspace = true +log.workspace = true parking_lot.workspace = true proto.workspace = true schemars.workspace = true diff --git a/crates/language_model/src/telemetry.rs b/crates/language_model/src/telemetry.rs index 62b199bd456bf6f164b03e265fd6cec2f6e468a0..9bd9b903c20e1b314a9e1f945ca1ddd1bb608279 100644 --- a/crates/language_model/src/telemetry.rs +++ b/crates/language_model/src/telemetry.rs @@ -1,5 +1,5 @@ -use anthropic::{ANTHROPIC_API_URL, AnthropicError}; -use anyhow::{Context as _, Result, anyhow}; +use anthropic::ANTHROPIC_API_URL; +use anyhow::{Context as _, anyhow}; use client::telemetry::Telemetry; use gpui::BackgroundExecutor; use http_client::{AsyncBody, HttpClient, Method, Request as HttpRequest}; @@ -20,13 +20,17 @@ pub fn report_assistant_event( if let Some(telemetry) = telemetry.as_ref() { telemetry.report_assistant_event(event.clone()); if telemetry.metrics_enabled() && event.model_provider == ANTHROPIC_PROVIDER_ID { - executor - .spawn(async move { - report_anthropic_event(event, client, model_api_key) - .await - .log_err(); - }) - .detach(); + if let Some(api_key) = model_api_key { + executor + .spawn(async move { + report_anthropic_event(event, client, api_key) + .await + .log_err(); + }) + .detach(); + } else { + log::error!("Cannot send Anthropic telemetry because API key is missing"); + } } } } @@ -34,17 +38,8 @@ pub fn report_assistant_event( async fn report_anthropic_event( event: AssistantEventData, client: Arc, - model_api_key: Option, -) -> Result<(), AnthropicError> { - let api_key = match model_api_key { - Some(key) => key, - None => { - return Err(AnthropicError::Other(anyhow!( - "Anthropic API key is not set" - ))); - } - }; - + api_key: String, +) -> anyhow::Result<()> { let uri = format!("{ANTHROPIC_API_URL}/v1/log/zed"); let request_builder = HttpRequest::builder() .method(Method::POST) @@ -72,19 +67,19 @@ async fn report_anthropic_event( let request = request_builder .body(AsyncBody::from(serialized_event.to_string())) - .context("failed to construct request body")?; + .context("Failed to construct Anthropic telemetry HTTP request body")?; let response = client .send(request) .await - .context("failed to send request to Anthropic")?; + .context("Failed to send telemetry HTTP request to Anthropic")?; if response.status().is_success() { - return Ok(()); + Ok(()) + } else { + Err(anyhow!( + "Anthropic telemetry logging failed with HTTP status: {}", + response.status() + )) } - - return Err(AnthropicError::Other(anyhow!( - "Failed to log: {}", - response.status(), - ))); }