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
Show file tree
Hide file tree
Changes from 7 commits
Commits
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
11 changes: 10 additions & 1 deletion packages/opentelemetry-plugin-fetch/src/fetch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,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 @@ -224,7 +227,8 @@ export class FetchPlugin extends core.BasePlugin<Promise<Response>> {
response: FetchResponse
) {
const endTime = core.hrTime();
spanData.observer.disconnect();
spanData.observer?.disconnect();

this._addFinalSpanAttributes(span, response);

setTimeout(() => {
Expand Down Expand Up @@ -322,6 +326,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;
}
55 changes: 52 additions & 3 deletions packages/opentelemetry-plugin-fetch/test/fetch.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,9 @@ describe('fetch', () => {
done: any,
fileUrl: string,
config: FetchPluginConfig,
method?: string
method?: string,
disablePerfObserver?: boolean,
disableGetEntries?: boolean
) => {
sandbox = sinon.createSandbox();
sandbox.useFakeTimers();
Expand Down Expand Up @@ -162,8 +164,16 @@ describe('fetch', () => {
})
);

const spyEntries = sandbox.stub(performance, 'getEntriesByType');
spyEntries.withArgs('resource').returns(resources);
if (disablePerfObserver) {
sandbox.stub(window, 'PerformanceObserver').value(undefined);
}
if (disableGetEntries) {
sandbox.stub(performance, 'getEntriesByType').value(undefined);
} else {
const spyEntries = sandbox.stub(performance, 'getEntriesByType');
spyEntries.withArgs('resource').returns(resources);
}

fetchPlugin = new FetchPlugin(config);
webTracerProviderWithZone = new WebTracerProvider({
logLevel: core.LogLevel.ERROR,
Expand Down Expand Up @@ -583,4 +593,43 @@ describe('fetch', () => {
);
});
});

describe('when PerformanceObserver is undefined', () => {
beforeEach(done => {
prepareData(done, url, {}, undefined, true, false);
});

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

it('should still create spans', () => {
assert.strictEqual(
exportSpy.args.length,
2,
`Wrong number of spans: ${exportSpy.args.length}`
);
});
});

describe('when PerformanceObserver and performance.getEntriesByType are undefined', () => {
beforeEach(done => {
prepareData(done, url, {}, undefined, true, true);
});
afterEach(() => {
clearData();
});
it('should capture fetch without preflight', () => {
assert.strictEqual(
exportSpy.args.length,
1,
`Wrong number of spans: ${exportSpy.args.length}`
);
assert.strictEqual(
exportSpy.args[0][0][0].name,
'HTTP GET',
'wrong span captured'
);
});
});
});