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

feat(http-instrumentation): add content size attributes to spans #1771

Merged
merged 5 commits into from
Dec 22, 2020
Merged

feat(http-instrumentation): add content size attributes to spans #1771

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
2fff599
feat(http-instrumentation): add content size attributes to spans
vmarchaud Dec 19, 2020
4a60254
Merge branch 'master' into http-instrumentation-content-size
dyladan Dec 21, 2020
40c2d2d
fix: review comment
vmarchaud Dec 21, 2020
3d3295e
Merge branch 'master' into http-instrumentation-content-size
dyladan Dec 21, 2020
513f7c7
Merge branch 'master' into http-instrumentation-content-size
dyladan Dec 22, 2020
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
62 changes: 62 additions & 0 deletions packages/opentelemetry-instrumentation-http/src/utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,66 @@ export const setSpanWithError = (
span.setStatus(status);
};

/**
* Adds attributes for request content-length and content-encoding HTTP headers
* @param { IncomingMessage } Request object whose headers will be analyzed
* @param { Attributes } Attributes object to be modified
*/
export const setRequestContentLengthAttribute = (
Copy link
Member

Choose a reason for hiding this comment

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

What is the benefit of these functions?
It seems they are only called from tests but not from the plugin code itself.

Copy link
Member Author

Choose a reason for hiding this comment

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

They are called just under by getOutgoingRequestAttributesOnResponse and getIncomingRequestAttributes

request: IncomingMessage,
attributes: Attributes
) => {
const length = getContentLength(request.headers);
if (length === null) return;

if (isCompressed(request.headers)) {
attributes[HttpAttribute.HTTP_REQUEST_CONTENT_LENGTH] = length;
} else {
attributes[HttpAttribute.HTTP_REQUEST_CONTENT_LENGTH_UNCOMPRESSED] = length;
}
};

/**
* Adds attributes for response content-length and content-encoding HTTP headers
* @param { IncomingMessage } Response object whose headers will be analyzed
* @param { Attributes } Attributes object to be modified
*/
export const setResponseContentLengthAttribute = (
response: IncomingMessage,
attributes: Attributes
) => {
const length = getContentLength(response.headers);
if (length === null) return;

if (isCompressed(response.headers)) {
attributes[HttpAttribute.HTTP_RESPONSE_CONTENT_LENGTH] = length;
} else {
attributes[
HttpAttribute.HTTP_RESPONSE_CONTENT_LENGTH_UNCOMPRESSED
] = length;
}
};

function getContentLength(
headers: OutgoingHttpHeaders | IncomingHttpHeaders
): number | null {
const contentLengthHeader = headers['content-length'];
Copy link
Member

Choose a reason for hiding this comment

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

At least for outgoing HTTP users may specify headers with any casing.

Copy link
Member Author

Choose a reason for hiding this comment

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

They should be lower-cased by node when set: https://github.com/nodejs/node/blob/master/lib/_http_outgoing.js#L572

Copy link
Member

Choose a reason for hiding this comment

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

You refer to the object used by node http internally on symbol kOutHeaders accessible via the apis setHeader/ getHeader/... on outgoing requests.
But actually this is not applicable here. outgoing messages have not .header property at all.

But it seems this is no problem because this plugin captures content-size only for server requests (incoming) and client response (also incoming) where node parses and prepares the lowercased headers.

if (contentLengthHeader === undefined) return null;

const contentLength = parseInt(contentLengthHeader as string, 10);
if (isNaN(contentLength)) return null;

return contentLength;
}

export const isCompressed = (
headers: OutgoingHttpHeaders | IncomingHttpHeaders
): boolean => {
const encoding = headers['content-encoding'];

return !!encoding && encoding !== 'identity';
};

/**
* Makes sure options is an url object
* return an object with default value and parsed options
Expand Down Expand Up @@ -326,6 +386,7 @@ export const getOutgoingRequestAttributesOnResponse = (
[GeneralAttribute.NET_PEER_PORT]: remotePort,
[HttpAttribute.HTTP_HOST]: `${options.hostname}:${remotePort}`,
};
setResponseContentLengthAttribute(response, attributes);

if (statusCode) {
attributes[HttpAttribute.HTTP_STATUS_CODE] = statusCode;
Expand Down Expand Up @@ -386,6 +447,7 @@ export const getIncomingRequestAttributes = (
if (userAgent !== undefined) {
attributes[HttpAttribute.HTTP_USER_AGENT] = userAgent;
}
setRequestContentLengthAttribute(request, attributes);

const httpKindAttributes = getAttributesFromHttpKind(httpVersion);
return Object.assign(attributes, httpKindAttributes);
Expand Down
142 changes: 142 additions & 0 deletions packages/opentelemetry-instrumentation-http/test/functionals/utils.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
* limitations under the License.
*/
import {
Attributes,
StatusCode,
ROOT_CONTEXT,
SpanKind,
Expand Down Expand Up @@ -308,4 +309,145 @@ describe('Utility', () => {
assert.deepEqual(attributes[HttpAttribute.HTTP_ROUTE], undefined);
});
});
// Verify the key in the given attributes is set to the given value,
// and that no other HTTP Content Length attributes are set.
function verifyValueInAttributes(
attributes: Attributes,
key: string | undefined,
value: number
) {
const httpAttributes = [
HttpAttribute.HTTP_RESPONSE_CONTENT_LENGTH_UNCOMPRESSED,
HttpAttribute.HTTP_RESPONSE_CONTENT_LENGTH,
HttpAttribute.HTTP_REQUEST_CONTENT_LENGTH_UNCOMPRESSED,
HttpAttribute.HTTP_REQUEST_CONTENT_LENGTH,
];

for (const attr of httpAttributes) {
if (attr === key) {
assert.strictEqual(attributes[attr], value);
} else {
assert.strictEqual(attributes[attr], undefined);
}
}
}

