Skip to content
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

[msal-node]: Add regional authorities telemetry #3662

Merged
merged 42 commits into from
Jul 7, 2021
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
42 commits
Select commit Hold shift + click to select a range
aeeeac8
Change files
samuelkubai May 20, 2021
ddab044
feat: update the confidential and public client test with region disc…
samuelkubai May 20, 2021
962bf0c
fix: update the server telemetry manager test
samuelkubai May 20, 2021
7c26215
Merge branch 'dev' into regional-authorities-telemetry
samuelkubai May 20, 2021
67697ac
fix: update sever telemetry manager test with jest command
samuelkubai May 20, 2021
42b33d2
Merge branch 'dev' into regional-authorities-telemetry
samuelkubai May 20, 2021
312977d
feat: upgraded the telemetry manager
samuelkubai May 26, 2021
01b95a8
fix: client credentials sample
samuelkubai May 26, 2021
91f9633
feat: refactor the telemetry implementation
samuelkubai Jun 2, 2021
1bfa82c
fix: update the server telemetry documentation
samuelkubai Jun 3, 2021
7e91518
Merge branch 'dev' into regional-authorities-telemetry
samuelkubai Jun 3, 2021
dfe03ab
Merge branch 'regional-authorities-telemetry' of github.com:AzureAD/m…
samuelkubai Jun 3, 2021
cb67535
Update change/@azure-msal-common-3150477f-929f-4013-8176-77b1a63f8912…
samuelkubai Jun 23, 2021
93ffc8c
refactor: replace the enum values with the expected header values
samuelkubai Jun 23, 2021
4ac4c4c
Update change/@azure-msal-node-f63abd0d-4eab-4485-ba86-ca75799c6065.json
samuelkubai Jun 23, 2021
34c3812
Merge branch 'dev' into regional-authorities-telemetry
samuelkubai Jun 23, 2021
dc183e9
Merge branch 'regional-authorities-telemetry' of github.com:AzureAD/m…
samuelkubai Jun 25, 2021
d5e5f19
Merge branch 'dev' into regional-authorities-telemetry
samuelkubai Jun 25, 2021
f62a571
tests: update the expected telemetry version to 5
samuelkubai Jun 28, 2021
98ebdb0
Merge branch 'regional-authorities-telemetry' of github.com:AzureAD/m…
samuelkubai Jun 28, 2021
a73b13f
test: upgrade the authority definition in the tests
samuelkubai Jun 28, 2021
165e1e9
Merge branch 'dev' into regional-authorities-telemetry
sameerag Jun 28, 2021
9caf09e
Update lib/msal-common/src/telemetry/server/ServerTelemetryManager.ts
sameerag Jun 28, 2021
2ba6f8c
Merge branch 'dev' into regional-authorities-telemetry
sameerag Jun 28, 2021
4cb59e6
refactor: improve the readability of server telemetry on the OBO flow…
samuelkubai Jun 29, 2021
ab0e546
Merge branch 'dev' into regional-authorities-telemetry
samuelkubai Jun 29, 2021
1cb3dad
feat: implemented the changes from the pull request
samuelkubai Jun 29, 2021
02957cb
Merge branch 'regional-authorities-telemetry' of github.com:AzureAD/m…
samuelkubai Jun 29, 2021
2bf61e9
Change files
samuelkubai Jun 29, 2021
9f12137
Update @azure-msal-node-extensions-775467ab-ea19-4849-9a64-291a495d42…
samuelkubai Jun 29, 2021
de672a7
fix: fixing the npm audit issues
samuelkubai Jun 29, 2021
9b19470
Change files
samuelkubai Jun 29, 2021
4c9e261
fix: implement the changes suggested on the pull request
samuelkubai Jun 30, 2021
6688e4c
Merge branch 'dev' into regional-authorities-telemetry
samuelkubai Jun 30, 2021
6266eb8
Merge branch 'regional-authorities-telemetry' of github.com:AzureAD/m…
samuelkubai Jun 30, 2021
57e920c
Merge branch 'dev' into regional-authorities-telemetry
samuelkubai Jun 30, 2021
01c0a7e
fix: removed unused variable in sever telemetry manager
samuelkubai Jun 30, 2021
1052214
Merge branch 'regional-authorities-telemetry' of github.com:AzureAD/m…
samuelkubai Jun 30, 2021
582d63f
fix: update the server telemetry manager test
samuelkubai Jun 30, 2021
2a7da53
fix: cleanup the region discovery api
samuelkubai Jun 30, 2021
ac9a7fd
refactor: improve code readability of the RegionDiscovery file
samuelkubai Jul 1, 2021
0dbf971
fix: replace the auto discover flag with the flag from the .NET team
samuelkubai Jul 1, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "minor",
"comment": "feat: add regional authority telemetry",
samuelkubai marked this conversation as resolved.
Show resolved Hide resolved
"packageName": "@azure/msal-common",
"email": "samuelkamau@microsoft.com",
"dependentChangeType": "patch"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "minor",
"comment": "feat: add regional authority telemetry",
samuelkubai marked this conversation as resolved.
Show resolved Hide resolved
"packageName": "@azure/msal-node",
"email": "samuelkamau@microsoft.com",
"dependentChangeType": "patch"
}
36 changes: 34 additions & 2 deletions lib/msal-common/src/authority/Authority.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { UrlString } from "../url/UrlString";
import { IUri } from "../url/IUri";
import { ClientAuthError } from "../error/ClientAuthError";
import { INetworkModule } from "../network/INetworkModule";
import { AuthorityMetadataSource, Constants } from "../utils/Constants";
import { AuthorityMetadataSource, Constants, RegionDiscoveryOutcomes } from "../utils/Constants";
import { ClientConfigurationError } from "../error/ClientConfigurationError";
import { ProtocolMode } from "./ProtocolMode";
import { ICacheManager } from "../cache/interface/ICacheManager";
Expand All @@ -18,6 +18,7 @@ import { AuthorityOptions } from "./AuthorityOptions";
import { CloudInstanceDiscoveryResponse, isCloudInstanceDiscoveryResponse } from "./CloudInstanceDiscoveryResponse";
import { CloudDiscoveryMetadata } from "./CloudDiscoveryMetadata";
import { RegionDiscovery } from "./RegionDiscovery";
import { RegionDiscoveryMetadata } from "./RegionDiscoveryMetadata";

