-
Notifications
You must be signed in to change notification settings - Fork 21
fix(libdd-common): Add fallback logic for resolving Azure Functions instance name [SVLS-8931] #2077
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
Changes from all commits
bb64700
2af348b
8d0b58e
3f3fb3e
ff8ee41
869d570
9e5bad5
aeffe22
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -11,16 +11,19 @@ const WEBSITE_SITE_NAME: &str = "WEBSITE_SITE_NAME"; | |
| const WEBSITE_RESOURCE_GROUP: &str = "WEBSITE_RESOURCE_GROUP"; | ||
| const SITE_EXTENSION_VERSION: &str = "DD_AAS_DOTNET_EXTENSION_VERSION"; | ||
| const WEBSITE_OS: &str = "WEBSITE_OS"; | ||
| const INSTANCE_NAME: &str = "COMPUTERNAME"; | ||
| const COMPUTERNAME: &str = "COMPUTERNAME"; | ||
| const CONTAINER_NAME: &str = "CONTAINER_NAME"; | ||
| const WEBSITE_POD_NAME: &str = "WEBSITE_POD_NAME"; | ||
| const INSTANCE_ID: &str = "WEBSITE_INSTANCE_ID"; | ||
| const SERVICE_CONTEXT: &str = "DD_AZURE_APP_SERVICES"; | ||
| const FUNCTIONS_WORKER_RUNTIME: &str = "FUNCTIONS_WORKER_RUNTIME"; | ||
| const FUNCTIONS_WORKER_RUNTIME_VERSION: &str = "FUNCTIONS_WORKER_RUNTIME_VERSION"; | ||
| const FUNCTIONS_EXTENSION_VERSION: &str = "FUNCTIONS_EXTENSION_VERSION"; | ||
| const DD_AZURE_RESOURCE_GROUP: &str = "DD_AZURE_RESOURCE_GROUP"; | ||
| const WEBSITE_SKU: &str = "WEBSITE_SKU"; | ||
| pub const WEBSITE_SKU: &str = "WEBSITE_SKU"; | ||
| pub const REGION_NAME: &str = "REGION_NAME"; | ||
|
|
||
| const UNKNOWN_VALUE: &str = "unknown"; | ||
| pub const UNKNOWN_VALUE: &str = "unknown"; | ||
|
|
||
| enum AzureContext { | ||
| AzureFunctions, | ||
|
|
@@ -63,7 +66,9 @@ const AAS_VAR_NAMES: &[&str] = &[ | |
| WEBSITE_RESOURCE_GROUP, | ||
| SITE_EXTENSION_VERSION, | ||
| WEBSITE_OS, | ||
| INSTANCE_NAME, | ||
| COMPUTERNAME, | ||
| CONTAINER_NAME, | ||
| WEBSITE_POD_NAME, | ||
| INSTANCE_ID, | ||
| SERVICE_CONTEXT, | ||
| FUNCTIONS_WORKER_RUNTIME, | ||
|
|
@@ -223,12 +228,14 @@ impl AzureMetadata { | |
| _ => ("app".to_owned(), "app".to_owned()), | ||
| }; | ||
|
|
||
| let website_sku = query.get_var(WEBSITE_SKU); | ||
|
|
||
| let resource_group = query | ||
| .get_var(DD_AZURE_RESOURCE_GROUP) | ||
| .or_else(|| query.get_var(WEBSITE_RESOURCE_GROUP)) | ||
| .or_else(|| { | ||
| // Check if we're in flex consumption plan first | ||
| match query.get_var(WEBSITE_SKU).as_deref() { | ||
| match website_sku.as_deref() { | ||
| Some("FlexConsumption") => None, | ||
| /* Flex Consumption plans need the `DD_AZURE_RESOURCE_GROUP` env var. If this | ||
| * logic ever changes, update the logic in | ||
|
|
@@ -247,7 +254,16 @@ impl AzureMetadata { | |
| let operating_system = query | ||
| .get_var(WEBSITE_OS) | ||
| .unwrap_or(std::env::consts::OS.to_string()); | ||
| let instance_name = query.get_var(INSTANCE_NAME); | ||
|
|
||
| let computer_name = query.get_var(COMPUTERNAME); | ||
| let pod_name = query.get_var(WEBSITE_POD_NAME); | ||
| let container_name = query.get_var(CONTAINER_NAME); | ||
| let instance_name = resolve_instance_name( | ||
| computer_name.as_deref(), | ||
| pod_name.as_deref(), | ||
| container_name.as_deref(), | ||
| ); | ||
|
|
||
| let instance_id = query.get_var(INSTANCE_ID); | ||
|
|
||
| let runtime = query.get_var(FUNCTIONS_WORKER_RUNTIME); | ||
|
|
@@ -394,6 +410,22 @@ impl AzureMetadata { | |
| } | ||
| } | ||
|
|
||
| /// Resolves the instance name to match the Azure integration metric's `instance` tag. | ||
| fn resolve_instance_name( | ||
| computer_name: Option<&str>, | ||
| pod_name: Option<&str>, | ||
| container_name: Option<&str>, | ||
| ) -> Option<String> { | ||
| fn non_empty(s: Option<&str>) -> Option<&str> { | ||
| s.map(|v| v.trim()).filter(|v| !v.is_empty()) | ||
| } | ||
|
Comment on lines
+419
to
+421
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
|
|
||
| non_empty(computer_name) | ||
| .or_else(|| non_empty(pod_name)) | ||
| .or_else(|| non_empty(container_name)) | ||
| .map(|s| s.to_string()) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
In Useful? React with 👍 / 👎.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is intentional - the existing Example: Mixed case in existing instance name span attribute: Lowercase in integration metric |
||
| } | ||
|
|
||
| pub static AAS_METADATA: LazyLock<Option<AzureMetadata>> = | ||
| LazyLock::new(|| AzureMetadata::new(RealEnv {})); | ||
|
|
||
|
|
@@ -636,10 +668,10 @@ mod tests { | |
| "00000000-0000-0000-0000-000000000000+flex-EastUSwebspace-Linux", | ||
| ), | ||
| (WEBSITE_SKU, "FlexConsumption"), | ||
| (SERVICE_CONTEXT, "1"), | ||
| (FUNCTIONS_WORKER_RUNTIME, "node"), | ||
| ]); | ||
|
|
||
| let metadata = AzureMetadata::new(mocked_env).unwrap(); | ||
| let metadata = AzureMetadata::new_function(mocked_env).unwrap(); | ||
|
|
||
| assert_eq!(metadata.get_resource_group(), UNKNOWN_VALUE); | ||
| } | ||
|
|
@@ -653,10 +685,10 @@ mod tests { | |
| ), | ||
| (DD_AZURE_RESOURCE_GROUP, "test-flex-rg"), | ||
| (WEBSITE_SKU, "FlexConsumption"), | ||
| (SERVICE_CONTEXT, "1"), | ||
| (FUNCTIONS_WORKER_RUNTIME, "node"), | ||
| ]); | ||
|
|
||
| let metadata = AzureMetadata::new(mocked_env).unwrap(); | ||
| let metadata = AzureMetadata::new_function(mocked_env).unwrap(); | ||
|
|
||
| // Should use the DD_AZURE_RESOURCE_GROUP value instead of extracting from | ||
| // WEBSITE_OWNER_NAME | ||
|
|
@@ -811,7 +843,7 @@ mod tests { | |
| (WEBSITE_RESOURCE_GROUP, expected_resource_group.as_str()), | ||
| (SITE_EXTENSION_VERSION, expected_site_version.as_str()), | ||
| (WEBSITE_OS, expected_operating_system.as_str()), | ||
| (INSTANCE_NAME, expected_instance_name.as_str()), | ||
| (COMPUTERNAME, expected_instance_name.as_str()), | ||
| (INSTANCE_ID, expected_instance_id.as_str()), | ||
| (SERVICE_CONTEXT, "1"), | ||
| ( | ||
|
|
@@ -857,7 +889,7 @@ mod tests { | |
| (WEBSITE_RESOURCE_GROUP, expected_resource_group), | ||
| (SITE_EXTENSION_VERSION, expected_site_version), | ||
| (WEBSITE_OS, expected_operating_system), | ||
| (INSTANCE_NAME, expected_instance_name), | ||
| (COMPUTERNAME, expected_instance_name), | ||
| (INSTANCE_ID, expected_instance_id), | ||
| (SERVICE_CONTEXT, "1"), | ||
| ( | ||
|
|
@@ -929,7 +961,7 @@ mod tests { | |
| (WEBSITE_SITE_NAME, expected_site_name), | ||
| (WEBSITE_RESOURCE_GROUP, expected_resource_group), | ||
| (WEBSITE_OS, expected_operating_system), | ||
| (INSTANCE_NAME, expected_instance_name), | ||
| (COMPUTERNAME, expected_instance_name), | ||
| (INSTANCE_ID, expected_instance_id), | ||
| (SERVICE_CONTEXT, "1"), | ||
| ( | ||
|
|
@@ -1002,4 +1034,68 @@ mod tests { | |
| None | ||
| ); | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_instance_name_computer_name_wins_over_pod_and_container() { | ||
| let mocked_env = MockEnv::new(&[ | ||
| (FUNCTIONS_WORKER_RUNTIME, "node"), | ||
| (COMPUTERNAME, "10-20-30-40"), | ||
| (WEBSITE_POD_NAME, "pod-1"), | ||
| (CONTAINER_NAME, "container-1"), | ||
| ]); | ||
| let metadata = AzureMetadata::new_function(mocked_env).unwrap(); | ||
| assert_eq!(metadata.get_instance_name(), "10-20-30-40"); | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_instance_name_pod_name_preferred_over_container_name() { | ||
| let mocked_env = MockEnv::new(&[ | ||
| (FUNCTIONS_WORKER_RUNTIME, "node"), | ||
| (WEBSITE_POD_NAME, "0--abc-test"), | ||
| (CONTAINER_NAME, "container-1"), | ||
| ]); | ||
| let metadata = AzureMetadata::new_function(mocked_env).unwrap(); | ||
| assert_eq!(metadata.get_instance_name(), "0--abc-test"); | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_instance_name_falls_back_to_container_name() { | ||
| let mocked_env = MockEnv::new(&[ | ||
| (FUNCTIONS_WORKER_RUNTIME, "node"), | ||
| (CONTAINER_NAME, "0--abc-test"), | ||
| ]); | ||
| let metadata = AzureMetadata::new_function(mocked_env).unwrap(); | ||
| assert_eq!(metadata.get_instance_name(), "0--abc-test"); | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_instance_name_empty_string_treated_as_missing() { | ||
| let mocked_env = MockEnv::new(&[ | ||
| (FUNCTIONS_WORKER_RUNTIME, "node"), | ||
| (CONTAINER_NAME, ""), | ||
| (WEBSITE_POD_NAME, ""), | ||
| (COMPUTERNAME, "worker-1"), | ||
| ]); | ||
| let metadata = AzureMetadata::new_function(mocked_env).unwrap(); | ||
| assert_eq!(metadata.get_instance_name(), "worker-1"); | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_instance_name_whitespace_only_treated_as_missing() { | ||
| let mocked_env = MockEnv::new(&[ | ||
| (FUNCTIONS_WORKER_RUNTIME, "node"), | ||
| (CONTAINER_NAME, " "), | ||
| (WEBSITE_POD_NAME, " "), | ||
| (COMPUTERNAME, "worker-2"), | ||
| ]); | ||
| let metadata = AzureMetadata::new_function(mocked_env).unwrap(); | ||
| assert_eq!(metadata.get_instance_name(), "worker-2"); | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_instance_name_no_instance_vars_is_unknown() { | ||
| let mocked_env = MockEnv::new(&[(FUNCTIONS_WORKER_RUNTIME, "node")]); | ||
| let metadata = AzureMetadata::new_function(mocked_env).unwrap(); | ||
| assert_eq!(metadata.get_instance_name(), UNKNOWN_VALUE); | ||
| } | ||
| } | ||


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.
Do these constants need to be public?
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 had received some suggestions on my enhanced metrics PR to use the constants in libdatadog (example 1, example 2, and I can't use them in serverless-components unless they're public