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.

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

feat(plugin-document-load): new plugin for document load for web tracer #433

Merged
merged 41 commits into from
Oct 23, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
41 commits
Select commit Hold shift + click to select a range
df5ca9a
feat(plugin-document-load): new plugin for document load for web tracer
obecny Oct 14, 2019
a718b74
chore: lint
obecny Oct 15, 2019
fe04870
chore: removing unused dependency
obecny Oct 15, 2019
4563aa9
chore: adding prepare script
obecny Oct 15, 2019
0c435a0
chore: cleanup of not used span processor
obecny Oct 15, 2019
8301b03
chore: merging exporter-console into tracing
obecny Oct 15, 2019
f6a526e
chore: fixing timeOrigin when browser is using older version of perfo…
obecny Oct 15, 2019
edd9822
chore: removing @private
obecny Oct 15, 2019
6752333
chore: cleaning the docs
obecny Oct 15, 2019
1d6593d
chore: using stubs on public instead of private
obecny Oct 15, 2019
da85aa4
chore: added explanation when span can be undefined
obecny Oct 15, 2019
3aaf306
Merge branch 'master' into plugin-document-load
obecny Oct 15, 2019
2dba249
chore: adding unit test for case when passed "performanceNow" is equa…
obecny Oct 15, 2019
9c3f616
Merge branch 'master' into plugin-document-load
obecny Oct 15, 2019
ba49890
Merge branch 'plugin-document-load' of github.com:obecny/opentelemetr…
obecny Oct 15, 2019
2880dc3
chore: adding unit test for case when passed "performanceNow" is null…
obecny Oct 15, 2019
e6abf4e
chore: fixing unit test with null
obecny Oct 15, 2019
c80b4b5
chore: merge branch 'master' into plugin-document-loa
obecny Oct 16, 2019
921049d
chore: bump version
obecny Oct 16, 2019
da2f9bb
chore: after changing enum keys to capitals I had to use values to al…
obecny Oct 16, 2019
c1b17fb
chore: adding comments for interfaces
obecny Oct 17, 2019
486a740
feat: adding possibility of setting start time for event
obecny Oct 21, 2019
2607a8d
chore: refactoring document load to use events instead of new spans
obecny Oct 21, 2019
ec5cb9d
chore: reformatting
obecny Oct 21, 2019
cb6fe89
chore: merge branch 'master' into plugin-document-load
obecny Oct 21, 2019
a67db85
chore: updating loop
obecny Oct 21, 2019
ed4a0b5
chore: changing type for time
obecny Oct 21, 2019
5090b54
chore: refactoring loop, updating jsdoc
obecny Oct 21, 2019
8a38ca3
chore: splitting events into 2 spans
obecny Oct 21, 2019
cfcee4a
chore: adding possibility of calling addEvent with 2nd param as time
obecny Oct 21, 2019
9bb6197
chore: updating the last event to be "load end"
obecny Oct 22, 2019
d1502b3
chore: updating the name for attributes
obecny Oct 22, 2019
036944a
chore: fixing test
obecny Oct 22, 2019
006a467
chore: cleanups
obecny Oct 22, 2019
077eec2
chore: adding isTimeInput function with unit tests
obecny Oct 22, 2019
42aae8d
Merge branch 'master' into plugin-document-load
obecny Oct 22, 2019
eb30f1a
Merge branch 'master' into plugin-document-load
mayurkale22 Oct 23, 2019
734ffea
chore: adding component name
obecny Oct 23, 2019
31f8cf0
Merge branch 'plugin-document-load' of github.com:obecny/opentelemetr…
obecny Oct 23, 2019
7e4aab1
chore: adding license and readme
obecny Oct 23, 2019
abf408b
chore: updating lint and docs jobs to use node12 image in circleci
obecny Oct 23, 2019
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
4 changes: 2 additions & 2 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ browsers_unit_tests: &browsers_unit_tests
jobs:
lint:
docker:
- image: node
- image: node:12
steps:
- checkout
- run:
Expand All @@ -59,7 +59,7 @@ jobs:
command: yarn run check
docs:
docker:
- image: node
- image: node:12
steps:
- checkout
- run:
Expand Down
4 changes: 2 additions & 2 deletions examples/tracer-web/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,14 @@

<head>
<meta charset="utf-8">
<title>JS Example</title>
<title>Web Tracer Example</title>
<base href="/">

<meta name="viewport" content="width=device-width, initial-scale=1">
</head>