/**
* The authority class validates the authority URIs used by the user, and retrieves the OpenID Configuration Data from the
Expand All @@ -39,6 +40,8 @@ export class Authority {
private metadata: AuthorityMetadataEntity;
// Region discovery service
private regionDiscovery: RegionDiscovery;
// Region discovery metadata
private regionDiscoveryMetadata: RegionDiscoveryMetadata;

constructor(authority: string, networkInterface: INetworkModule, cacheManager: ICacheManager, authorityOptions: AuthorityOptions) {
this.canonicalAuthority = authority;
Expand All @@ -47,6 +50,7 @@ export class Authority {
this.cacheManager = cacheManager;
this.authorityOptions = authorityOptions;
this.regionDiscovery = new RegionDiscovery(networkInterface);
this.regionDiscoveryMetadata = { region_used: undefined, region_source: undefined, region_outcome: undefined };
}

// See above for AuthorityType
Expand Down Expand Up @@ -172,6 +176,15 @@ export class Authority {
}
}

/**
* Region used if any
*
* @returns string
*/
public getRegionDiscoveryMetadata(): RegionDiscoveryMetadata {
return this.regionDiscoveryMetadata;
samuelkubai marked this conversation as resolved.
Show resolved Hide resolved
}

/**
* Replaces tenant in url path with current tenant. Defaults to common.
* @param urlString
Expand Down Expand Up @@ -263,13 +276,32 @@ export class Authority {
if (metadata) {
// If the user prefers to use an azure region replace the global endpoints with regional information.
if (this.authorityOptions.azureRegionConfiguration?.azureRegion) {
const autodetectedRegionName = await this.regionDiscovery.detectRegion(this.authorityOptions.azureRegionConfiguration.environmentRegion);
const autodetectedRegionName = await this.regionDiscovery.detectRegion(this.authorityOptions.azureRegionConfiguration.environmentRegion, this.regionDiscoveryMetadata);

const azureRegion = this.authorityOptions.azureRegionConfiguration.azureRegion === Constants.AZURE_REGION_AUTO_DISCOVER_FLAG
? autodetectedRegionName
: this.authorityOptions.azureRegionConfiguration.azureRegion;

if (this.authorityOptions.azureRegionConfiguration.azureRegion === Constants.AZURE_REGION_AUTO_DISCOVER_FLAG) {
if (autodetectedRegionName) {
this.regionDiscoveryMetadata.region_outcome = RegionDiscoveryOutcomes.AUTO_DETECTION_REQUESTED_SUCCESSFUL;
samuelkubai marked this conversation as resolved.
Show resolved Hide resolved
} else {
this.regionDiscoveryMetadata.region_outcome = RegionDiscoveryOutcomes.AUTO_DETECTION_REQUESTED_FAILED;
}
} else {
if (autodetectedRegionName) {
if (this.authorityOptions.azureRegionConfiguration.azureRegion === autodetectedRegionName) {
samuelkubai marked this conversation as resolved.
Show resolved Hide resolved
this.regionDiscoveryMetadata.region_outcome = RegionDiscoveryOutcomes.CONFIGURED_MATCHES_DETECTED;
} else {
this.regionDiscoveryMetadata.region_outcome = RegionDiscoveryOutcomes.CONFIGURED_NOT_DETECTED;
}
} else {
this.regionDiscoveryMetadata.region_outcome = RegionDiscoveryOutcomes.CONFIGURED_NO_AUTO_DETECTION;
}
}

if (azureRegion) {
this.regionDiscoveryMetadata.region_used = azureRegion;
metadata = Authority.replaceWithRegionalInformation(metadata, azureRegion);
}
}
Expand Down
15 changes: 12 additions & 3 deletions lib/msal-common/src/authority/RegionDiscovery.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@
import { INetworkModule } from "../network/INetworkModule";
import { NetworkResponse } from "../network/NetworkManager";
import { IMDSBadResponse } from "../response/IMDSBadResponse";
import { Constants, ResponseCodes } from "../utils/Constants";
import { Constants, RegionDiscoverySources, ResponseCodes } from "../utils/Constants";
import { RegionDiscoveryMetadata } from "./RegionDiscoveryMetadata";

export class RegionDiscovery {
// Network interface to make requests with.
Expand All @@ -23,7 +24,7 @@ export class RegionDiscovery {
*
* @returns Promise<string | null>
*/
public async detectRegion(environmentRegion: string | undefined): Promise<string | null> {
public async detectRegion(environmentRegion: string | undefined, regionDiscoveryMetadata: RegionDiscoveryMetadata): Promise<string | null> {
// Initialize auto detected region with the region from the envrionment
let autodetectedRegionName = environmentRegion;

Expand All @@ -38,19 +39,27 @@ export class RegionDiscovery {
if (response.status === ResponseCodes.httpBadRequest) {
const latestIMDSVersion = await this.getCurrentVersion();
if (!latestIMDSVersion) {
regionDiscoveryMetadata.region_source = RegionDiscoverySources.FAILED_AUTO_DETECTION;
return null;
}

const response = await this.getRegionFromIMDS(latestIMDSVersion);
if (response.status === ResponseCodes.httpSuccess) {
autodetectedRegionName = response.body;
}
samuelkubai marked this conversation as resolved.
Show resolved Hide resolved
}
}

if (autodetectedRegionName) regionDiscoveryMetadata.region_source = RegionDiscoverySources.IMDS;
samuelkubai marked this conversation as resolved.
Show resolved Hide resolved
} catch(e) {
regionDiscoveryMetadata.region_source = RegionDiscoverySources.FAILED_AUTO_DETECTION;
return null;
}
} else {
regionDiscoveryMetadata.region_source = RegionDiscoverySources.ENVIRONMENT_VARIABLE;
}

