Skip to content

Commit

Permalink
fix: add license header, rename enum files and refactoring
Browse files Browse the repository at this point in the history
refactor: make integration tests mandatory for ci only
remove duplicate tests
remove dead comments

Signed-off-by: Olivier Albertini <[email protected]>
  • Loading branch information
OlivierAlbertini committed Aug 30, 2019
1 parent d612642 commit 23f745b
Show file tree
Hide file tree
Showing 10 changed files with 59 additions and 135 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,6 @@
*/
export enum AttributeNames {
HTTP_HOSTNAME = 'http.hostname',
// NOT ON OFFICIAL SPEC
ERROR = 'error',
COMPONENT = 'component',
HTTP_METHOD = 'http.method',
HTTP_PATH = 'http.path',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
* limitations under the License.
*/

// Should we export this to the types package in order to expose all values ?
export enum Format {
HTTP = 'HttpTraceContext',
}
4 changes: 2 additions & 2 deletions packages/opentelemetry-plugin-http/src/http.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,8 @@ import {
ParsedRequestOptions,
HttpRequestArgs,
} from './types';
import { Format } from './enums/format';
import { AttributeNames } from './enums/attributeNames';
import { Format } from './enums/Format';
import { AttributeNames } from './enums/AttributeNames';
import { Utils } from './utils';

/**
Expand Down
4 changes: 2 additions & 2 deletions packages/opentelemetry-plugin-http/src/index.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
export * from './http';
export * from './types';
export * from './utils';
export * from './enums/attributeNames';
export * from './enums/format';
export * from './enums/AttributeNames';
export * from './enums/Format';
3 changes: 1 addition & 2 deletions packages/opentelemetry-plugin-http/src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import {
IncomingHttpHeaders,
} from 'http';
import { IgnoreMatcher } from './types';
import { AttributeNames } from './enums/attributeNames';
import { AttributeNames } from './enums/AttributeNames';
import * as url from 'url';

/**
Expand Down Expand Up @@ -140,7 +140,6 @@ export class Utils {
static setSpanOnError(span: Span, obj: IncomingMessage | ClientRequest) {
obj.on('error', error => {
span.setAttributes({
[AttributeNames.ERROR]: true,
[AttributeNames.HTTP_ERROR_NAME]: error.name,
[AttributeNames.HTTP_ERROR_MESSAGE]: error.message,
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ import { DummyPropagation } from '../utils/DummyPropagation';
import { httpRequest } from '../utils/httpRequest';
import { TracerTest } from '../utils/TracerTest';
import { SpanAuditProcessor } from '../utils/SpanAuditProcessor';
import * as url from 'url';

let server: http.Server;
const serverPort = 12345;
Expand Down Expand Up @@ -279,127 +278,5 @@ describe('HttpPlugin', () => {
assert.strictEqual(spans.length, 0);
});
}

it('should create a rootSpan for GET requests and add propagation headers', async () => {
nock.enableNetConnect();
const spans = audit.processSpans();
assert.strictEqual(spans.length, 0);
await httpRequest.get(`http://google.fr/?query=test`).then(result => {
const spans = audit.processSpans();
assert.strictEqual(spans.length, 1);
assert.ok(spans[0].name.indexOf('GET /') >= 0);

const span = spans[0];
const validations = {
hostname: 'google.fr',
httpStatusCode: result.statusCode!,
httpMethod: 'GET',
pathname: '/',
path: '/?query=test',
resHeaders: result.resHeaders,
reqHeaders: result.reqHeaders,
};
assertSpan(span, SpanKind.CLIENT, validations);
});
nock.disableNetConnect();
});

it('custom attributes should show up on client spans', async () => {
nock.enableNetConnect();

await httpRequest.get(`http://google.fr/`).then(result => {
const spans = audit.processSpans();
assert.strictEqual(spans.length, 1);
assert.ok(spans[0].name.indexOf('GET /') >= 0);

const span = spans[0];
const validations = {
hostname: 'google.fr',
httpStatusCode: result.statusCode!,
httpMethod: 'GET',
pathname: '/',
resHeaders: result.resHeaders,
reqHeaders: result.reqHeaders,
};
assert.strictEqual(span.attributes['span kind'], SpanKind.CLIENT);
assertSpan(span, SpanKind.CLIENT, validations);
});
nock.disableNetConnect();
});

it('should create a span for GET requests and add propagation headers with Expect headers', async () => {
nock.enableNetConnect();
const spans = audit.processSpans();
assert.strictEqual(spans.length, 0);
const options = Object.assign(
{ headers: { Expect: '100-continue' } },
url.parse('http://google.fr/')
);
await httpRequest.get(options).then(result => {
const spans = audit.processSpans();
assert.strictEqual(spans.length, 1);
assert.ok(spans[0].name.indexOf('GET /') >= 0);

const span = spans[0];
const validations = {
hostname: 'google.fr',
httpStatusCode: 301,
httpMethod: 'GET',
pathname: '/',
resHeaders: result.resHeaders,
reqHeaders: result.reqHeaders,
};
assertSpan(span, SpanKind.CLIENT, validations);
});
nock.disableNetConnect();
});
for (const headers of [
{ Expect: '100-continue', 'user-agent': 'http-plugin-test' },
{ 'user-agent': 'http-plugin-test' },
]) {
it(`should create a span for GET requests and add propagation when using the following signature: http.get(url, options, callback) and following headers: ${JSON.stringify(
headers
)}`, done => {
nock.enableNetConnect();
const spans = audit.processSpans();
assert.strictEqual(spans.length, 0);
const options = { headers };
http.get('http://google.fr/', options, (resp: http.IncomingMessage) => {
const res = (resp as unknown) as http.IncomingMessage & {
req: http.IncomingMessage;
};
let data = '';
resp.on('data', chunk => {
data += chunk;
});
resp.on('end', () => {
const spans = audit.processSpans();
assert.strictEqual(spans.length, 1);
assert.ok(spans[0].name.indexOf('GET /') >= 0);
const validations = {
hostname: 'google.fr',
httpStatusCode: 301,
httpMethod: 'GET',
pathname: '/',
resHeaders: resp.headers,
/* tslint:disable:no-any */
reqHeaders: (res.req as any).getHeaders
? (res.req as any).getHeaders()
: (res.req as any)._headers,
/* tslint:enable:no-any */
};
assert.ok(data);
assert.ok(
validations.reqHeaders[DummyPropagation.TRACE_CONTEXT_KEY]
);
assert.ok(
validations.reqHeaders[DummyPropagation.SPAN_CONTEXT_KEY]
);
done();
});
});
nock.disableNetConnect();
});
}
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import { httpRequest } from '../utils/httpRequest';
import { TracerTest } from '../utils/TracerTest';
import { SpanAuditProcessor } from '../utils/SpanAuditProcessor';
import * as url from 'url';
import { Utils } from '../utils/Utils';