describe('setRequestContentLengthAttributes()', () => {
it('should set request content-length uncompressed attribute with no content-encoding header', () => {
const attributes: Attributes = {};
const request = {} as IncomingMessage;

request.headers = {
'content-length': '1200',
};
utils.setRequestContentLengthAttribute(request, attributes);

verifyValueInAttributes(
attributes,
HttpAttribute.HTTP_REQUEST_CONTENT_LENGTH_UNCOMPRESSED,
1200
);
});

it('should set request content-length uncompressed attribute with "identity" content-encoding header', () => {
const attributes: Attributes = {};
const request = {} as IncomingMessage;
request.headers = {
'content-length': '1200',
'content-encoding': 'identity',
};
utils.setRequestContentLengthAttribute(request, attributes);

verifyValueInAttributes(
attributes,
HttpAttribute.HTTP_REQUEST_CONTENT_LENGTH_UNCOMPRESSED,
1200
);
});

it('should set request content-length compressed attribute with "gzip" content-encoding header', () => {
const attributes: Attributes = {};
const request = {} as IncomingMessage;
request.headers = {
'content-length': '1200',
'content-encoding': 'gzip',
};
utils.setRequestContentLengthAttribute(request, attributes);

verifyValueInAttributes(
attributes,
HttpAttribute.HTTP_REQUEST_CONTENT_LENGTH,
1200
);
});
});

describe('setResponseContentLengthAttributes()', () => {
it('should set response content-length uncompressed attribute with no content-encoding header', () => {
const attributes: Attributes = {};

const response = {} as IncomingMessage;

response.headers = {
'content-length': '1200',
};
utils.setResponseContentLengthAttribute(response, attributes);

verifyValueInAttributes(
attributes,
HttpAttribute.HTTP_RESPONSE_CONTENT_LENGTH_UNCOMPRESSED,
1200
);
});

it('should set response content-length uncompressed attribute with "identity" content-encoding header', () => {
const attributes: Attributes = {};

const response = {} as IncomingMessage;

response.headers = {
'content-length': '1200',
'content-encoding': 'identity',
};

utils.setResponseContentLengthAttribute(response, attributes);

verifyValueInAttributes(
attributes,
HttpAttribute.HTTP_RESPONSE_CONTENT_LENGTH_UNCOMPRESSED,
1200
);
});

it('should set response content-length compressed attribute with "gzip" content-encoding header', () => {
const attributes: Attributes = {};

const response = {} as IncomingMessage;

response.headers = {
'content-length': '1200',
'content-encoding': 'gzip',
};

utils.setResponseContentLengthAttribute(response, attributes);

verifyValueInAttributes(
attributes,
HttpAttribute.HTTP_RESPONSE_CONTENT_LENGTH,
1200
);
});

it('should set no attributes with no content-length header', () => {
const attributes: Attributes = {};
const message = {} as IncomingMessage;

message.headers = {
'content-encoding': 'gzip',
};
utils.setResponseContentLengthAttribute(message, attributes);

verifyValueInAttributes(attributes, undefined, 1200);
});
});
});
40 changes: 40 additions & 0 deletions packages/opentelemetry-instrumentation-http/test/utils/assertSpan.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,26 @@ export const assertSpan = (
}
}
if (span.kind === SpanKind.CLIENT) {
if (validations.resHeaders['content-length']) {
const contentLength = Number(validations.resHeaders['content-length']);

if (
validations.resHeaders['content-encoding'] &&
validations.resHeaders['content-encoding'] !== 'identity'
) {
assert.strictEqual(
span.attributes[HttpAttribute.HTTP_RESPONSE_CONTENT_LENGTH],
contentLength
);
} else {
assert.strictEqual(
span.attributes[
HttpAttribute.HTTP_RESPONSE_CONTENT_LENGTH_UNCOMPRESSED
],
contentLength
);
}
}
assert.strictEqual(
span.attributes[GeneralAttribute.NET_PEER_NAME],
validations.hostname,
Expand All @@ -108,6 +128,26 @@ export const assertSpan = (
);
}
if (span.kind === SpanKind.SERVER) {
if (validations.reqHeaders && validations.reqHeaders['content-length']) {
const contentLength = validations.reqHeaders['content-length'];

if (
validations.reqHeaders['content-encoding'] &&
validations.reqHeaders['content-encoding'] !== 'identity'
) {
assert.strictEqual(
span.attributes[HttpAttribute.HTTP_REQUEST_CONTENT_LENGTH],
contentLength
);
} else {
assert.strictEqual(
span.attributes[
HttpAttribute.HTTP_REQUEST_CONTENT_LENGTH_UNCOMPRESSED
],
contentLength
);
}
}
if (validations.serverName) {
assert.strictEqual(
span.attributes[HttpAttribute.HTTP_SERVER_NAME],
Expand Down