if (!autodetectedRegionName) regionDiscoveryMetadata.region_source = RegionDiscoverySources.FAILED_AUTO_DETECTION;

return autodetectedRegionName || null;
}

Expand Down
15 changes: 15 additions & 0 deletions lib/msal-common/src/authority/RegionDiscoveryMetadata.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
/*
* Copyright (c) Microsoft Corporation. All rights reserved.
* Licensed under the MIT License.
*/

import {
RegionDiscoveryOutcomes,
RegionDiscoverySources
} from "../utils/Constants";

export type RegionDiscoveryMetadata = {
region_used?: string;
region_source?: RegionDiscoverySources;
region_outcome?: RegionDiscoveryOutcomes;
};
78 changes: 76 additions & 2 deletions lib/msal-common/src/telemetry/server/ServerTelemetryManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,13 @@
* Licensed under the MIT License.
*/

import { SERVER_TELEM_CONSTANTS, Separators, Constants } from "../../utils/Constants";
import { SERVER_TELEM_CONSTANTS, Separators, Constants, RegionDiscoverySources, RegionDiscoveryOutcomes } from "../../utils/Constants";
import { CacheManager } from "../../cache/CacheManager";
import { AuthError } from "../../error/AuthError";
import { ServerTelemetryRequest } from "./ServerTelemetryRequest";
import { ServerTelemetryEntity } from "../../cache/entities/ServerTelemetryEntity";
import { StringUtils } from "../../utils/StringUtils";
import { RegionDiscoveryMetadata } from "../../authority/RegionDiscoveryMetadata";