<body>
Testing, debugging in development
Example of using Web Tracer with document load plugin and console exporter
<script type="text/javascript" src="/bundle.js"></script>
</body>

Expand Down
41 changes: 7 additions & 34 deletions examples/tracer-web/index.js
Original file line number Diff line number Diff line change
@@ -1,38 +1,11 @@
import { ConsoleSpanExporter, SimpleSpanProcessor } from '@opentelemetry/tracing';
import { WebTracer } from '@opentelemetry/web';
import { DocumentLoad } from '@opentelemetry/plugin-document-load';

import * as shimmer from 'shimmer';

class Tester {
constructor() {
}
add(name) {
console.log('calling add', name);
}
}

const tester = new Tester();

const webTracer = new WebTracer();
const span = webTracer.startSpan('span1');

shimmer.wrap(Tester.prototype, 'add', (originalFunction) => {
return function patchedFunction() {
try {
span.addEvent('start');
} catch (e) {
console.log('error', e);
} finally {
const result = originalFunction.apply(this, arguments);
span.addEvent('after call');
span.end();
return result;
}
};
});

webTracer.withSpan(span, function () {
console.log(this === span);
const webTracer = new WebTracer({
plugins: [
new DocumentLoad()
]
});

tester.add('foo');
console.log(span);
webTracer.addSpanProcessor(new SimpleSpanProcessor(new ConsoleSpanExporter()));
7 changes: 4 additions & 3 deletions examples/tracer-web/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -26,16 +26,17 @@
},
"devDependencies": {
"@babel/core": "^7.6.0",
"@types/shimmer": "^1.0.1",
"babel-loader": "^8.0.6",
"shimmer": "^1.2.0",
"ts-loader": "^6.0.4",
"webpack": "^4.35.2",
"webpack-cli": "^3.3.9",
"webpack-dev-server": "^3.8.1",
"webpack-merge": "^4.2.2"
},
"dependencies": {
"@opentelemetry/plugin-document-load": "^0.1.1",
"@opentelemetry/tracing": "^0.1.1",
"@opentelemetry/web": "^0.1.1"
},
"homepage": "https://github.com/open-telemetry/opentelemetry-js#readme"
}
}
45 changes: 37 additions & 8 deletions packages/opentelemetry-core/src/common/time.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

import * as types from '@opentelemetry/types';
import { otperformance as performance } from '../platform';
import { TimeOriginLegacy } from './types';

const NANOSECOND_DIGITS = 9;
const SECOND_TO_NANOSECONDS = Math.pow(10, NANOSECOND_DIGITS);
Expand All @@ -32,10 +33,21 @@ function numberToHrtime(epochMillis: number): types.HrTime {
return [seconds, nanos];
}

function getTimeOrigin(): number {
let timeOrigin = performance.timeOrigin;
if (typeof timeOrigin !== 'number') {
const perf: TimeOriginLegacy = (performance as unknown) as TimeOriginLegacy;
timeOrigin = perf.timing && perf.timing.fetchStart;
}
return timeOrigin;
}

