-
Notifications
You must be signed in to change notification settings - Fork 264
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add TelemetryPolicy #210
Merged
Merged
Add TelemetryPolicy #210
Changes from all commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
2d1d028
Add TelemetryPolicy
heaths a7a2589
Satisfy `cargo fmt`
heaths d53f233
Resolve PR feedback
heaths 250c336
Satisfy `cargo fmt`
heaths 4075f13
Add tests
heaths a6e592d
Change how pipelines are created
heaths 0a70036
Format the code again
heaths 045f78f
Resolve PR feedback
heaths File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
use rustc_version::version; | ||
|
||
fn main() { | ||
let version = match version() { | ||
Ok(version) => version.to_string(), | ||
Err(_) => "unknown".to_owned(), | ||
}; | ||
println!("cargo:rustc-env=AZSDK_RUSTC_VERSION={}", version); | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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<Arc<dyn Policy>>, | ||
pub per_retry_policies: Vec<Arc<dyn Policy>>, | ||
|
||
pub telemetry: TelemetryOptions, | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,131 @@ | ||
use crate::policies::{Policy, PolicyResult}; | ||
use crate::{Context, Request, Response}; | ||
|
||
use http::{header::USER_AGENT, HeaderValue}; | ||
use std::env::consts::{ARCH, OS}; | ||
use std::sync::Arc; | ||
|
||
#[derive(Clone, Debug, Default)] | ||
pub struct TelemetryOptions { | ||
pub application_id: Option<String>, | ||
} | ||
|
||
#[derive(Clone, Debug)] | ||
pub struct TelemetryPolicy { | ||
header: String, | ||
} | ||
|
||
/// 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 new_with_rustc_version( | ||
crate_name: Option<&'a str>, | ||
crate_version: Option<&'a str>, | ||
rustc_version: Option<&'a str>, | ||
options: &TelemetryOptions, | ||
) -> Self { | ||
const UNKNOWN: &'static str = "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,); | ||
|
||
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 | ||
), | ||
None => format!( | ||
"azsdk-rust-{}/{} {}", | ||
crate_name, crate_version, platform_info | ||
), | ||
}; | ||
|
||
TelemetryPolicy { header: header } | ||
} | ||
} | ||
|
||
#[async_trait::async_trait] | ||
impl Policy for TelemetryPolicy { | ||
async fn send( | ||
&self, | ||
ctx: &mut Context, | ||
request: &mut Request, | ||
next: &[Arc<dyn Policy>], | ||
) -> PolicyResult<Response> { | ||
request | ||
.headers_mut() | ||
.insert(USER_AGENT, HeaderValue::from_str(&self.header)?); | ||
|
||
next[0].send(ctx, request, &next[1..]).await | ||
} | ||
} | ||
|
||
#[cfg(test)] | ||
mod test { | ||
use super::*; | ||
|
||
#[test] | ||
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-test/1.2.3 (4.5.6; {}; {})", OS, ARCH) | ||
heaths marked this conversation as resolved.
Show resolved
Hide resolved
|
||
); | ||
} | ||
|
||
#[test] | ||
fn test_with_application_id() { | ||
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!("my_app azsdk-rust-test/1.2.3 (4.5.6; {}; {})", OS, ARCH) | ||
); | ||
} | ||
|
||
#[test] | ||
fn test_missing_env() { | ||
// 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) | ||
) | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The number of arguments here is getting pretty long. What speaks about
crate_name
andcrate_version
being inside ofClientOptions
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is temporary. For example, the retry policy needs to be removed in favor of retry options - higher-level abstracts that can pick the right policy (then keep those internal). But I can't use CARGO_* env vars from code in azure_core or they will always end up being "core" and azure_core's version. All our other SDKs pass this information in unless it can be retrieved automatically e.g. in .NET by checking for an assembly-level attribute.
It's probably worth using a builder at this point, which is what most of our other SDKs do. But that's something I want to tackle in a different PR. This PR is focused on the TelemetryPolicy specifically.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I should also mention that most of the parameters you see here will actually be part of
ClientOptions
. In other languages, that's where we consistently provide telemetry, diagnostics (logging, mostly), retry options, and even the transport itself (optional override). For .NET, for example, since it can use reflection to get the name and version, it's only 3 parameters: options, and client-provided per-call and per-retry policies.