export class ServerTelemetryManager {
private cacheManager: CacheManager;
Expand All @@ -18,6 +19,9 @@ export class ServerTelemetryManager {
private telemetryCacheKey: string;
private wrapperSKU: String;
private wrapperVer: String;
private regionUsed: string | undefined;
private regionSource: RegionDiscoverySources | undefined;
private regionOutcome: RegionDiscoveryOutcomes | undefined;

constructor(telemetryRequest: ServerTelemetryRequest, cacheManager: CacheManager) {
this.cacheManager = cacheManager;
Expand All @@ -37,8 +41,10 @@ export class ServerTelemetryManager {
const forceRefreshInt = this.forceRefresh ? 1 : 0;
const request = `${this.apiId}${SERVER_TELEM_CONSTANTS.VALUE_SEPARATOR}${forceRefreshInt}`;
const platformFields = [this.wrapperSKU, this.wrapperVer].join(SERVER_TELEM_CONSTANTS.VALUE_SEPARATOR);
const regionDiscoveryFields = this.getRegionDiscoveryFields();
const requestWithRegionDiscoveryFields = [request, regionDiscoveryFields].join(SERVER_TELEM_CONSTANTS.VALUE_SEPARATOR);

return [SERVER_TELEM_CONSTANTS.SCHEMA_VERSION, request, platformFields].join(SERVER_TELEM_CONSTANTS.CATEGORY_SEPARATOR);
return [SERVER_TELEM_CONSTANTS.SCHEMA_VERSION, requestWithRegionDiscoveryFields, platformFields].join(SERVER_TELEM_CONSTANTS.CATEGORY_SEPARATOR);
tnorling marked this conversation as resolved.
Show resolved Hide resolved
}

/**
Expand Down Expand Up @@ -158,4 +164,72 @@ export class ServerTelemetryManager {

return maxErrors;
}

/**
* Get the region discovery fields
*
* @returns string
*/
getRegionDiscoveryFields(): string {
const regionDiscoveryFields: string[] = [];

regionDiscoveryFields.push(this.regionUsed ? this.regionUsed : "");
sameerag marked this conversation as resolved.
Show resolved Hide resolved
regionDiscoveryFields.push(this.getRegionSourceValue(this.regionSource));
regionDiscoveryFields.push(this.getRegionOutcomeValue(this.regionOutcome));

return regionDiscoveryFields.join(",");
}

/**
* Get the header value for the region source
* @param regionSource
* @returns string
*/
getRegionSourceValue(regionSource: RegionDiscoverySources | undefined): string {
if (!regionSource) return "";

switch (regionSource) {
case RegionDiscoverySources.FAILED_AUTO_DETECTION:
return "1";
case RegionDiscoverySources.INTERNAL_CACHE:
return "2";
case RegionDiscoverySources.ENVIRONMENT_VARIABLE:
return "3";
case RegionDiscoverySources.IMDS:
return "4";
default:
return "0";
}
}

getRegionOutcomeValue(regionOutcome: RegionDiscoveryOutcomes | undefined): string {
if (!regionOutcome) return "";

switch (regionOutcome) {
case RegionDiscoveryOutcomes.CONFIGURED_MATCHES_DETECTED:
return "1";
case RegionDiscoveryOutcomes.CONFIGURED_NO_AUTO_DETECTION:
return "2";
case RegionDiscoveryOutcomes.CONFIGURED_NOT_DETECTED:
return "3";
case RegionDiscoveryOutcomes.AUTO_DETECTION_REQUESTED_SUCCESSFUL:
return "4";
case RegionDiscoveryOutcomes.AUTO_DETECTION_REQUESTED_FAILED:
return "5";
default:
return "0";
}
}

/**
* Update the region discovery metadata
*
* @param regionDiscoveryMetadata
* @returns void
*/
updateRegionDiscoveryMetadata(regionDiscoveryMetadata: RegionDiscoveryMetadata): void {
this.regionUsed = regionDiscoveryMetadata.region_used;
this.regionSource = regionDiscoveryMetadata.region_source;
this.regionOutcome = regionDiscoveryMetadata.region_outcome;
}
}
21 changes: 21 additions & 0 deletions lib/msal-common/src/utils/Constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -349,3 +349,24 @@ export enum ResponseCodes {
httpSuccess = 200,
httpBadRequest = 400
}

/**
* Region Discovery Sources
*/
export enum RegionDiscoverySources {
FAILED_AUTO_DETECTION = "FAILED AUTO DETECTION",
INTERNAL_CACHE = "INTERNAL CACHE",
ENVIRONMENT_VARIABLE = "ENVIRONMENT VARIABLE",
IMDS = "IMDS",
}

/**
* Region Discovery Outcomes
*/
export enum RegionDiscoveryOutcomes {
CONFIGURED_MATCHES_DETECTED = "Configured by developer and matches auto-detected",
samuelkubai marked this conversation as resolved.
Show resolved Hide resolved
CONFIGURED_NO_AUTO_DETECTION = "Configured by developer, cannot be auto-detected",
CONFIGURED_NOT_DETECTED = "Configured by developer, does not match auto-detected",
AUTO_DETECTION_REQUESTED_SUCCESSFUL = "Auto-detect requested and auto-detection worked",
AUTO_DETECTION_REQUESTED_FAILED = "Auto-detect requested and failed, fallback to global"
}
Original file line number Diff line number Diff line change
Expand Up @@ -111,14 +111,14 @@ describe("ServerTelemetryManager.ts", () => {
it("Adds telemetry headers with current request", () => {
const telemetryManager = new ServerTelemetryManager(testTelemetryPayload, testCacheManager);
const currHeaderVal = telemetryManager.generateCurrentRequestHeaderValue();
expect(currHeaderVal).toBe(`2|${testApiCode},0|,`);
expect(currHeaderVal).toEqual(`2|${testApiCode},0,,,|,`);
});

it("Adds telemetry headers with current request with forceRefresh true", () => {
const testPayload: ServerTelemetryRequest = {...testTelemetryPayload, forceRefresh: true };
const telemetryManager = new ServerTelemetryManager(testPayload, testCacheManager);
const currHeaderVal = telemetryManager.generateCurrentRequestHeaderValue();
expect(currHeaderVal).toBe(`2|${testApiCode},1|,`);
expect(currHeaderVal).toEqual(`2|${testApiCode},1,,,|,`);
});

it("Adds telemetry headers with last failed request", () => {
Expand Down
2 changes: 2 additions & 0 deletions lib/msal-node/src/client/ClientApplication.ts
Original file line number Diff line number Diff line change
Expand Up @@ -240,6 +240,8 @@ export abstract class ClientApplication {

const discoveredAuthority = await this.createAuthority(authority, azureRegionConfiguration);

serverTelemetryManager?.updateRegionDiscoveryMetadata(discoveredAuthority.getRegionDiscoveryMetadata());

return {
authOptions: {
clientId: this.config.auth.clientId,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@ mocked(StringUtils.isEmpty).mockImplementation((str) => {

describe('ConfidentialClientApplication', () => {
const authority: Authority = {
getRegionDiscoveryMetadata: () => {
return { region_used: undefined, region_source: undefined, region_outcome: undefined };
},
resolveEndpointsAsync: () => {
return new Promise<void>(resolve => {
resolve();
Expand Down
3 changes: 3 additions & 0 deletions lib/msal-node/test/client/PublicClientApplication.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,9 @@ jest.mock('@azure/msal-common');

describe('PublicClientApplication', () => {
const authority: Authority = {
getRegionDiscoveryMetadata: () => {
return { region_used: undefined, region_source: undefined, region_outcome: undefined };
},
resolveEndpointsAsync: () => {
return new Promise<void>(resolve => {
resolve();
Expand Down