Skip to content

Commit

Permalink
fix: use originalUrl for Express middleware's request url (#476)
Browse files Browse the repository at this point in the history
Fixes: #472
  • Loading branch information
hgiasac authored and ofrobots committed May 14, 2019
1 parent afe89de commit 0ee71bd
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 14 deletions.
8 changes: 6 additions & 2 deletions src/middleware/express/make-http-request.ts
Expand Up @@ -18,14 +18,18 @@ import * as http from 'http';

import {StackdriverHttpRequest} from '../../http-request';

export interface ServerRequest extends http.IncomingMessage {
originalUrl: string;
}

export function makeHttpRequestData(
req: http.IncomingMessage,
req: ServerRequest,
res: http.ServerResponse,
latencyMilliseconds: number
): StackdriverHttpRequest {
return {
status: res.statusCode,
requestUrl: req.url,
requestUrl: req.originalUrl,
requestMethod: req.method,
userAgent: req.headers['user-agent'],
responseSize:
Expand Down
10 changes: 3 additions & 7 deletions src/middleware/express/make-middleware.ts
Expand Up @@ -18,10 +18,10 @@ import * as http from 'http';
import onFinished = require('on-finished');
import {getOrInjectContext, makeHeaderWrapper} from '../context';

import {makeHttpRequestData} from './make-http-request';
import {makeHttpRequestData, ServerRequest} from './make-http-request';
import {StackdriverHttpRequest} from '../../http-request';

interface AnnotatedRequestType<LoggerType> extends http.IncomingMessage {
interface AnnotatedRequestType<LoggerType> extends ServerRequest {
log: LoggerType;
}

Expand All @@ -46,11 +46,7 @@ export function makeMiddleware<LoggerType>(
makeChildLogger: (trace: string) => LoggerType,
emitRequestLog?: (httpRequest: StackdriverHttpRequest, trace: string) => void
) {
return (
req: http.IncomingMessage,
res: http.ServerResponse,
next: Function
) => {
return (req: ServerRequest, res: http.ServerResponse, next: Function) => {
// TODO(ofrobots): use high-resolution timer.
const requestStartMs = Date.now();

Expand Down
13 changes: 8 additions & 5 deletions test/middleware/express/test-make-http-request.ts
Expand Up @@ -15,31 +15,34 @@
*/

import * as assert from 'assert';
import {IncomingMessage, ServerResponse} from 'http';
import {makeHttpRequestData} from '../../../src/middleware/express/make-http-request';
import {ServerResponse} from 'http';
import {
makeHttpRequestData,
ServerRequest,
} from '../../../src/middleware/express/make-http-request';

describe('middleware/express/make-http-request', () => {
it('should convert latency to proto Duration', () => {
const fakeRequest = {headers: {}};
const fakeResponse = {};

const h1 = makeHttpRequestData(
fakeRequest as IncomingMessage,
fakeRequest as ServerRequest,
fakeResponse as ServerResponse,
1003
);
assert.deepStrictEqual(h1.latency, {seconds: 1, nanos: 3e6});

const h2 = makeHttpRequestData(
fakeRequest as IncomingMessage,
fakeRequest as ServerRequest,
fakeResponse as ServerResponse,
9003.1
);
assert.deepStrictEqual(h2.latency, {seconds: 9, nanos: 3.1e6});

// Make sure we nanos is uint32.
const h3 = makeHttpRequestData(
fakeRequest as IncomingMessage,
fakeRequest as ServerRequest,
fakeResponse as ServerResponse,
1.0000000001
);
Expand Down

0 comments on commit 0ee71bd

Please sign in to comment.