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.

Sign up for GitHub

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

Jump to bottom

fix(AutoCollectHttpRequests): Http requests aren't collect if we call dispose+setup after http.createServer #947

Merged
merged 4 commits into from
Apr 15, 2022
Merged

fix(AutoCollectHttpRequests): Http requests aren't collect if we call dispose+setup after http.createServer #947

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
ace4fba
fix(AutoCollectHttpRequests): Http requests aren't collect if we call…
Apr 13, 2022
af8252b
refactor(AutoCollectHttpRequests): Add underscore on private function
Apr 13, 2022
4d61b38
fix(Tests-EndToEnd): Http requests handler isn't placed on stub creat…
Apr 15, 2022
4f985d8
refactor(AutoCollectHttpRequests): Set private the static property
Apr 15, 2022
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
63 changes: 36 additions & 27 deletions AutoCollection/HttpRequests.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import http = require("http");
import https = require("https");
import url = require("url");

import Contracts = require("../Declarations/Contracts");
import TelemetryClient = require("../Library/TelemetryClient");
Expand All @@ -14,6 +13,7 @@ import AutoCollectPerformance = require("./Performance");
class AutoCollectHttpRequests {

public static INSTANCE: AutoCollectHttpRequests;
private static HANDLER_READY: boolean = false;

private static alreadyAutoCollectedFlag = "_appInsightsAutoCollected";

Expand Down Expand Up @@ -75,9 +75,38 @@ class AutoCollectHttpRequests {
);
}

private _registerRequest(request: http.ServerRequest, response: http.ServerResponse, onRequest: Function) {
// Set up correlation context
const requestParser = new HttpRequestParser(request);
const correlationContext = this._generateCorrelationContext(requestParser);

// Note: Check for if correlation is enabled happens within this method.
// If not enabled, function will directly call the callback.
CorrelationContextManager.runWithContext(correlationContext, () => {
if (this._isEnabled) {
// Mark as auto collected
(<any>request)[AutoCollectHttpRequests.alreadyAutoCollectedFlag] = true;

// Auto collect request
AutoCollectHttpRequests.trackRequest(this._client, { request: request, response: response }, requestParser);
}

if (typeof onRequest === "function") {
onRequest(request, response);
}
});
}

private _initialize() {
this._isInitialized = true;

// Avoid the creation of multiple handler on http(s).createServer
if (AutoCollectHttpRequests.HANDLER_READY) {
return;
}

AutoCollectHttpRequests.HANDLER_READY = true;

const wrapOnRequestHandler: Function = (onRequest?: Function) => {
if (!onRequest) {
return undefined;
Expand All @@ -91,32 +120,14 @@ class AutoCollectHttpRequests {
const shouldCollect: boolean = request && !(<any>request)[AutoCollectHttpRequests.alreadyAutoCollectedFlag];

if (request && shouldCollect) {
// Set up correlation context
const requestParser = new HttpRequestParser(request);
const correlationContext = this._generateCorrelationContext(requestParser);

// Note: Check for if correlation is enabled happens within this method.
// If not enabled, function will directly call the callback.
CorrelationContextManager.runWithContext(correlationContext, () => {
if (this._isEnabled) {
// Mark as auto collected
(<any>request)[AutoCollectHttpRequests.alreadyAutoCollectedFlag] = true;

// Auto collect request
AutoCollectHttpRequests.trackRequest(this._client, { request: request, response: response }, requestParser);
}

if (typeof onRequest === "function") {
onRequest(request, response);
}
});
AutoCollectHttpRequests.INSTANCE?._registerRequest(request, response, onRequest)
} else {
if (typeof onRequest === "function") {
onRequest(request, response);
}
}
}
}
};

// The `http.createServer` function will instantiate a new http.Server object.
// Inside the Server's constructor, it is using addListener to register the
Expand Down Expand Up @@ -144,7 +155,7 @@ class AutoCollectHttpRequests {
};
// on is an alias to addListener only
server.on = server.addListener;
}
};

const originalHttpServer: any = http.createServer;

Expand All @@ -163,14 +174,14 @@ class AutoCollectHttpRequests {
wrapServerEventHandler(server);
return server;
}
}
};

const originalHttpsServer = https.createServer;
https.createServer = (options: https.ServerOptions, onRequest?: Function) => {
const server: https.Server = originalHttpsServer(options, wrapOnRequestHandler(onRequest));
wrapServerEventHandler(server);
return server;
}
};
}

/**
Expand Down Expand Up @@ -242,7 +253,7 @@ class AutoCollectHttpRequests {
if (telemetry.request.on) {
telemetry.request.on("aborted", () => {
const errorMessage = "The request has been aborted and the network socket has closed.";
AutoCollectHttpRequests.endRequest(client, requestParser, telemetry, null, errorMessage);
AutoCollectHttpRequests.endRequest(client, requestParser, telemetry, null, errorMessage);
});
}
}
Expand Down Expand Up @@ -295,6 +306,4 @@ class AutoCollectHttpRequests {
}
}



export = AutoCollectHttpRequests;
9 changes: 9 additions & 0 deletions Tests/EndToEnd.tests.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import Util = require("../Library/Util");
import { JsonConfig } from "../Library/JsonConfig";
import { FileAccessControl } from "../Library/FileAccessControl";
import FileSystemHelper = require("../Library/FileSystemHelper");
import AutoCollectHttpRequests = require("../AutoCollection/HttpRequests");

/**
* A fake response class that passes by default
Expand Down Expand Up @@ -242,6 +243,10 @@ describe("EndToEnd", () => {
return fakeHttpSrv;
});

// We stub the property indicating if the handler on http(s).createServer is ready so that the handler is
// placed on the function http(s).createServer stub
sandbox.stub(AutoCollectHttpRequests, 'HANDLER_READY', false);

AppInsights
.setup("ikey")
.setAutoCollectRequests(true)
Expand All @@ -265,6 +270,10 @@ describe("EndToEnd", () => {
return fakeHttpSrv;
});

// We stub the property indicating if the handler on http(s).createServer is ready so that the handler is
// placed on the function http(s).createServer stub
sandbox.stub(AutoCollectHttpRequests, 'HANDLER_READY', false);

AppInsights
.setup("ikey")
.setAutoCollectRequests(true)
Expand Down