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

Remove propagation of role name in correlation headers #398

Merged
merged 1 commit into from
Jun 1, 2018
Merged

Remove propagation of role name in correlation headers #398

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
5 changes: 2 additions & 3 deletions AutoCollection/HttpDependencies.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,16 +128,15 @@ class AutoCollectHttpDependencies {
if (correlationHeader) {
const components = correlationHeader.split(",");
const key = `${RequestResponseHeaders.requestContextSourceKey}=`;
const roleNameKey = `${RequestResponseHeaders.requestContextSourceRoleNameKey}=`;
if (!components.some((value) => value.substring(0,key.length) === key)) {
telemetry.request.setHeader(
RequestResponseHeaders.requestContextHeader,
`${correlationHeader},${RequestResponseHeaders.requestContextSourceKey}=${client.config.correlationId},${RequestResponseHeaders.requestContextSourceRoleNameKey}=${client.context.tags[client.context.keys.cloudRole]}`);
`${correlationHeader},${RequestResponseHeaders.requestContextSourceKey}=${client.config.correlationId}`);
}
} else {
telemetry.request.setHeader(
RequestResponseHeaders.requestContextHeader,
`${RequestResponseHeaders.requestContextSourceKey}=${client.config.correlationId},${RequestResponseHeaders.requestContextSourceRoleNameKey}=${client.context.tags[client.context.keys.cloudRole]}`);
`${RequestResponseHeaders.requestContextSourceKey}=${client.config.correlationId}`);
}
}