const serverPort = 12345;
const hostname = 'localhost';
Expand All @@ -37,6 +38,22 @@ export const customAttributeFunction = (span: Span): void => {

describe('HttpPlugin Integration tests', () => {
describe('enable()', () => {
before(function(done) {
// mandatory
if (process.env.CI) {
done();
return;
}

Utils.checkInternet(isConnected => {
if (!isConnected) {
this.skip();
// don't disturbe people
}
done();
});
});

const scopeManager = new AsyncHooksScopeManager();
const httpTextFormat = new DummyPropagation();
const logger = new NoopLogger();
Expand Down Expand Up @@ -134,7 +151,13 @@ describe('HttpPlugin Integration tests', () => {
resHeaders: result.resHeaders,
reqHeaders: result.reqHeaders,
};
assertSpan(span, SpanKind.CLIENT, validations);
try {
assertSpan(span, SpanKind.CLIENT, validations);
} catch (error) {
// temporary redirect is also correct
validations.httpStatusCode = 307;
assertSpan(span, SpanKind.CLIENT, validations);
}
});
});
for (const headers of [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
* limitations under the License.
*/
import { SpanContext, HttpTextFormat } from '@opentelemetry/types';
// import * as http from 'http';

export class DummyPropagation implements HttpTextFormat {
static TRACE_CONTEXT_KEY = 'x-dummy-trace-id';
Expand Down
29 changes: 29 additions & 0 deletions packages/opentelemetry-plugin-http/test/utils/Utils.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
/**
* Copyright 2019, OpenTelemetry Authors
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

import * as dns from 'dns';

export class Utils {
static checkInternet(cb: (isConnected: boolean) => void) {
dns.lookup('google.com', err => {
if (err && err.code === 'ENOTFOUND') {
cb(false);
} else {
cb(true);
}
});
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
import { SpanKind } from '@opentelemetry/types';
import * as assert from 'assert';
import * as http from 'http';
import { AttributeNames } from '../../src/enums/attributeNames';
import { AttributeNames } from '../../src/enums/AttributeNames';
import { HttpPlugin } from '../../src/http';
import { Utils } from '../../src/utils';
import { DummyPropagation } from './DummyPropagation';
Expand Down

0 comments on commit 23f745b

Please sign in to comment.