Skip to content

Commit 902b6a5

Browse files
authored
[SVLS-6337] Set resource group correctly for Azure functions on flex consumption plans (#1241)
nit: fix WEBSITE_OWNER_NAME mispelling Check DD_AZURE_RESOURCE_GROUP env variable when getting resource group for flex consumption functions nit: Update panic message ran cargo fmt Changed logic order to check DD_AZURE_RESOURCE_GROUP first Set resource group to unknown instead of panicking nit: fix formatting Use WEBSITE_SKU instead of WEBSITE_OWNER_NAME to check for flex consumption function Add comment to ensure future developers know to update other repos if flex consumption rg detection logic changes Merge branch 'main' into kathie.huang/fix-flex-rg-span-attribute Merge branch 'main' into kathie.huang/fix-flex-rg-span-attribute
1 parent d346130 commit 902b6a5

File tree

1 file changed

+90
-19
lines changed

1 file changed

+90
-19
lines changed

ddcommon/src/azure_app_services.rs

Lines changed: 90 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ use regex::Regex;
55
use std::env;
66
use std::sync::LazyLock;
77

8-
const WEBSITE_ONWER_NAME: &str = "WEBSITE_OWNER_NAME";
8+
const WEBSITE_OWNER_NAME: &str = "WEBSITE_OWNER_NAME";
99
const WEBSITE_SITE_NAME: &str = "WEBSITE_SITE_NAME";
1010
const WEBSITE_RESOURCE_GROUP: &str = "WEBSITE_RESOURCE_GROUP";
1111
const SITE_EXTENSION_VERSION: &str = "DD_AAS_DOTNET_EXTENSION_VERSION";
@@ -16,6 +16,8 @@ const SERVICE_CONTEXT: &str = "DD_AZURE_APP_SERVICES";
1616
const FUNCTIONS_WORKER_RUNTIME: &str = "FUNCTIONS_WORKER_RUNTIME";
1717
const FUNCTIONS_WORKER_RUNTIME_VERSION: &str = "FUNCTIONS_WORKER_RUNTIME_VERSION";
1818
const FUNCTIONS_EXTENSION_VERSION: &str = "FUNCTIONS_EXTENSION_VERSION";
19+
const DD_AZURE_RESOURCE_GROUP: &str = "DD_AZURE_RESOURCE_GROUP";
20+
const WEBSITE_SKU: &str = "WEBSITE_SKU";
1921

2022
const UNKNOWN_VALUE: &str = "unknown";
2123

@@ -131,7 +133,7 @@ impl AzureMetadata {
131133

132134
fn build_metadata<T: QueryEnv>(query: T) -> Option<Self> {
133135
let subscription_id =
134-
AzureMetadata::extract_subscription_id(query.get_var(WEBSITE_ONWER_NAME));
136+
AzureMetadata::extract_subscription_id(query.get_var(WEBSITE_OWNER_NAME));
135137
let site_name = query.get_var(WEBSITE_SITE_NAME);
136138

137139
let (site_kind, site_type) = match AzureMetadata::get_azure_context(&query) {
@@ -140,8 +142,20 @@ impl AzureMetadata {
140142
};
141143

142144
let resource_group = query
143-
.get_var(WEBSITE_RESOURCE_GROUP)
144-
.or_else(|| AzureMetadata::extract_resource_group(query.get_var(WEBSITE_ONWER_NAME)));
145+
.get_var(DD_AZURE_RESOURCE_GROUP)
146+
.or_else(|| query.get_var(WEBSITE_RESOURCE_GROUP))
147+
.or_else(|| {
148+
// Check if we're in flex consumption plan first
149+
match query.get_var(WEBSITE_SKU).as_deref() {
150+
Some("FlexConsumption") => None,
151+
/* Flex Consumption plans need the `DD_AZURE_RESOURCE_GROUP` env var. If this
152+
* logic ever changes, update the logic in
153+
* `serverless-components/src/datadog-trace-agent` and the serverless compat
154+
* layers accordingly. */
155+
_ => AzureMetadata::extract_resource_group(query.get_var(WEBSITE_OWNER_NAME)),
156+
}
157+
});
158+
145159
let resource_id = AzureMetadata::build_resource_id(
146160
subscription_id.as_ref(),
147161
site_name.as_ref(),
@@ -262,7 +276,7 @@ mod tests {
262276

263277
use indexmap::IndexMap;
264278

265-
use crate::azure_app_services::{QueryEnv, WEBSITE_ONWER_NAME};
279+
use crate::azure_app_services::{QueryEnv, WEBSITE_OWNER_NAME};
266280

267281
use super::*;
268282

@@ -353,7 +367,7 @@ mod tests {
353367

354368
#[test]
355369
fn test_extract_subscription_without_plus_sign() {
356-
let mocked_env = MockEnv::new(&[(WEBSITE_ONWER_NAME, "foo"), (SERVICE_CONTEXT, "1")]);
370+
let mocked_env = MockEnv::new(&[(WEBSITE_OWNER_NAME, "foo"), (SERVICE_CONTEXT, "1")]);
357371

358372
let metadata = AzureMetadata::new(mocked_env).unwrap();
359373

@@ -364,7 +378,7 @@ mod tests {
364378

365379
#[test]
366380
fn test_extract_subscription_with_plus_sign() {
367-
let mocked_env = MockEnv::new(&[(WEBSITE_ONWER_NAME, "foo+bar"), (SERVICE_CONTEXT, "1")]);
381+
let mocked_env = MockEnv::new(&[(WEBSITE_OWNER_NAME, "foo+bar"), (SERVICE_CONTEXT, "1")]);
368382

369383
let metadata = AzureMetadata::new(mocked_env).unwrap();
370384

@@ -374,7 +388,7 @@ mod tests {
374388

375389
#[test]
376390
fn test_extract_subscription_with_empty_string() {
377-
let mocked_env = MockEnv::new(&[(WEBSITE_ONWER_NAME, ""), (SERVICE_CONTEXT, "1")]);
391+
let mocked_env = MockEnv::new(&[(WEBSITE_OWNER_NAME, ""), (SERVICE_CONTEXT, "1")]);
378392

379393
let metadata = AzureMetadata::new(mocked_env).unwrap();
380394

@@ -383,7 +397,7 @@ mod tests {
383397

384398
#[test]
385399
fn test_extract_subscription_with_only_whitespaces() {
386-
let mocked_env = MockEnv::new(&[(WEBSITE_ONWER_NAME, " "), (SERVICE_CONTEXT, "1")]);
400+
let mocked_env = MockEnv::new(&[(WEBSITE_OWNER_NAME, " "), (SERVICE_CONTEXT, "1")]);
387401

388402
let metadata = AzureMetadata::new(mocked_env).unwrap();
389403

@@ -392,7 +406,7 @@ mod tests {
392406

393407
#[test]
394408
fn test_extract_subscription_with_only_plus_sign() {
395-
let mocked_env = MockEnv::new(&[(WEBSITE_ONWER_NAME, "+"), (SERVICE_CONTEXT, "1")]);
409+
let mocked_env = MockEnv::new(&[(WEBSITE_OWNER_NAME, "+"), (SERVICE_CONTEXT, "1")]);
396410

397411
let metadata = AzureMetadata::new(mocked_env).unwrap();
398412

@@ -401,7 +415,7 @@ mod tests {
401415

402416
#[test]
403417
fn test_extract_subscription_with_whitespaces_separated_by_plus() {
404-
let mocked_env = MockEnv::new(&[(WEBSITE_ONWER_NAME, " + "), (SERVICE_CONTEXT, "1")]);
418+
let mocked_env = MockEnv::new(&[(WEBSITE_OWNER_NAME, " + "), (SERVICE_CONTEXT, "1")]);
405419

406420
let metadata = AzureMetadata::new(mocked_env).unwrap();
407421

@@ -410,7 +424,7 @@ mod tests {
410424

411425
#[test]
412426
fn test_extract_subscription_plus_sign_and_other_string() {
413-
let mocked_env = MockEnv::new(&[(WEBSITE_ONWER_NAME, "+other"), (SERVICE_CONTEXT, "1")]);
427+
let mocked_env = MockEnv::new(&[(WEBSITE_OWNER_NAME, "+other"), (SERVICE_CONTEXT, "1")]);
414428

415429
let metadata = AzureMetadata::new(mocked_env).unwrap();
416430

@@ -421,7 +435,7 @@ mod tests {
421435
fn test_extract_resource_group_pattern_match_linux() {
422436
let mocked_env = MockEnv::new(&[
423437
(
424-
WEBSITE_ONWER_NAME,
438+
WEBSITE_OWNER_NAME,
425439
"00000000-0000-0000-0000-000000000000+test-rg-EastUSwebspace-Linux",
426440
),
427441
("FUNCTIONS_WORKER_RUNTIME", "node"),
@@ -439,7 +453,7 @@ mod tests {
439453
fn test_extract_resource_group_pattern_match_windows() {
440454
let mocked_env = MockEnv::new(&[
441455
(
442-
WEBSITE_ONWER_NAME,
456+
WEBSITE_OWNER_NAME,
443457
"00000000-0000-0000-0000-000000000000+test-rg-EastUSwebspace",
444458
),
445459
("FUNCTIONS_WORKER_RUNTIME", "node"),
@@ -456,7 +470,7 @@ mod tests {
456470
#[test]
457471
fn test_extract_resource_group_no_pattern_match() {
458472
let mocked_env = MockEnv::new(&[
459-
(WEBSITE_ONWER_NAME, "foo"),
473+
(WEBSITE_OWNER_NAME, "foo"),
460474
(FUNCTIONS_WORKER_RUNTIME, "node"),
461475
(FUNCTIONS_EXTENSION_VERSION, "~4"),
462476
]);
@@ -471,10 +485,11 @@ mod tests {
471485
let mocked_env = MockEnv::new(&[
472486
(WEBSITE_RESOURCE_GROUP, "test-rg-env-var"),
473487
(
474-
WEBSITE_ONWER_NAME,
488+
WEBSITE_OWNER_NAME,
475489
"00000000-0000-0000-0000-000000000000+test-rg-EastUSwebspace-Linux",
476490
),
477491
(SERVICE_CONTEXT, "1"),
492+
(WEBSITE_SKU, "ElasticPremium"),
478493
]);
479494

480495
let metadata = AzureMetadata::new(mocked_env).unwrap();
@@ -484,10 +499,66 @@ mod tests {
484499
assert_eq!(metadata.get_resource_group(), expected_resource_group);
485500
}
486501

502+
#[test]
503+
fn test_flex_consumption_resource_group_is_none_without_dd_azure_resource_group() {
504+
let mocked_env = MockEnv::new(&[
505+
(
506+
WEBSITE_OWNER_NAME,
507+
"00000000-0000-0000-0000-000000000000+flex-EastUSwebspace-Linux",
508+
),
509+
(WEBSITE_SKU, "FlexConsumption"),
510+
(SERVICE_CONTEXT, "1"),
511+
]);
512+
513+
let metadata = AzureMetadata::new(mocked_env).unwrap();
514+
515+
assert_eq!(metadata.get_resource_group(), UNKNOWN_VALUE);
516+
}
517+
518+
#[test]
519+
fn test_flex_consumption_uses_dd_azure_resource_group() {
520+
let mocked_env = MockEnv::new(&[
521+
(
522+
WEBSITE_OWNER_NAME,
523+
"00000000-0000-0000-0000-000000000000+flex-EastUSwebspace-Linux",
524+
),
525+
(DD_AZURE_RESOURCE_GROUP, "test-flex-rg"),
526+
(WEBSITE_SKU, "FlexConsumption"),
527+
(SERVICE_CONTEXT, "1"),
528+
]);
529+
530+
let metadata = AzureMetadata::new(mocked_env).unwrap();
531+
532+
// Should use the DD_AZURE_RESOURCE_GROUP value instead of extracting from
533+
// WEBSITE_OWNER_NAME
534+
assert_eq!(metadata.get_resource_group(), "test-flex-rg");
535+
}
536+
537+
#[test]
538+
fn test_dd_azure_resource_group_has_highest_priority() {
539+
let mocked_env = MockEnv::new(&[
540+
(WEBSITE_RESOURCE_GROUP, "test-rg-env-var"),
541+
(
542+
WEBSITE_OWNER_NAME,
543+
"00000000-0000-0000-0000-000000000000+test-rg-EastUSwebspace-Linux",
544+
),
545+
(DD_AZURE_RESOURCE_GROUP, "dd-azure-rg-override"),
546+
(SERVICE_CONTEXT, "1"),
547+
]);
548+
549+
let metadata = AzureMetadata::new(mocked_env).unwrap();
550+
551+
// DD_AZURE_RESOURCE_GROUP should have highest priority over WEBSITE_RESOURCE_GROUP and
552+
// WEBSITE_OWNER_NAME
553+
let expected_resource_group = "dd-azure-rg-override";
554+
555+
assert_eq!(metadata.get_resource_group(), expected_resource_group);
556+
}
557+
487558
#[test]
488559
fn test_build_resource_id() {
489560
let mocked_env = MockEnv::new(&[
490-
(WEBSITE_ONWER_NAME, "foo"),
561+
(WEBSITE_OWNER_NAME, "foo"),
491562
(WEBSITE_SITE_NAME, "my_website"),
492563
(WEBSITE_RESOURCE_GROUP, "resource_group"),
493564
(SERVICE_CONTEXT, "1"),
@@ -517,7 +588,7 @@ mod tests {
517588
#[test]
518589
fn test_build_resource_id_with_missing_site_name() {
519590
let mocked_env = MockEnv::new(&[
520-
(WEBSITE_ONWER_NAME, "foo"),
591+
(WEBSITE_OWNER_NAME, "foo"),
521592
(WEBSITE_RESOURCE_GROUP, "resource_group"),
522593
(SERVICE_CONTEXT, "1"),
523594
]);
@@ -530,7 +601,7 @@ mod tests {
530601
#[test]
531602
fn test_build_resource_id_with_missing_resource_group() {
532603
let mocked_env = MockEnv::new(&[
533-
(WEBSITE_ONWER_NAME, "foo"),
604+
(WEBSITE_OWNER_NAME, "foo"),
534605
(WEBSITE_SITE_NAME, "my_website"),
535606
(SERVICE_CONTEXT, "1"),
536607
]);

0 commit comments

Comments
 (0)