From 2d1d028cc6a5c5da57bfde9ab5a984f3f70fe4ac Mon Sep 17 00:00:00 2001 From: Heath Stewart Date: Fri, 28 May 2021 14:11:24 -0700 Subject: [PATCH 1/8] Add TelemetryPolicy --- sdk/core/Cargo.toml | 20 ++++--- sdk/core/src/pipeline.rs | 13 +++-- sdk/core/src/policies/mod.rs | 2 + sdk/core/src/policies/telemetry_policy.rs | 67 +++++++++++++++++++++++ sdk/core/src/request.rs | 4 ++ 5 files changed, 92 insertions(+), 14 deletions(-) create mode 100644 sdk/core/src/policies/telemetry_policy.rs diff --git a/sdk/core/Cargo.toml b/sdk/core/Cargo.toml index b7c05ba4e9..4b91cf0bb7 100644 --- a/sdk/core/Cargo.toml +++ b/sdk/core/Cargo.toml @@ -13,24 +13,26 @@ categories = ["api-bindings"] edition = "2018" [dependencies] +async-trait = "0.1" +bytes = "1.0" chrono = "0.4" -http = "0.2" +clap = "2.33.3" +dyn-clone = "1.0" futures = "0.3" +http = "0.2" hyper = { version = "0.14", optional = true } +hyper-rustls = { version = "0.22", optional = true } log = "0.4" -thiserror = "1.0" +oauth2 = "4.0.0" +rand = "0.7" +reqwest = { version = "0.11", features = ["stream"], optional = true } +rustc_version = "0.3.3" serde = "1.0" serde_derive = "1.0" serde_json = "1.0" +thiserror = "1.0" url = "2.2" uuid = { version = "0.8" } -bytes = "1.0" -hyper-rustls = { version = "0.22", optional = true } -async-trait = "0.1" -oauth2 = "4.0.0" -reqwest = { version = "0.11", features = ["stream"], optional = true } -rand = "0.7" -dyn-clone = "1.0" [dev-dependencies] env_logger = "0.8" diff --git a/sdk/core/src/pipeline.rs b/sdk/core/src/pipeline.rs index dc17d445b3..a007231c4b 100644 --- a/sdk/core/src/pipeline.rs +++ b/sdk/core/src/pipeline.rs @@ -1,4 +1,4 @@ -use crate::policies::{Policy, PolicyResult}; +use crate::policies::{Policy, PolicyResult, TelemetryPolicy}; use crate::{Context, Request, Response}; use std::sync::Arc; @@ -8,10 +8,10 @@ use std::sync::Arc; /// /// 1. Per call policies are executed. Per call policies can fail and bail out of the pipeline /// immediately. -/// 2. Retry policy. It allows to reexecute the following policies. -/// 3. Per retry policies. Per retry polices are always executed at least once but are reexecuted +/// 2. Retry policy. It allows to re-execute the following policies. +/// 3. Per retry policies. Per retry polices are always executed at least once but are re-executed /// in case of retries. -/// 4. Transport policy. Transtport policy is always the last policy and is the policy that +/// 4. Transport policy. Transport policy is always the last policy and is the policy that /// actually constructs the `Response` to be passed up the pipeline. /// /// A pipeline is immutable. In other words a policy can either succeed and call the following @@ -31,7 +31,10 @@ impl Pipeline { transport_policy: Arc, ) -> Self { let mut pipeline = - Vec::with_capacity(per_call_policies.len() + per_retry_policies.len() + 2); + Vec::>::with_capacity(per_call_policies.len() + per_retry_policies.len() + 3); + + // TODO: Create pipeline from ClientOptions which should contain user-specified policies + client-added policies. + pipeline.push(Arc::new(TelemetryPolicy::default())); pipeline.extend_from_slice(&per_call_policies); pipeline.push(retry); diff --git a/sdk/core/src/policies/mod.rs b/sdk/core/src/policies/mod.rs index 2845106c63..c98823dd4b 100644 --- a/sdk/core/src/policies/mod.rs +++ b/sdk/core/src/policies/mod.rs @@ -1,10 +1,12 @@ mod retry_policies; +mod telemetry_policy; mod transport; use crate::{Context, Request, Response}; pub use retry_policies::*; use std::error::Error; use std::sync::Arc; +pub use telemetry_policy::*; pub use transport::*; pub type PolicyResult = Result>; diff --git a/sdk/core/src/policies/telemetry_policy.rs b/sdk/core/src/policies/telemetry_policy.rs new file mode 100644 index 0000000000..f635e27a03 --- /dev/null +++ b/sdk/core/src/policies/telemetry_policy.rs @@ -0,0 +1,67 @@ +use crate::{Context, Request, Response}; +use crate::policies::{Policy, PolicyResult}; + +use http::{HeaderValue, header::USER_AGENT}; +use rustc_version::{Version, version}; +use std::env::consts::{ARCH, OS}; +use std::sync::Arc; + +#[derive(Clone, Debug, Default)] +pub struct TelemetryOptions { + application_id: Option, +} + +impl TelemetryOptions { + pub fn new(application_id: Option) -> Self { + Self { application_id } + } +} + +#[derive(Clone, Debug)] +pub struct TelemetryPolicy { + header: String, +} + +const EMPTY_VERSION: Version = Version { + major: 0, + minor: 0, + patch: 0, + pre: Vec::new(), + build: Vec::new(), +}; + +impl TelemetryPolicy { + pub fn new(options: TelemetryOptions) -> Self { + let platform_info = format!("({}; {}; {})", version().unwrap_or(EMPTY_VERSION), OS, ARCH); + if let Some(application_id) = options.application_id { + TelemetryPolicy { + header: format!("{} azsdk-rust-{}/{} {}", application_id, clap::crate_name!(), clap::crate_version!(), platform_info), + } + } else { + TelemetryPolicy { + header: format!("azsdk-rust-{}/{} {}", clap::crate_name!(), clap::crate_version!(), platform_info), + } + } + } +} + +impl Default for TelemetryPolicy { + fn default() -> Self { + TelemetryPolicy::new(TelemetryOptions::default()) + } +} + +#[async_trait::async_trait] +impl Policy for TelemetryPolicy { + async fn send( + &self, + ctx: &mut Context, + request: &mut Request, + next: &[Arc], + ) -> PolicyResult { + + request.headers_mut().insert(USER_AGENT, HeaderValue::from_str(&self.header).unwrap()); + + next[0].send(ctx, request, &next[1..]).await + } +} diff --git a/sdk/core/src/request.rs b/sdk/core/src/request.rs index 11d170c936..7d1c0a779e 100644 --- a/sdk/core/src/request.rs +++ b/sdk/core/src/request.rs @@ -45,6 +45,10 @@ impl Request { &self.headers } + pub fn headers_mut(&mut self) -> &mut HeaderMap { + &mut self.headers + } + pub fn body(&self) -> &Body { &self.body } From a7a25891723dcdc27b850a7237e1453a96c0c0ba Mon Sep 17 00:00:00 2001 From: Heath Stewart Date: Fri, 28 May 2021 14:22:18 -0700 Subject: [PATCH 2/8] Satisfy `cargo fmt` --- sdk/core/src/pipeline.rs | 5 +++-- sdk/core/src/policies/telemetry_policy.rs | 26 +++++++++++++++++------ 2 files changed, 22 insertions(+), 9 deletions(-) diff --git a/sdk/core/src/pipeline.rs b/sdk/core/src/pipeline.rs index a007231c4b..6ad2c42bb4 100644 --- a/sdk/core/src/pipeline.rs +++ b/sdk/core/src/pipeline.rs @@ -30,8 +30,9 @@ impl Pipeline { per_retry_policies: Vec>, transport_policy: Arc, ) -> Self { - let mut pipeline = - Vec::>::with_capacity(per_call_policies.len() + per_retry_policies.len() + 3); + let mut pipeline = Vec::>::with_capacity( + per_call_policies.len() + per_retry_policies.len() + 3, + ); // TODO: Create pipeline from ClientOptions which should contain user-specified policies + client-added policies. pipeline.push(Arc::new(TelemetryPolicy::default())); diff --git a/sdk/core/src/policies/telemetry_policy.rs b/sdk/core/src/policies/telemetry_policy.rs index f635e27a03..fa6a57e8b6 100644 --- a/sdk/core/src/policies/telemetry_policy.rs +++ b/sdk/core/src/policies/telemetry_policy.rs @@ -1,8 +1,8 @@ -use crate::{Context, Request, Response}; use crate::policies::{Policy, PolicyResult}; +use crate::{Context, Request, Response}; -use http::{HeaderValue, header::USER_AGENT}; -use rustc_version::{Version, version}; +use http::{header::USER_AGENT, HeaderValue}; +use rustc_version::{version, Version}; use std::env::consts::{ARCH, OS}; use std::sync::Arc; @@ -35,11 +35,22 @@ impl TelemetryPolicy { let platform_info = format!("({}; {}; {})", version().unwrap_or(EMPTY_VERSION), OS, ARCH); if let Some(application_id) = options.application_id { TelemetryPolicy { - header: format!("{} azsdk-rust-{}/{} {}", application_id, clap::crate_name!(), clap::crate_version!(), platform_info), + header: format!( + "{} azsdk-rust-{}/{} {}", + application_id, + clap::crate_name!(), + clap::crate_version!(), + platform_info + ), } } else { TelemetryPolicy { - header: format!("azsdk-rust-{}/{} {}", clap::crate_name!(), clap::crate_version!(), platform_info), + header: format!( + "azsdk-rust-{}/{} {}", + clap::crate_name!(), + clap::crate_version!(), + platform_info + ), } } } @@ -59,8 +70,9 @@ impl Policy for TelemetryPolicy { request: &mut Request, next: &[Arc], ) -> PolicyResult { - - request.headers_mut().insert(USER_AGENT, HeaderValue::from_str(&self.header).unwrap()); + request + .headers_mut() + .insert(USER_AGENT, HeaderValue::from_str(&self.header).unwrap()); next[0].send(ctx, request, &next[1..]).await } From d53f2333631569dce6683bcc486413c649c8e50b Mon Sep 17 00:00:00 2001 From: Heath Stewart Date: Wed, 2 Jun 2021 00:22:04 -0700 Subject: [PATCH 3/8] Resolve PR feedback --- sdk/core/Cargo.toml | 5 ++- sdk/core/build.rs | 5 +++ sdk/core/src/pipeline.rs | 2 +- sdk/core/src/policies/telemetry_policy.rs | 55 +++++++++++------------ 4 files changed, 35 insertions(+), 32 deletions(-) create mode 100644 sdk/core/build.rs diff --git a/sdk/core/Cargo.toml b/sdk/core/Cargo.toml index 4b91cf0bb7..77a36574c6 100644 --- a/sdk/core/Cargo.toml +++ b/sdk/core/Cargo.toml @@ -16,7 +16,6 @@ edition = "2018" async-trait = "0.1" bytes = "1.0" chrono = "0.4" -clap = "2.33.3" dyn-clone = "1.0" futures = "0.3" http = "0.2" @@ -26,7 +25,6 @@ log = "0.4" oauth2 = "4.0.0" rand = "0.7" reqwest = { version = "0.11", features = ["stream"], optional = true } -rustc_version = "0.3.3" serde = "1.0" serde_derive = "1.0" serde_json = "1.0" @@ -34,6 +32,9 @@ thiserror = "1.0" url = "2.2" uuid = { version = "0.8" } +[build-dependencies] +rustc_version = "0.3.3" + [dev-dependencies] env_logger = "0.8" tokio = { version = "1.0", features = ["default"] } diff --git a/sdk/core/build.rs b/sdk/core/build.rs new file mode 100644 index 0000000000..6ad24b1f83 --- /dev/null +++ b/sdk/core/build.rs @@ -0,0 +1,5 @@ +use rustc_version::version; + +fn main() { + println!("cargo:rustc-env=AZSDK_RUSTC_VERSION={}", version().unwrap()); +} diff --git a/sdk/core/src/pipeline.rs b/sdk/core/src/pipeline.rs index 6ad2c42bb4..0c83c4cf85 100644 --- a/sdk/core/src/pipeline.rs +++ b/sdk/core/src/pipeline.rs @@ -30,7 +30,7 @@ impl Pipeline { per_retry_policies: Vec>, transport_policy: Arc, ) -> Self { - let mut pipeline = Vec::>::with_capacity( + let mut pipeline: Vec> = Vec::with_capacity( per_call_policies.len() + per_retry_policies.len() + 3, ); diff --git a/sdk/core/src/policies/telemetry_policy.rs b/sdk/core/src/policies/telemetry_policy.rs index fa6a57e8b6..81b1841a1d 100644 --- a/sdk/core/src/policies/telemetry_policy.rs +++ b/sdk/core/src/policies/telemetry_policy.rs @@ -2,7 +2,6 @@ use crate::policies::{Policy, PolicyResult}; use crate::{Context, Request, Response}; use http::{header::USER_AGENT, HeaderValue}; -use rustc_version::{version, Version}; use std::env::consts::{ARCH, OS}; use std::sync::Arc; @@ -22,36 +21,34 @@ pub struct TelemetryPolicy { header: String, } -const EMPTY_VERSION: Version = Version { - major: 0, - minor: 0, - patch: 0, - pre: Vec::new(), - build: Vec::new(), -}; - impl TelemetryPolicy { pub fn new(options: TelemetryOptions) -> Self { - let platform_info = format!("({}; {}; {})", version().unwrap_or(EMPTY_VERSION), OS, ARCH); - if let Some(application_id) = options.application_id { - TelemetryPolicy { - header: format!( - "{} azsdk-rust-{}/{} {}", - application_id, - clap::crate_name!(), - clap::crate_version!(), - platform_info - ), - } - } else { - TelemetryPolicy { - header: format!( - "azsdk-rust-{}/{} {}", - clap::crate_name!(), - clap::crate_version!(), - platform_info - ), - } + let crate_name = env!("CARGO_PKG_NAME"); + let crate_version = env!("CARGO_PKG_VERSION"); + let platform_info = format!( + "({}; {}; {})", + env!("AZSDK_RUSTC_VERSION"), + OS, + ARCH, + ); + let header = match options.application_id { + Some(application_id) => format!( + "{} azsdk-rust-{}/{} {}", + application_id, + crate_name, + crate_version, + platform_info + ), + None => format!( + "azsdk-rust-{}/{} {}", + crate_name, + crate_version, + platform_info + ), + }; + + TelemetryPolicy { + header: header, } } } From 250c3362b462c59df4b7580048cf7a3c9d3a8473 Mon Sep 17 00:00:00 2001 From: Heath Stewart Date: Wed, 2 Jun 2021 13:34:14 -0700 Subject: [PATCH 4/8] Satisfy `cargo fmt` --- sdk/core/src/pipeline.rs | 5 ++--- sdk/core/src/policies/telemetry_policy.rs | 20 ++++---------------- 2 files changed, 6 insertions(+), 19 deletions(-) diff --git a/sdk/core/src/pipeline.rs b/sdk/core/src/pipeline.rs index 0c83c4cf85..1aa0a294e6 100644 --- a/sdk/core/src/pipeline.rs +++ b/sdk/core/src/pipeline.rs @@ -30,9 +30,8 @@ impl Pipeline { per_retry_policies: Vec>, transport_policy: Arc, ) -> Self { - let mut pipeline: Vec> = Vec::with_capacity( - per_call_policies.len() + per_retry_policies.len() + 3, - ); + let mut pipeline: Vec> = + Vec::with_capacity(per_call_policies.len() + per_retry_policies.len() + 3); // TODO: Create pipeline from ClientOptions which should contain user-specified policies + client-added policies. pipeline.push(Arc::new(TelemetryPolicy::default())); diff --git a/sdk/core/src/policies/telemetry_policy.rs b/sdk/core/src/policies/telemetry_policy.rs index 81b1841a1d..89441f8969 100644 --- a/sdk/core/src/policies/telemetry_policy.rs +++ b/sdk/core/src/policies/telemetry_policy.rs @@ -25,31 +25,19 @@ impl TelemetryPolicy { pub fn new(options: TelemetryOptions) -> Self { let crate_name = env!("CARGO_PKG_NAME"); let crate_version = env!("CARGO_PKG_VERSION"); - let platform_info = format!( - "({}; {}; {})", - env!("AZSDK_RUSTC_VERSION"), - OS, - ARCH, - ); + let platform_info = format!("({}; {}; {})", env!("AZSDK_RUSTC_VERSION"), OS, ARCH,); let header = match options.application_id { Some(application_id) => format!( "{} azsdk-rust-{}/{} {}", - application_id, - crate_name, - crate_version, - platform_info + application_id, crate_name, crate_version, platform_info ), None => format!( "azsdk-rust-{}/{} {}", - crate_name, - crate_version, - platform_info + crate_name, crate_version, platform_info ), }; - TelemetryPolicy { - header: header, - } + TelemetryPolicy { header: header } } } From 4075f1338fc7b6fb038941a48ef63ab125d825d3 Mon Sep 17 00:00:00 2001 From: Heath Stewart Date: Wed, 2 Jun 2021 15:17:28 -0700 Subject: [PATCH 5/8] Add tests --- sdk/core/build.rs | 6 +- sdk/core/src/policies/telemetry_policy.rs | 88 ++++++++++++++++++++++- 2 files changed, 90 insertions(+), 4 deletions(-) diff --git a/sdk/core/build.rs b/sdk/core/build.rs index 6ad24b1f83..39b42daeb2 100644 --- a/sdk/core/build.rs +++ b/sdk/core/build.rs @@ -1,5 +1,9 @@ use rustc_version::version; fn main() { - println!("cargo:rustc-env=AZSDK_RUSTC_VERSION={}", version().unwrap()); + let version = match version() { + Ok(version) => version.to_string(), + Err(_) => "unknown".to_string(), + }; + println!("cargo:rustc-env=AZSDK_RUSTC_VERSION={}", version); } diff --git a/sdk/core/src/policies/telemetry_policy.rs b/sdk/core/src/policies/telemetry_policy.rs index 89441f8969..7ba6e8a2a4 100644 --- a/sdk/core/src/policies/telemetry_policy.rs +++ b/sdk/core/src/policies/telemetry_policy.rs @@ -23,9 +23,15 @@ pub struct TelemetryPolicy { impl TelemetryPolicy { pub fn new(options: TelemetryOptions) -> Self { - let crate_name = env!("CARGO_PKG_NAME"); - let crate_version = env!("CARGO_PKG_VERSION"); - let platform_info = format!("({}; {}; {})", env!("AZSDK_RUSTC_VERSION"), OS, ARCH,); + Self::with_environment::(options) + } + + fn with_environment(options: TelemetryOptions) -> Self { + const UNKNOWN: &'static str = "unknown"; + let crate_name = T::crate_name().unwrap_or(UNKNOWN); + let crate_version = T::crate_version().unwrap_or(UNKNOWN); + let rustc_version = T::rustc_version().unwrap_or(UNKNOWN); + let platform_info = format!("({}; {}; {})", rustc_version, OS, ARCH,); let header = match options.application_id { Some(application_id) => format!( "{} azsdk-rust-{}/{} {}", @@ -47,6 +53,23 @@ impl Default for TelemetryPolicy { } } +trait Environment { + fn crate_name() -> Option<&'static str> { + option_env!("CARGO_PKG_NAME") + } + + fn crate_version() -> Option<&'static str> { + option_env!("CARGO_PKG_VERSION") + } + + fn rustc_version() -> Option<&'static str> { + option_env!("AZSDK_RUSTC_VERSION") + } +} + +struct Env; +impl Environment for Env {} + #[async_trait::async_trait] impl Policy for TelemetryPolicy { async fn send( @@ -62,3 +85,62 @@ impl Policy for TelemetryPolicy { next[0].send(ctx, request, &next[1..]).await } } + +#[cfg(test)] +mod test { + use super::*; + + // tests assume cargo + rustc + const CRATE_NAME: &'static str = env!("CARGO_PKG_NAME"); + const CRATE_VERSION: &'static str = env!("CARGO_PKG_VERSION"); + const RUSTC_VERSION: &'static str = env!("AZSDK_RUSTC_VERSION"); + + struct EmptyEnv; + impl Environment for EmptyEnv { + fn crate_name() -> Option<&'static str> { + None + } + + fn crate_version() -> Option<&'static str> { + None + } + + fn rustc_version() -> Option<&'static str> { + None + } + } + + #[test] + fn test_default() { + let policy = TelemetryPolicy::default(); + assert_eq!( + policy.header, + format!( + "azsdk-rust-{}/{} ({}; {}; {})", + CRATE_NAME, CRATE_VERSION, RUSTC_VERSION, OS, ARCH + ) + ); + } + + #[test] + fn test_with_application_id() { + let options = TelemetryOptions::new(Some("test".to_string())); + let policy = TelemetryPolicy::new(options); + assert_eq!( + policy.header, + format!( + "test azsdk-rust-{}/{} ({}; {}; {})", + CRATE_NAME, CRATE_VERSION, RUSTC_VERSION, OS, ARCH + ) + ); + } + + #[test] + fn test_missing_env() { + let policy = TelemetryPolicy::with_environment::(TelemetryOptions::default()); + assert_eq!( + policy.header, + format!("azsdk-rust-unknown/unknown (unknown; {}; {})", OS, ARCH) + ) + } +} From a6e592de5b19f3afb501b74f1aed596d83c27163 Mon Sep 17 00:00:00 2001 From: Heath Stewart Date: Thu, 3 Jun 2021 16:59:07 -0700 Subject: [PATCH 6/8] Change how pipelines are created Also introduces base ClientOptions, which all our other SDKs have. --- sdk/core/src/client_options.rs | 12 ++ sdk/core/src/lib.rs | 2 + sdk/core/src/pipeline.rs | 29 +++-- sdk/core/src/policies/telemetry_policy.rs | 127 ++++++++++------------ sdk/cosmos/src/clients/cosmos_client.rs | 6 + 5 files changed, 96 insertions(+), 80 deletions(-) create mode 100644 sdk/core/src/client_options.rs diff --git a/sdk/core/src/client_options.rs b/sdk/core/src/client_options.rs new file mode 100644 index 0000000000..041a2bd171 --- /dev/null +++ b/sdk/core/src/client_options.rs @@ -0,0 +1,12 @@ +use crate::policies::{Policy, TelemetryOptions}; +use std::sync::Arc; + +/// Options passed clients to customer policies, telemetry, etc. +#[derive(Clone, Debug, Default)] +pub struct ClientOptions { + // TODO: Expose retry options and transport overrides. + pub per_call_policies: Vec>, + pub per_retry_policies: Vec>, + + pub telemetry: TelemetryOptions, +} diff --git a/sdk/core/src/lib.rs b/sdk/core/src/lib.rs index 8c69e6473e..43adbfe89b 100644 --- a/sdk/core/src/lib.rs +++ b/sdk/core/src/lib.rs @@ -10,6 +10,7 @@ extern crate serde_derive; mod macros; mod bytes_stream; +pub mod client_options; mod constants; mod context; mod errors; @@ -35,6 +36,7 @@ use std::fmt::Debug; use uuid::Uuid; pub use bytes_stream::*; +pub use client_options::ClientOptions; pub use constants::*; pub use context::Context; pub use errors::*; diff --git a/sdk/core/src/pipeline.rs b/sdk/core/src/pipeline.rs index 1aa0a294e6..c1142a0497 100644 --- a/sdk/core/src/pipeline.rs +++ b/sdk/core/src/pipeline.rs @@ -1,17 +1,20 @@ use crate::policies::{Policy, PolicyResult, TelemetryPolicy}; -use crate::{Context, Request, Response}; +use crate::{ClientOptions, Context, Request, Response}; use std::sync::Arc; /// Execution pipeline. /// /// A pipeline follows a precise flow: /// -/// 1. Per call policies are executed. Per call policies can fail and bail out of the pipeline +/// 1. Client library-specified per-call policies are executed. Per-call policies can fail and bail out of the pipeline /// immediately. -/// 2. Retry policy. It allows to re-execute the following policies. -/// 3. Per retry policies. Per retry polices are always executed at least once but are re-executed +/// 2. User-specified per-call policies are executed. +/// 3. Telemetry policy. +/// 4. Retry policy. It allows to re-execute the following policies. +/// 5. Client library-specified per-retry policies. Per-retry polices are always executed at least once but are re-executed /// in case of retries. -/// 4. Transport policy. Transport policy is always the last policy and is the policy that +/// 6. User-specified per-retry policies are executed. +/// 7. Transport policy. Transport policy is always the last policy and is the policy that /// actually constructs the `Response` to be passed up the pipeline. /// /// A pipeline is immutable. In other words a policy can either succeed and call the following @@ -24,21 +27,29 @@ pub struct Pipeline { } impl Pipeline { + /// Creates a new pipeline given the client library crate name and version, + /// alone with user-specified and client library-specified policies. + /// + /// Crates can simply pass `option_env!("CARGO_PKG_NAME")` and `option_env!("CARGO_PKG_VERSION")` for the + /// `crate_name` and `crate_version` arguments respectively. pub fn new( + crate_name: Option<&'static str>, + crate_version: Option<&'static str>, + options: &ClientOptions, per_call_policies: Vec>, retry: Arc, per_retry_policies: Vec>, transport_policy: Arc, ) -> Self { let mut pipeline: Vec> = - Vec::with_capacity(per_call_policies.len() + per_retry_policies.len() + 3); - - // TODO: Create pipeline from ClientOptions which should contain user-specified policies + client-added policies. - pipeline.push(Arc::new(TelemetryPolicy::default())); + Vec::with_capacity(options.per_call_policies.len() + per_call_policies.len() + options.per_retry_policies.len() + per_retry_policies.len() + 3); pipeline.extend_from_slice(&per_call_policies); + pipeline.extend_from_slice(&options.per_call_policies); + pipeline.push(Arc::new(TelemetryPolicy::new(crate_name, crate_version, &options.telemetry))); pipeline.push(retry); pipeline.extend_from_slice(&per_retry_policies); + pipeline.extend_from_slice(&options.per_retry_policies); pipeline.push(transport_policy); Self { pipeline } diff --git a/sdk/core/src/policies/telemetry_policy.rs b/sdk/core/src/policies/telemetry_policy.rs index 7ba6e8a2a4..5562f28c5e 100644 --- a/sdk/core/src/policies/telemetry_policy.rs +++ b/sdk/core/src/policies/telemetry_policy.rs @@ -7,13 +7,7 @@ use std::sync::Arc; #[derive(Clone, Debug, Default)] pub struct TelemetryOptions { - application_id: Option, -} - -impl TelemetryOptions { - pub fn new(application_id: Option) -> Self { - Self { application_id } - } + pub application_id: Option, } #[derive(Clone, Debug)] @@ -21,18 +15,44 @@ pub struct TelemetryPolicy { header: String, } -impl TelemetryPolicy { - pub fn new(options: TelemetryOptions) -> Self { - Self::with_environment::(options) +/// Sets the User-Agent header with useful information in a typical format for Azure SDKs. +/// +/// Client libraries should create a `TelemetryPolicy` using `option_env!()` like so: +/// ``` +/// use azure_core::policies::{TelemetryOptions, TelemetryPolicy}; +/// let policy = TelemetryPolicy::new(option_env!("CARGO_PKG_NAME"), option_env!("CARGO_PKG_VERSION"), &TelemetryOptions::default()); +/// ``` +impl<'a> TelemetryPolicy { + pub fn new( + crate_name: Option<&'a str>, + crate_version: Option<&'a str>, + options: &TelemetryOptions, + ) -> Self { + Self::new_with_rustc_version( + crate_name, + crate_version, + option_env!("AZSDK_RUSTC_VERSION"), + options, + ) } - fn with_environment(options: TelemetryOptions) -> Self { + fn new_with_rustc_version( + crate_name: Option<&'a str>, + crate_version: Option<&'a str>, + rustc_version: Option<&'static str>, + options: &TelemetryOptions, + ) -> Self { const UNKNOWN: &'static str = "unknown"; - let crate_name = T::crate_name().unwrap_or(UNKNOWN); - let crate_version = T::crate_version().unwrap_or(UNKNOWN); - let rustc_version = T::rustc_version().unwrap_or(UNKNOWN); + let mut crate_name = crate_name.unwrap_or(UNKNOWN); + let crate_version = crate_version.unwrap_or(UNKNOWN); + let rustc_version = rustc_version.unwrap_or(UNKNOWN); let platform_info = format!("({}; {}; {})", rustc_version, OS, ARCH,); - let header = match options.application_id { + + if let Some(name) = crate_name.strip_prefix("azure_") { + crate_name = name; + } + + let header = match &options.application_id { Some(application_id) => format!( "{} azsdk-rust-{}/{} {}", application_id, crate_name, crate_version, platform_info @@ -47,29 +67,6 @@ impl TelemetryPolicy { } } -impl Default for TelemetryPolicy { - fn default() -> Self { - TelemetryPolicy::new(TelemetryOptions::default()) - } -} - -trait Environment { - fn crate_name() -> Option<&'static str> { - option_env!("CARGO_PKG_NAME") - } - - fn crate_version() -> Option<&'static str> { - option_env!("CARGO_PKG_VERSION") - } - - fn rustc_version() -> Option<&'static str> { - option_env!("AZSDK_RUSTC_VERSION") - } -} - -struct Env; -impl Environment for Env {} - #[async_trait::async_trait] impl Policy for TelemetryPolicy { async fn send( @@ -90,54 +87,42 @@ impl Policy for TelemetryPolicy { mod test { use super::*; - // tests assume cargo + rustc - const CRATE_NAME: &'static str = env!("CARGO_PKG_NAME"); - const CRATE_VERSION: &'static str = env!("CARGO_PKG_VERSION"); - const RUSTC_VERSION: &'static str = env!("AZSDK_RUSTC_VERSION"); - - struct EmptyEnv; - impl Environment for EmptyEnv { - fn crate_name() -> Option<&'static str> { - None - } - - fn crate_version() -> Option<&'static str> { - None - } - - fn rustc_version() -> Option<&'static str> { - None - } - } - #[test] - fn test_default() { - let policy = TelemetryPolicy::default(); + fn test_without_application_id() { + let policy = TelemetryPolicy::new_with_rustc_version( + Some("azure_test"), // Tests that "azure_" is removed. + Some("1.2.3"), + Some("4.5.6"), + &TelemetryOptions::default(), + ); assert_eq!( policy.header, - format!( - "azsdk-rust-{}/{} ({}; {}; {})", - CRATE_NAME, CRATE_VERSION, RUSTC_VERSION, OS, ARCH - ) + format!("azsdk-rust-test/1.2.3 (4.5.6; {}; {})", OS, ARCH) ); } #[test] fn test_with_application_id() { - let options = TelemetryOptions::new(Some("test".to_string())); - let policy = TelemetryPolicy::new(options); + let options = TelemetryOptions { + application_id: Some("my_app".to_string()), + }; + let policy = TelemetryPolicy::new_with_rustc_version( + Some("test"), + Some("1.2.3"), + Some("4.5.6"), + &options, + ); assert_eq!( policy.header, - format!( - "test azsdk-rust-{}/{} ({}; {}; {})", - CRATE_NAME, CRATE_VERSION, RUSTC_VERSION, OS, ARCH - ) + format!("my_app azsdk-rust-test/1.2.3 (4.5.6; {}; {})", OS, ARCH) ); } #[test] fn test_missing_env() { - let policy = TelemetryPolicy::with_environment::(TelemetryOptions::default()); + // Would simulate if option_env!("CARGO_PKG_NAME"), for example, returned None. + let policy = + TelemetryPolicy::new_with_rustc_version(None, None, None, &TelemetryOptions::default()); assert_eq!( policy.header, format!("azsdk-rust-unknown/unknown (unknown; {}; {})", OS, ARCH) diff --git a/sdk/cosmos/src/clients/cosmos_client.rs b/sdk/cosmos/src/clients/cosmos_client.rs index 245adbe44c..1807cfa005 100644 --- a/sdk/cosmos/src/clients/cosmos_client.rs +++ b/sdk/cosmos/src/clients/cosmos_client.rs @@ -4,6 +4,7 @@ use crate::resources::ResourceType; use crate::{headers::*, CosmosError}; use crate::{requests, ReadonlyString}; +use azure_core::client_options::ClientOptions; use azure_core::pipeline::Pipeline; use azure_core::policies::{LinearRetryPolicy, Policy, TransportOptions, TransportPolicy}; use azure_core::Context; @@ -32,6 +33,7 @@ pub struct CosmosClient { } /// TODO pub struct CosmosOptions { + options: ClientOptions, retry: Arc, transport: TransportOptions, } @@ -40,6 +42,7 @@ impl CosmosOptions { /// TODO pub fn with_client(client: Arc) -> Self { Self { + options: ClientOptions::default(), retry: Arc::new(LinearRetryPolicy::default()), // this defaults to linear backoff transport: TransportOptions::new(client), } @@ -52,6 +55,9 @@ fn new_pipeline_from_options(options: CosmosOptions) -> Pipeline { let per_retry_policies = Vec::new(); let transport_policy = TransportPolicy::new(options.transport); Pipeline::new( + option_env!("CARGO_PKG_NAME"), + option_env!("CARGO_PKG_VERSION"), + &options.options, per_call_policies, options.retry, per_retry_policies, From 0a7003616231a8bfca4392054ea6f99178d88144 Mon Sep 17 00:00:00 2001 From: Heath Stewart Date: Fri, 4 Jun 2021 09:48:59 -0700 Subject: [PATCH 7/8] Format the code again --- sdk/core/src/pipeline.rs | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/sdk/core/src/pipeline.rs b/sdk/core/src/pipeline.rs index c1142a0497..fc6b73404e 100644 --- a/sdk/core/src/pipeline.rs +++ b/sdk/core/src/pipeline.rs @@ -41,12 +41,21 @@ impl Pipeline { per_retry_policies: Vec>, transport_policy: Arc, ) -> Self { - let mut pipeline: Vec> = - Vec::with_capacity(options.per_call_policies.len() + per_call_policies.len() + options.per_retry_policies.len() + per_retry_policies.len() + 3); + let mut pipeline: Vec> = Vec::with_capacity( + options.per_call_policies.len() + + per_call_policies.len() + + options.per_retry_policies.len() + + per_retry_policies.len() + + 3, + ); pipeline.extend_from_slice(&per_call_policies); pipeline.extend_from_slice(&options.per_call_policies); - pipeline.push(Arc::new(TelemetryPolicy::new(crate_name, crate_version, &options.telemetry))); + pipeline.push(Arc::new(TelemetryPolicy::new( + crate_name, + crate_version, + &options.telemetry, + ))); pipeline.push(retry); pipeline.extend_from_slice(&per_retry_policies); pipeline.extend_from_slice(&options.per_retry_policies); From 045f78fad1308619a40486c7b713e43d679cf9bf Mon Sep 17 00:00:00 2001 From: Heath Stewart Date: Fri, 4 Jun 2021 22:39:27 -0700 Subject: [PATCH 8/8] Resolve PR feedback --- sdk/core/build.rs | 2 +- sdk/core/src/policies/telemetry_policy.rs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/sdk/core/build.rs b/sdk/core/build.rs index 39b42daeb2..db4187ddc0 100644 --- a/sdk/core/build.rs +++ b/sdk/core/build.rs @@ -3,7 +3,7 @@ use rustc_version::version; fn main() { let version = match version() { Ok(version) => version.to_string(), - Err(_) => "unknown".to_string(), + Err(_) => "unknown".to_owned(), }; println!("cargo:rustc-env=AZSDK_RUSTC_VERSION={}", version); } diff --git a/sdk/core/src/policies/telemetry_policy.rs b/sdk/core/src/policies/telemetry_policy.rs index 5562f28c5e..5d9d0eff03 100644 --- a/sdk/core/src/policies/telemetry_policy.rs +++ b/sdk/core/src/policies/telemetry_policy.rs @@ -39,7 +39,7 @@ impl<'a> TelemetryPolicy { fn new_with_rustc_version( crate_name: Option<&'a str>, crate_version: Option<&'a str>, - rustc_version: Option<&'static str>, + rustc_version: Option<&'a str>, options: &TelemetryOptions, ) -> Self { const UNKNOWN: &'static str = "unknown"; @@ -77,7 +77,7 @@ impl Policy for TelemetryPolicy { ) -> PolicyResult { request .headers_mut() - .insert(USER_AGENT, HeaderValue::from_str(&self.header).unwrap()); + .insert(USER_AGENT, HeaderValue::from_str(&self.header)?); next[0].send(ctx, request, &next[1..]).await }