Skip to content

Commit

Permalink
Remove propagation of role name in correlation headers (#398)
Browse files Browse the repository at this point in the history
  • Loading branch information
OsvaldoRosado authored Jun 1, 2018
1 parent 7588e9a commit 0e15408
Show file tree
Hide file tree
Showing 6 changed files with 12 additions and 31 deletions.
5 changes: 2 additions & 3 deletions AutoCollection/HttpDependencies.ts
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
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
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
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
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
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

0 comments on commit 0e15408

Please sign in to comment.