Update UserAgentPolicy to better align with guidelines (#2743)

Adds documentation to our repo's root README - which is also now syntactically verified when testing `azure_core_test`, and removes the `UserAgentOptions::disabled` in favor of, like most languages, recommending a user policy to remove the `User-Agent` header.

Author: heaths

README.md

@@ -39,6 +39,52 @@ Building each crate should be as straight forward as `cargo build`, but check ea
 - Have a question, or find a bug? File an issue via [GitHub Issues](https://github.com/Azure/azure-sdk-for-rust/issues/new/choose).
 - Check [previous questions](https://stackoverflow.com/questions/tagged/azure+rust) or ask new ones on StackOverflow using the `azure` and `rust` tags.
 
+## Data Collection
+
+The software may collect information about you and your use of the software and send it to Microsoft. Microsoft may use this information to provide services and improve our products and services. You may turn off the telemetry as described below. You can learn more about data collection and use in the help documentation and Microsoft’s [privacy statement](https://go.microsoft.com/fwlink/?LinkID=824704). For more information on the data collected by the Azure SDK, please visit the [Telemetry Guidelines](https://azure.github.io/azure-sdk/general_azurecore.html#telemetry-policy) page.
+
+### Telemetry Configuration
+
+A `User-Agent` header is sent in requests by default with a value similar to:
+
+> azsdk-rust-security_keyvault_secrets/0.4.0 (1.86.0; linux; aarch64)
+
+You can assign an optional application ID for your own telemetry by setting `UserPolicyOptions::application_id`. This will appear at the beginning of the `User-Agent` header.
+
+To disable sending the `User-Agent` header entirely, you can write a `Policy` that will remove it:
+
+```rust no_run
+use async_trait::async_trait;
+use azure_core::http::{
+    policies::{Policy, PolicyResult},
+    Context, Request,
+};
+use std::sync::Arc;
+
+// Define a policy that will remove the User-Agent header.
+#[derive(Debug)]
+struct RemoveUserAgent;
+
+#[async_trait]
+impl Policy for RemoveUserAgent {
+    async fn send(
+        &self,
+        ctx: &Context,
+        request: &mut Request,
+        next: &[Arc<dyn Policy>],
+    ) -> PolicyResult {
+        let headers = request.headers_mut();
+
+        // Note: HTTP headers are case-insensitive but client-added headers are normalized to lowercase.
+        headers.remove("user-agent");
+
+        next[0].send(ctx, request, &next[1..]).await
+    }
+}
+```
+
+For a complete example, see our [`azure_core` example](https://github.com/Azure/azure-sdk-for-rust/blob/main/sdk/core/azure_core/examples/core_remove_user_agent.rs).
+
 ### Reporting security issues and security bugs
 
 Security issues and bugs should be reported privately, via email, to the Microsoft Security Response Center (MSRC) <secure@microsoft.com>. You should receive a response within 24 hours. If for some reason you do not, please follow up via email to ensure we received your original message. Further information, including the MSRC PGP key, can be found in the [Security TechCenter](https://www.microsoft.com/msrc/faqs-report-an-issue).

sdk/core/azure_core/CHANGELOG.md

@@ -5,7 +5,6 @@
 ### Features Added
 
 - Added `get_async_runtime()` and `set_async_runtime()` to allow customers to replace the asynchronous runtime used by the Azure SDK.
-- Added `UserAgentOptions::enabled` to allow disabling sending the `User-Agent` header.
 
 ### Breaking Changes
 

sdk/core/azure_core/examples/core_remove_user_agent.rs

@@ -0,0 +1,110 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT License.
+
+use async_trait::async_trait;
+use azure_core::{
+    credentials::TokenCredential,
+    http::{
+        headers::Headers,
+        policies::{Policy, PolicyResult},
+        Context, HttpClient, Method, RawResponse, Request, StatusCode, TransportOptions,
+    },
+};
+use azure_core_test::{credentials::MockCredential, http::MockHttpClient};
+use azure_security_keyvault_secrets::{SecretClient, SecretClientOptions};
+use futures::FutureExt;
+use std::sync::Arc;
+
+// Define a policy that will remove the User-Agent header.
+#[derive(Debug)]
+struct RemoveUserAgent;
+
+#[cfg_attr(target_arch = "wasm32", async_trait(?Send))]
+#[cfg_attr(not(target_arch = "wasm32"), async_trait)]
+impl Policy for RemoveUserAgent {
+    async fn send(
+        &self,
+        ctx: &Context,
+        request: &mut Request,
+        next: &[Arc<dyn Policy>],
+    ) -> PolicyResult {
+        let headers = request.headers_mut();
+
+        // Note: HTTP headers are case-insensitive but client-added headers are normalized to lowercase.
+        headers.remove("user-agent");
+
+        next[0].send(ctx, request, &next[1..]).await
+    }
+}
+
+async fn test_remove_user_agent() -> Result<(), Box<dyn std::error::Error>> {
+    // Policies are created in an Arc to be generally shared.
+    let remove_user_agent = Arc::new(RemoveUserAgent);
+
+    // Construct client options with your policy that runs after the built-in per-call UserAgentPolicy.
+    let mut options = SecretClientOptions::default();
+    options
+        .client_options
+        .per_call_policies
+        .push(remove_user_agent);
+
+    // Ignore: this is only set up for testing.
+    // You normally would create credentials from `azure_identity` and
+    // use the default transport in production.
+    let (credential, transport) = setup()?;
+    options.client_options.transport = Some(TransportOptions::new(transport));
+
+    // Construct the client with these options and a shared credential.
+    let client = SecretClient::new(
+        "https://my-vault.vault.azure.net",
+        credential.clone(),
+        Some(options),
+    )?;
+
+    // We'll fetch a secret and let the mock client assert the User-Agent header was removed.
+    let secret = client
+        .get_secret("my-secret", "", None)
+        .await?
+        .into_body()
+        .await?;
+    assert_eq!(secret.value.as_deref(), Some("secret-value"));
+
+    Ok(())
+}
+
+// ----- BEGIN TEST SETUP -----
+#[tokio::test]
+async fn test_core_remove_user_agent() -> Result<(), Box<dyn std::error::Error>> {
+    test_remove_user_agent().await
+}
+
+#[tokio::main]
+async fn main() -> Result<(), Box<dyn std::error::Error>> {
+    test_remove_user_agent().await
+}
+
+#[allow(clippy::type_complexity)]
+fn setup() -> Result<(Arc<dyn TokenCredential>, Arc<dyn HttpClient>), Box<dyn std::error::Error>> {
+    let client = MockHttpClient::new(|request| {
+        async move {
+            assert!(request.url().path().starts_with("/secrets/my-secret"));
+            assert_eq!(*request.method(), Method::Get);
+            assert!(
+                !request
+                    .headers()
+                    .iter()
+                    .any(|(name, _)| name.as_str().eq_ignore_ascii_case("user-agent")),
+                "user-agent header should be absent"
+            );
+            Ok(RawResponse::from_bytes(
+                StatusCode::Ok,
+                Headers::new(),
+                r#"{"value":"secret-value"}"#,
+            ))
+        }
+        .boxed()
+    });
+
+    Ok((MockCredential::new()?, Arc::new(client)))
+}
+// ----- END TEST SETUP -----

sdk/core/azure_core/src/http/options/user_agent.rs

@@ -5,8 +5,10 @@
 #[derive(Clone, Debug, Default)]
 pub struct UserAgentOptions {
     /// Set the application ID in the `User-Agent` header that can be telemetered.
+    ///
+    /// # Panics
+    ///
+    /// Panics if [`UserAgentOptions::application_id`] is greater than 24 characters.
+    /// See [guidelines](https://azure.github.io/azure-sdk/general_azurecore.html#azurecore-http-telemetry-appid-length) for details.
     pub application_id: Option<String>,
-
-    /// Disable to prevent sending the `User-Agent` header in requests.
-    pub disabled: bool,
 }

sdk/core/azure_core/src/http/pipeline.rs

@@ -52,10 +52,8 @@ impl Pipeline {
         push_unique(&mut per_call_policies, ClientRequestIdPolicy::default());
 
         let (user_agent, options) = options.deconstruct();
-        if !user_agent.disabled {
-            let telemetry_policy = UserAgentPolicy::new(crate_name, crate_version, &user_agent);
-            push_unique(&mut per_call_policies, telemetry_policy);
-        }
+        let telemetry_policy = UserAgentPolicy::new(crate_name, crate_version, &user_agent);
+        push_unique(&mut per_call_policies, telemetry_policy);
 
         Self(http::Pipeline::new(
             options,
@@ -263,17 +261,24 @@ mod tests {
     }
 
     #[tokio::test]
-    async fn pipeline_with_user_agent_disabled() {
+    async fn pipeline_with_custom_application_id() {
         // Arrange
+        const CUSTOM_APPLICATION_ID: &str = "my-custom-app/2.1.0";
         let ctx = Context::new();
 
         let transport = TransportOptions::new(Arc::new(MockHttpClient::new(|req| {
             async {
                 // Assert
-                let user_agent = req.headers().get_optional_str(&headers::USER_AGENT);
+                let user_agent = req
+                    .headers()
+                    .get_optional_str(&headers::USER_AGENT)
+                    .expect("User-Agent header should be present");
+                // The user agent should contain the custom application_id followed by the standard Azure SDK format
+                // Expected format: my-custom-app/2.1.0 azsdk-rust-test-crate/1.0.0 (<rustc_version>; <OS>; <ARCH>)
                 assert!(
-                    user_agent.is_none(),
-                    "User-Agent header should not be present when disabled"
+                    user_agent.starts_with("my-custom-app/2.1.0 azsdk-rust-test-crate/1.0.0 "),
+                    "User-Agent header should start with custom application_id and expected prefix, got: {}",
+                    user_agent
                 );
 
                 Ok(RawResponse::from_bytes(
@@ -284,13 +289,14 @@ mod tests {
             }
             .boxed()
         })));
-        let user_agent = UserAgentOptions {
-            disabled: true,
-            ..Default::default()
+
+        let user_agent_options = UserAgentOptions {
+            application_id: Some(CUSTOM_APPLICATION_ID.to_string()),
         };
+
         let options = ClientOptions {
             transport: Some(transport),
-            user_agent: Some(user_agent),
+            user_agent: Some(user_agent_options),
             ..Default::default()
         };
 

sdk/core/azure_core/src/http/policies/user_agent.rs

@@ -10,13 +10,19 @@ use std::sync::Arc;
 use typespec_client_core::http::policies::{Policy, PolicyResult};
 use typespec_client_core::http::{Context, Request};
 
-/// Sets the User-Agent header with useful information in a typical format for Azure SDKs.
+/// Sets the `User-Agent` header with useful information in a typical format for Azure SDKs.
 #[derive(Clone, Debug)]
 pub struct UserAgentPolicy {
     header: String,
 }
 
 impl<'a> UserAgentPolicy {
+    /// Create a new `UserAgentPolicy`.
+    ///
+    /// # Panics
+    ///
+    /// Panics if [`UserAgentOptions::application_id`] is greater than 24 characters.
+    /// See [guidelines](https://azure.github.io/azure-sdk/general_azurecore.html#azurecore-http-telemetry-appid-length) for details.
     pub fn new(
         crate_name: Option<&'a str>,
         crate_version: Option<&'a str>,
@@ -46,8 +52,15 @@ impl<'a> UserAgentPolicy {
             crate_name = name;
         }
 
+        const MAX_APPLICATION_ID_LEN: usize = 24;
         let header = match &options.application_id {
             Some(application_id) => {
+                if application_id.len() > MAX_APPLICATION_ID_LEN {
+                    panic!(
+                        "application_id must be shorter than {} characters",
+                        MAX_APPLICATION_ID_LEN + 1
+                    );
+                }
                 format!("{application_id} azsdk-rust-{crate_name}/{crate_version} {platform_info}")
             }
             None => format!("azsdk-rust-{crate_name}/{crate_version} {platform_info}"),
@@ -94,7 +107,6 @@ mod tests {
     fn with_application_id() {
         let options = UserAgentOptions {
             application_id: Some("my_app".to_string()),
-            ..Default::default()
         };
         let policy = UserAgentPolicy::new_with_rustc_version(
             Some("test"),
@@ -118,4 +130,37 @@ mod tests {
             format!("azsdk-rust-unknown/unknown (unknown; {OS}; {ARCH})")
         );
     }
+
+    #[test]
+    #[should_panic(expected = "application_id must be shorter than 25 characters")]
+    fn panics_when_application_id_too_long() {
+        let options = UserAgentOptions {
+            application_id: Some(
+                "this_application_id_is_way_too_long_and_exceeds_limit".to_string(),
+            ), // 53 characters
+        };
+        let _policy = UserAgentPolicy::new_with_rustc_version(
+            Some("test"),
+            Some("1.2.3"),
+            Some("4.5.6"),
+            &options,
+        );
+    }
+
+    #[test]
+    fn works_with_application_id_at_limit() {
+        let options = UserAgentOptions {
+            application_id: Some("exactly_24_characters!".to_string()), // Exactly 24 characters
+        };
+        let policy = UserAgentPolicy::new_with_rustc_version(
+            Some("test"),
+            Some("1.2.3"),
+            Some("4.5.6"),
+            &options,
+        );
+        assert_eq!(
+            policy.header,
+            format!("exactly_24_characters! azsdk-rust-test/1.2.3 (4.5.6; {OS}; {ARCH})")
+        );
+    }
 }

sdk/core/azure_core_test/src/credentials.rs

@@ -13,6 +13,13 @@ use std::{env, sync::Arc};
 #[derive(Clone, Debug, Default)]
 pub struct MockCredential;
 
+impl MockCredential {
+    /// Create a new `MockCredential`.
+    pub fn new() -> azure_core::Result<Arc<Self>> {
+        Ok(Arc::new(MockCredential {}))
+    }
+}
+
 #[cfg_attr(target_arch = "wasm32", async_trait::async_trait(?Send))]
 #[cfg_attr(not(target_arch = "wasm32"), async_trait::async_trait)]
 impl TokenCredential for MockCredential {

sdk/core/azure_core_test/src/lib.rs

@@ -8,6 +8,8 @@ pub mod http;
 pub mod proxy;
 pub mod recorded;
 mod recording;
+#[cfg(doctest)]
+mod root_readme;
 pub mod stream;
 
 use azure_core::Error;

sdk/core/azure_core_test/src/root_readme.rs

@@ -0,0 +1,3 @@
+#![doc = include_str!("../../../../README.md")]
+
+// Make sure the repository's root README.md has no syntactical errors.