Skip to content

Commit

Permalink
Fix Crashing with Old Node.JS Versions When Manually Instrumenting (#…
Browse files Browse the repository at this point in the history
…1362)

* Fix node 14 support by conditionally loading core-auth.

* Add README note.

* Remove unneeded core-auth check from the agent code.

* Update package-lock.json

* Update README.md

* Update README.md

* Update package-lock.json
  • Loading branch information
JacksonWeber authored Jul 16, 2024
1 parent 2b68662 commit 8173f7c
Show file tree
Hide file tree
Showing 5 changed files with 30 additions and 22 deletions.
5 changes: 2 additions & 3 deletions Bootstrap/Default.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import * as azureCoreAuth from "@azure/core-auth";

import type { TokenCredential } from "@azure/core-auth";
import * as types from "../applicationinsights";
import * as Helpers from "./Helpers";
import Constants = require("../Declarations/Constants");
Expand Down Expand Up @@ -40,7 +39,7 @@ export function setStatusLogger(statusLogger: StatusLogger) {
* Try to setup and start this app insights instance if attach is enabled.
* @param aadTokenCredential Optional AAD credential
*/
export function setupAndStart(aadTokenCredential?: azureCoreAuth.TokenCredential, isAzureFunction?: boolean): typeof types | null {
export function setupAndStart(aadTokenCredential?: TokenCredential, isAzureFunction?: boolean): typeof types | null {
// If app already contains SDK, skip agent attach
if (!forceStart && Helpers.sdkAlreadyExists(_logger)) {
_statusLogger.logStatus({
Expand Down
4 changes: 2 additions & 2 deletions Declarations/Interfaces.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import http = require("http");
import https = require("https");
import * as azureCoreAuth from "@azure/core-auth";
import type { TokenCredential } from "@azure/core-auth";
import { DistributedTracingModes } from "../applicationinsights";
import { IDisabledExtendedMetrics } from "../AutoCollection/NativePerformance";

Expand Down Expand Up @@ -224,5 +224,5 @@ export interface IConfig extends IBaseConfig {
/** An https.Agent to use for SDK HTTPS traffic (Optional, Default undefined) */
httpsAgent: https.Agent;
/** AAD TokenCredential to use to authenticate the app */
aadTokenCredential?: azureCoreAuth.TokenCredential;
aadTokenCredential?: TokenCredential;
}
34 changes: 22 additions & 12 deletions Library/AuthorizationHandler.ts
Original file line number Diff line number Diff line change
@@ -1,32 +1,42 @@
import http = require("http");
import https = require("https");
import azureCoreAuth = require("@azure/core-auth");
import azureCore = require("@azure/core-rest-pipeline");
import { TokenCredential } from "@azure/core-auth";
import { PipelineRequest, PipelineResponse, PipelinePolicy } from "@azure/core-rest-pipeline";
import Logging = require("./Logging");

const applicationInsightsResource = "https://monitor.azure.com//.default";

let azureCore: any;
try {
azureCore = require("@azure/core-rest-pipeline");
} catch (e) {
Logging.warn("Cannot load @azure/core-auth package. This package is required for AAD token authentication. It's likely that your node.js version is not supported by the JS Azure SDK.");
}

function emptySendRequest(_request: azureCore.PipelineRequest): Promise<azureCore.PipelineResponse> {
function emptySendRequest(_request: PipelineRequest): Promise<PipelineResponse> {
return null;
}

class AuthorizationHandler {

private _azureTokenPolicy: azureCore.PipelinePolicy;
private _azureTokenPolicy: PipelinePolicy;

constructor(credential: azureCoreAuth.TokenCredential, aadAudience?: string) {
let scopes: string[] = aadAudience ? [aadAudience] : [applicationInsightsResource];
this._azureTokenPolicy = azureCore.bearerTokenAuthenticationPolicy({ credential, scopes });
constructor(credential: TokenCredential, aadAudience?: string) {
if (azureCore) {
let scopes: string[] = aadAudience ? [aadAudience] : [applicationInsightsResource];
this._azureTokenPolicy = azureCore.bearerTokenAuthenticationPolicy({ credential, scopes });
}
}

/**
* Applies the Bearer token to the request through the Authorization header.
*/
public async addAuthorizationHeader(requestOptions: http.RequestOptions | https.RequestOptions): Promise<void> {
let authHeaderName = "authorization";
let webResource = azureCore.createPipelineRequest({ url: "https://" });
await this._azureTokenPolicy.sendRequest(webResource, emptySendRequest);
requestOptions.headers[authHeaderName] = webResource.headers.get(authHeaderName);
if (azureCore) {
let authHeaderName = "authorization";
let webResource = azureCore.createPipelineRequest({ url: "https://" });
await this._azureTokenPolicy.sendRequest(webResource, emptySendRequest);
requestOptions.headers[authHeaderName] = webResource.headers.get(authHeaderName);
}
}
}

Expand Down
5 changes: 2 additions & 3 deletions Library/Config.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
import azureCoreAuth = require("@azure/core-auth");

import CorrelationIdManager = require("./CorrelationIdManager");
import ConnectionStringParser = require("./ConnectionStringParser");
import Logging = require("./Logging");
Expand All @@ -11,6 +9,7 @@ import { JsonConfig } from "./JsonConfig";
import { IConfig, IWebInstrumentationConfig } from "../Declarations/Interfaces";
import { DistributedTracingModes } from "../applicationinsights";
import { IDisabledExtendedMetrics } from "../AutoCollection/NativePerformance";
import type { TokenCredential } from "@azure/core-auth";

class Config implements IConfig {

Expand All @@ -33,7 +32,7 @@ class Config implements IConfig {
public httpAgent: http.Agent;
public httpsAgent: https.Agent;
public ignoreLegacyHeaders: boolean;
public aadTokenCredential?: azureCoreAuth.TokenCredential;
public aadTokenCredential?: TokenCredential;
public aadAudience?: string;
public enableAutoCollectConsole: boolean;
public enableLoggerErrorToTrace: boolean;
Expand Down
4 changes: 2 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ You can manually track more aspects of your app and system using the API describ



## Supported Node.JS versions
## Supported Node.JS versions

| Platform Version | Supported |
|------------------|-------------------------------------------------|
Expand Down Expand Up @@ -225,7 +225,7 @@ separately from clients created with `new appInsights.TelemetryClient()`.
| noHttpAgentKeepAlive | HTTPS without a passed in agent |
| httpAgent | An http.Agent to use for SDK HTTP traffic (Optional, Default undefined) |
| httpsAgent | An https.Agent to use for SDK HTTPS traffic (Optional, Default undefined)
| aadTokenCredential| Azure Credential instance to be used to authenticate the App. [AAD Identity Credential Classes](https://github.com/Azure/azure-sdk-for-js/tree/master/sdk/identity/identity#credential-classes)
| aadTokenCredential| Azure Credential instance to be used to authenticate the App. [AAD Identity Credential Classes](https://github.com/Azure/azure-sdk-for-js/tree/master/sdk/identity/identity#credential-classes). Only versions of Node.JS supported by the [JS Azure SDK](https://github.com/Azure/azure-sdk-for-js/blob/main/SUPPORT.md) support AAD authentication.
| enableWebInstrumentation | Sets the state of automatic web Instrumentation (Optional, disabled by default). If true, web instrumentation will be enabled on valid node server http response with the connection string used for SDK initialization
| webInstrumentationConnectionString | Sets connection string used for web Instrumentation (Optional, Default undefined)|
| webInstrumentationSrc | Sets web Instrumentation CDN url (Optional). see more details at [ApplicationInsights JavaScript SDK](https://github.com/microsoft/ApplicationInsights-JS)|
Expand Down

0 comments on commit 8173f7c

Please sign in to comment.