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

fix(plugin-fetch): check if PerformanceObserver exists #1662

Merged
merged 12 commits into from
Jan 20, 2021
Merged
12 changes: 11 additions & 1 deletion packages/opentelemetry-plugin-fetch/src/fetch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,9 @@ export class FetchPlugin extends core.BasePlugin<Promise<Response>> {
): void {
let resources: PerformanceResourceTiming[] = resourcesObserver.entries;
if (!resources.length) {
if (!performance.getEntriesByType) {
Copy link
Member

Choose a reason for hiding this comment

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

Please add unit test for that - use sandbox

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done, wanted to do a cleaner solution but ran into a bunch of issues

return;
}
// fallback - either Observer is not available or it took longer
// then OBSERVER_WAIT_TIME_MS and observer didn't collect enough
// information
Expand Down Expand Up @@ -225,7 +228,9 @@ export class FetchPlugin extends core.BasePlugin<Promise<Response>> {
response: FetchResponse
) {
const endTime = core.hrTime();
spanData.observer.disconnect();
if (spanData.observer) {
Copy link
Member

Choose a reason for hiding this comment

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

you can shorten the whole if to one liner as it was previously just add ?

Suggested change
if (spanData.observer) {
spanData.observer?.disconnect();

spanData.observer.disconnect();
}
this._addFinalSpanAttributes(span, response);

setTimeout(() => {
Expand Down Expand Up @@ -323,6 +328,11 @@ export class FetchPlugin extends core.BasePlugin<Promise<Response>> {
private _prepareSpanData(spanUrl: string): SpanData {
const startTime = core.hrTime();
const entries: PerformanceResourceTiming[] = [];

if (typeof window.PerformanceObserver === 'undefined') {
return { entries, startTime, spanUrl };
}

const observer: PerformanceObserver = new PerformanceObserver(list => {
const entries = list.getEntries() as PerformanceResourceTiming[];
entries.forEach(entry => {
Expand Down
2 changes: 1 addition & 1 deletion packages/opentelemetry-plugin-fetch/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ export interface FetchError {
*/
export interface SpanData {
entries: PerformanceResourceTiming[];
observer: PerformanceObserver;
observer?: PerformanceObserver;
spanUrl: string;
startTime: api.HrTime;
}
18 changes: 18 additions & 0 deletions packages/opentelemetry-plugin-fetch/test/fetch.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -583,4 +583,22 @@ describe('fetch', () => {
);
});
});

describe('when PerformanceObserver is undefined', () => {
beforeEach(done => {
Object.defineProperty(window, 'PerformanceObserver', {
Copy link
Member

Choose a reason for hiding this comment

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

please use sandbox.stub for that

value: undefined,
});

prepareData(done, url, {});
});

afterEach(() => {
clearData();
});

it('should still create spans', () => {
assert.strictEqual(exportSpy.args.length, 2, 'no spans created');
});
});
});