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

Adding Application Insights support to base images #18

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
40 changes: 35 additions & 5 deletions 8.1/startup/generateStartupCommand.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,15 +18,45 @@ if (typeof process.env.WEBSITE_ROLE_INSTANCE_ID !== 'undefined'
roleInstanceId = process.env.WEBSITE_ROLE_INSTANCE_ID;
}

var startupCommand = fs.readFileSync(CMDFILE, 'utf8').trim();
// Is Application Insights enabled, with an associated ikey?
var appInsightsEnabled = process.env.ENABLE_APPINSIGHTS && process.env.APPINSIGHTS_INSTRUMENTATIONKEY;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Per our previous discussion, the expectation is that if the ENABLE_APPINSIGHTS is set, then an Application Insights resource has been provisioned, and its associated instrumentation key has been specified via the APPINSIGHTS_INSTRUMENTATIONKEY variable (which the Application Insights SDK auto-detects).

var appInsightsPreloadArg = "--require /opt/startup/initAppInsights.js";
var nodeCommandPrefix = "node ";

if (appInsightsEnabled) {
console.log("Application Insights enabled");
nodeCommandPrefix += appInsightsPreloadArg + " ";
}

function augmentCommandIfNeccessary(command) {
if (!command || !appInsightsEnabled) {
return command;
}

// Application Insights is enabled, so we need to
// to update the specified startup command to pre-load it.
if (command.indexOf("pm2 start ") === 0) {
return command += " --node-args='" + appInsightsPreloadArg + "'";
} else if (command.indexOf("node ") === 0) {
// Simply replacing the prefix allows the user to specify
// additional Node flags, in addition to the AI preload one.
return command.replace("node ", nodeCommandPrefix);
}

// The command is using an unknown executable, and therefore,
// the App Insights runtime can't be automatically enabled.
return command;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is primarily meant to support the NPM start script scenario, where the app could specific an arbitrary command, that I'm not aware of a deterministic way to bootstrap Application Insights into. In practice, accommodating PM2 and Node should cover most scenarios.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be fine for now.

}

var startupCommand = augmentCommandIfNeccessary(fs.readFileSync(CMDFILE, 'utf8').trim());
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Besides pm2 start <process.json> and node <startScript>, are there any other startup command formats that could be provided by the user and passed in when the Docker container is run? I'm not aware of any, but I just wanted to confirm.


// No user-provided startup command, check for scripts.start
if (!startupCommand) {
var packageJsonPath = "/home/site/wwwroot/package.json";
var json = fs.existsSync(packageJsonPath) && JSON.parse(fs.readFileSync(packageJsonPath, 'utf8'))
if (typeof json == 'object' && typeof json.scripts == 'object' && typeof json.scripts.start == 'string') {
console.log("Found scripts.start in package.json")
startupCommand = 'npm start';
console.log("Found scripts.start in package.json");
startupCommand = augmentCommandIfNeccessary(json.scripts.start.trim());
}
}

Expand All @@ -37,7 +67,7 @@ if (!startupCommand) {
var filename = "/home/site/wwwroot/" + autos[i];
if (fs.existsSync(filename)) {
console.log("No startup command entered, but found " + filename);
startupCommand = "node " + filename;
startupCommand = nodeCommandPrefix + filename;
break;
}
}
Expand All @@ -47,7 +77,7 @@ if (!startupCommand) {
if (!startupCommand) {
console.log("No startup command or autodetected startup script " +
"found. Running default static site.");
startupCommand = "node " + DEFAULTAPP;
startupCommand = nodeCommandPrefix + DEFAULTAPP;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could potentially skip initializing the AI runtime here, since it's just a "getting started" app. But, if the user enabled AI, they might expect to be able to see the HTTP traffic for the starter, in order to get a feel for the experience, before they push their own app code. Feedback would be appreciated here.

}

// If HTTP logging is enabled and it doesn't appear that the user has tried to do any
Expand Down
8 changes: 8 additions & 0 deletions 8.1/startup/initAppInsights.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
try {
// Initialize the Application Insights runtime, using the
// instrumentation key provided by the environment.
require("applicationinsights").setup().start();
} catch (error) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since I check for the presence of the ikey before loading this script, there aren't are known reasons why this would fail. However, I added this error swallowing logic, since I assume we want to be sure this feature add-on doesn't impact the stability of the app itself.

// Swallow any errors, in order to safeguard the
// user's app from any unexpected exceptions.
}
Loading