Expand Down
4 changes: 1 addition & 3 deletions AutoCollection/HttpDependencyParser.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ import CorrelationIdManager = require("../Library/CorrelationIdManager");
*/
class HttpDependencyParser extends RequestParser {
private correlationId: string;
private targetRoleName: string;

constructor(requestOptions: string | http.RequestOptions | https.RequestOptions, request: http.ClientRequest) {
super();
Expand All @@ -41,7 +40,6 @@ class HttpDependencyParser extends RequestParser {
public onResponse(response: http.ClientResponse) {
this._setStatus(response.statusCode, undefined);
this.correlationId = Util.getCorrelationContextTarget(response, RequestResponseHeaders.requestContextTargetKey);
this.targetRoleName = Util.getCorrelationContextTarget(response, RequestResponseHeaders.requestContextTargetRoleNameKey);
}

/**
Expand All @@ -60,7 +58,7 @@ class HttpDependencyParser extends RequestParser {
if (this.correlationId) {
remoteDependencyType = Contracts.RemoteDependencyDataConstants.TYPE_AI;
if (this.correlationId !== CorrelationIdManager.correlationIdPrefix) {
remoteDependencyTarget = urlObject.hostname + " | " + this.correlationId + " | roleName:" + this.targetRoleName;
remoteDependencyTarget = urlObject.hostname + " | " + this.correlationId;
}
} else {
remoteDependencyType = Contracts.RemoteDependencyDataConstants.TYPE_HTTP;
Expand Down
4 changes: 1 addition & 3 deletions AutoCollection/HttpRequestParser.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ class HttpRequestParser extends RequestParser {
private legacySocketRemoteAddress: string;
private userAgent: string;
private sourceCorrelationId: string;
private sourceRoleName: string;
private parentId: string;
private operationId: string;
private requestId: string;
Expand Down Expand Up @@ -70,7 +69,7 @@ class HttpRequestParser extends RequestParser {
See https://github.com/Microsoft/ApplicationInsights-dotnet-server/blob/25d695e6a906fbe977f67be3966d25dbf1c50a79/Src/Web/Web.Shared.Net/RequestTrackingTelemetryModule.cs#L250
for reference
*/
source: this.sourceCorrelationId + " | roleName:" + this.sourceRoleName,
source: this.sourceCorrelationId,
duration: this.duration,
resultCode: this.statusCode ? this.statusCode.toString() : null,
success: this._isSuccess(),
Expand Down Expand Up @@ -200,7 +199,6 @@ class HttpRequestParser extends RequestParser {
this.rawHeaders = request.headers || (<any>request).rawHeaders;
this.userAgent = request.headers && request.headers["user-agent"];
this.sourceCorrelationId = Util.getCorrelationContextTarget(request, RequestResponseHeaders.requestContextSourceKey);
this.sourceRoleName = Util.getCorrelationContextTarget(request, RequestResponseHeaders.requestContextSourceRoleNameKey);

if (request.headers) {
this.correlationContextHeader = request.headers[RequestResponseHeaders.correlationContextHeader];
Expand Down
4 changes: 2 additions & 2 deletions AutoCollection/HttpRequests.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -230,12 +230,12 @@ class AutoCollectHttpRequests {
if (!components.some((value) => value.substring(0,key.length) === key)) {
response.setHeader(
RequestResponseHeaders.requestContextHeader,
`${correlationHeader},${RequestResponseHeaders.requestContextSourceKey}=${client.config.correlationId},${RequestResponseHeaders.requestContextSourceRoleNameKey}=${client.context.tags[client.context.keys.cloudRole]}`);
`${correlationHeader},${RequestResponseHeaders.requestContextSourceKey}=${client.config.correlationId}`);
}
} else {
response.setHeader(
RequestResponseHeaders.requestContextHeader,
`${RequestResponseHeaders.requestContextSourceKey}=${client.config.correlationId},${RequestResponseHeaders.requestContextSourceRoleNameKey}=${client.context.tags[client.context.keys.cloudRole]}`);
`${RequestResponseHeaders.requestContextSourceKey}=${client.config.correlationId}`);
}
}
}
Expand Down
12 changes: 0 additions & 12 deletions Library/RequestResponseHeaders.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,18 +17,6 @@ export = {
*/
requestContextTargetKey: "appId",

/**
* Source-RoleName key in the request context header that is added by an application
* while making http requests and retrieved by the other application when processing incoming requests.
*/
requestContextSourceRoleNameKey: "roleName",

/**
* Target-RoleName key in the request context header that is added to the response
* and retrieved by the calling application when processing incoming responses.
*/
requestContextTargetRoleNameKey: "roleName",

/**
* Request-Id header
*/
Expand Down
14 changes: 6 additions & 8 deletions Tests/Library/Client.tests.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -379,8 +379,7 @@ describe("Library/TelemetryClient", () => {

// Simulate an incoming request that has a different source correlationId header.
let testCorrelationId = 'cid-v1:Application-Id-98765-4321A';
let testRoleName = "Backend";
request.headers[RequestResponseHeaders.requestContextHeader] = `${RequestResponseHeaders.requestContextSourceKey}=${testCorrelationId},${RequestResponseHeaders.requestContextSourceRoleNameKey}=${testRoleName}`;
request.headers[RequestResponseHeaders.requestContextHeader] = `${RequestResponseHeaders.requestContextSourceKey}=${testCorrelationId}`;

client.trackNodeHttpRequest({ request: <any>request, response: <any>response, properties: properties });

Expand All @@ -392,11 +391,11 @@ describe("Library/TelemetryClient", () => {
response.emitFinish();
assert.ok(trackStub.calledOnce);
var requestTelemetry = <Contracts.RequestTelemetry>trackStub.firstCall.args[0];
assert.equal(requestTelemetry.source, testCorrelationId + " | roleName:" + testRoleName);
assert.equal(requestTelemetry.source, testCorrelationId);

// The client's correlationId should have been added as the response target correlationId header.
assert.equal(response.headers[RequestResponseHeaders.requestContextHeader],
`${RequestResponseHeaders.requestContextSourceKey}=${client.config.correlationId},${RequestResponseHeaders.requestContextSourceRoleNameKey}=${client.context.tags[client.context.keys.cloudRole]}`);
`${RequestResponseHeaders.requestContextSourceKey}=${client.config.correlationId}`);
});

it('should NOT use source and target correlationId headers when url is on the excluded list', () => {
Expand Down Expand Up @@ -564,24 +563,23 @@ describe("Library/TelemetryClient", () => {

// The client's correlationId should have been added as the request source correlationId header.
assert.equal(request.headers[RequestResponseHeaders.requestContextHeader],
`${RequestResponseHeaders.requestContextSourceKey}=${client.config.correlationId},${RequestResponseHeaders.requestContextSourceRoleNameKey}=${client.context.tags[client.context.keys.cloudRole]}`);
`${RequestResponseHeaders.requestContextSourceKey}=${client.config.correlationId}`);

// response event was not emitted yet
assert.ok(trackStub.notCalled);

// Simulate a response from another service that includes a target correlationId header.
const targetCorrelationId = "cid-v1:Application-Key-98765-4321A";
const targetRoleName = "Backend";
response.headers[RequestResponseHeaders.requestContextHeader] =
`${RequestResponseHeaders.requestContextTargetKey}=${targetCorrelationId},${RequestResponseHeaders.requestContextTargetRoleNameKey}=${targetRoleName},`;
`${RequestResponseHeaders.requestContextTargetKey}=${targetCorrelationId}`;

// emit response event
clock.tick(10);
request.emitResponse();
assert.ok(trackStub.calledOnce);
var dependencyTelemetry = <Contracts.DependencyTelemetry>trackStub.firstCall.args[0];

assert.equal(dependencyTelemetry.target, "bing.com | " + targetCorrelationId + " | roleName:" + targetRoleName);
assert.equal(dependencyTelemetry.target, "bing.com | " + targetCorrelationId);
assert.equal(dependencyTelemetry.dependencyTypeName, "Http (tracked component)");
});

Expand Down