// Returns an hrtime calculated via performance component.
export function hrTime(performanceNow?: number): types.HrTime {
const timeOrigin = numberToHrtime(performance.timeOrigin);
const now = numberToHrtime(performanceNow || performance.now());
const timeOrigin = numberToHrtime(getTimeOrigin());
const now = numberToHrtime(
typeof performanceNow === 'number' ? performanceNow : performance.now()
obecny marked this conversation as resolved.
Show resolved Hide resolved
);

let seconds = timeOrigin[0] + now[0];
let nanos = timeOrigin[1] + now[1];
Expand All @@ -52,15 +64,14 @@ export function hrTime(performanceNow?: number): types.HrTime {
// Converts a TimeInput to an HrTime, defaults to _hrtime().
export function timeInputToHrTime(time: types.TimeInput): types.HrTime {
// process.hrtime
if (Array.isArray(time)) {
return time;
if (isTimeInputHrTime(time)) {
return time as types.HrTime;
} else if (typeof time === 'number') {
// Must be a performance.now() if it's smaller than process start time.
if (time < performance.timeOrigin) {
if (time < getTimeOrigin()) {
return hrTime(time);
}
// epoch milliseconds or performance.timeOrigin
else {
} else {
// epoch milliseconds or performance.timeOrigin
return numberToHrtime(time);
}
} else if (time instanceof Date) {
Expand Down Expand Up @@ -102,3 +113,21 @@ export function hrTimeToMilliseconds(hrTime: types.HrTime): number {
export function hrTimeToMicroseconds(hrTime: types.HrTime): number {
return Math.round(hrTime[0] * 1e6 + hrTime[1] / 1e3);
}

export function isTimeInputHrTime(value: unknown) {
return (
Array.isArray(value) &&
value.length === 2 &&
typeof value[0] === 'number' &&
typeof value[1] === 'number'
);
}

// check if input value is a correct types.TimeInput
export function isTimeInput(value: unknown) {
return (
isTimeInputHrTime(value) ||
typeof value === 'number' ||
value instanceof Date
);
}
11 changes: 11 additions & 0 deletions packages/opentelemetry-core/src/common/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,3 +21,14 @@ export enum LogLevel {
INFO,
DEBUG,
}

/**
* This interface defines a fallback to read a timeOrigin when it is not available on performance.timeOrigin,
* this happens for example on Safari Mac
* then the timeOrigin is taken from fetchStart - which is the closest to timeOrigin
*/
export interface TimeOriginLegacy {
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you add a comment for this interface to explain its purpose?

Copy link
Member Author

Choose a reason for hiding this comment

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

@draffensperger What do you think of that? Did folks discuss this at all at the SIG? we didn't , pls see my response below

Copy link
Contributor

Choose a reason for hiding this comment

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

Looks good, thanks.

timing: {
fetchStart: number;
};
}
70 changes: 70 additions & 0 deletions packages/opentelemetry-core/test/common/time.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import {
hrTimeToNanoseconds,
hrTimeToMilliseconds,
hrTimeToMicroseconds,
isTimeInput,
} from '../../src/common/time';

describe('time', () => {
Expand Down Expand Up @@ -62,6 +63,47 @@ describe('time', () => {
const output = hrTime(performanceNow);
assert.deepStrictEqual(output, [0, 23100000]);
});

it('should allow passed "performanceNow" equal to 0', () => {
sandbox.stub(performance, 'timeOrigin').value(11.5);
sandbox.stub(performance, 'now').callsFake(() => 11.3);

const output = hrTime(0);
assert.deepStrictEqual(output, [0, 11500000]);
});

it('should use performance.now() when "performanceNow" is equal to undefined', () => {
sandbox.stub(performance, 'timeOrigin').value(11.5);
sandbox.stub(performance, 'now').callsFake(() => 11.3);

const output = hrTime(undefined);
assert.deepStrictEqual(output, [0, 22800000]);
});

it('should use performance.now() when "performanceNow" is equal to null', () => {
sandbox.stub(performance, 'timeOrigin').value(11.5);
sandbox.stub(performance, 'now').callsFake(() => 11.3);

const output = hrTime(null as any);
assert.deepStrictEqual(output, [0, 22800000]);
});

describe('when timeOrigin is not available', () => {
it('should use the performance.timing.fetchStart as a fallback', () => {
Object.defineProperty(performance, 'timing', {
writable: true,
value: {
fetchStart: 11.5,
},
});

sandbox.stub(performance, 'timeOrigin').value(undefined);
sandbox.stub(performance, 'now').callsFake(() => 11.3);

const output = hrTime();
assert.deepStrictEqual(output, [0, 22800000]);
});
});
});

describe('#timeInputToHrTime', () => {
Expand Down Expand Up @@ -134,4 +176,32 @@ describe('time', () => {
assert.deepStrictEqual(output, 1200000);
});
});
describe('#isTimeInput', () => {
it('should return true for a number', () => {
assert.strictEqual(isTimeInput(12), true);
});
it('should return true for a date', () => {
assert.strictEqual(isTimeInput(new Date()), true);
});
it('should return true for an array with 2 elements type number', () => {
assert.strictEqual(isTimeInput([1, 1]), true);
});
it('should return FALSE for different cases for an array ', () => {
assert.strictEqual(isTimeInput([1, 1, 1]), false);
assert.strictEqual(isTimeInput([1]), false);
assert.strictEqual(isTimeInput([1, 'a']), false);
});
it('should return FALSE for a string', () => {
assert.strictEqual(isTimeInput('a'), false);
});
it('should return FALSE for an object', () => {
assert.strictEqual(isTimeInput({}), false);
});
it('should return FALSE for a null', () => {
assert.strictEqual(isTimeInput(null), false);
});
it('should return FALSE for undefined', () => {
assert.strictEqual(isTimeInput(undefined), false);
});
});
});
Loading