Don't spawn Anthropic telemetry event when API key is missing (#32813)

Richard Feldman created 5 months ago

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<String>` 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

Change summary

Cargo.lock                             |  1 
crates/language_model/Cargo.toml       |  1 
crates/language_model/src/telemetry.rs | 51 ++++++++++++---------------
3 files changed, 25 insertions(+), 28 deletions(-)

Detailed changes

Cargo.lock

@@ -8944,6 +8944,7 @@ dependencies = [
  "http_client",
  "icons",
  "image",
+ "log",
  "parking_lot",
  "proto",
  "schemars",

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

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<dyn HttpClient>,
-    model_api_key: Option<String>,
-) -> 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(),
-    )));
 }