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.

Sign up for GitHub

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(opentelemetry-instrumentation-document-load): updated instrumenta… #1075

Closed
wants to merge 1 commit into from
Closed

fix(opentelemetry-instrumentation-document-load): updated instrumenta… #1075

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
15 changes: 11 additions & 4 deletions plugins/web/opentelemetry-instrumentation-document-load/src/instrumentation.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ export class DocumentLoadInstrumentation extends InstrumentationBase<unknown> {
if (fetchSpan) {
context.with(trace.setSpan(context.active(), fetchSpan), () => {
addSpanNetworkEvents(fetchSpan, entries);
this._endSpan(fetchSpan, PTN.RESPONSE_END, entries);
this._endSpan(fetchSpan, PTN.RESPONSE_END, PTN.FETCH_START, entries);
});
}
});
Expand Down Expand Up @@ -141,25 +141,32 @@ export class DocumentLoadInstrumentation extends InstrumentationBase<unknown> {

addSpanPerformancePaintEvents(rootSpan);

this._endSpan(rootSpan, PTN.LOAD_EVENT_END, entries);
this._endSpan(rootSpan, PTN.LOAD_EVENT_END, PTN.FETCH_START, entries);
});
}

/**
* Helper function for ending span
* @param span
* @param performanceName name of performance entry for time end
* @param startPerformanceName name of the performance entry that has the time start.
* @param entries
*/
private _endSpan(
span: Span | undefined,
performanceName: string,
startPerformanceName: string,
entries: PerformanceEntries
) {
// span can be undefined when entries are missing the certain performance - the span will not be created
if (span) {
if (hasKey(entries, performanceName)) {
span.end(entries[performanceName]);
// @ts-ignore: Object is possibly 'null'.
if(hasKey(entries, startPerformanceName) && entries[startPerformanceName] > entries[performanceName]) {
span.end(entries[startPerformanceName]);
} else {
span.end(entries[performanceName]);
}
Copy link
Author

Choose a reason for hiding this comment

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

I could not seem to access the span end timestamp here which is why I'm passing the original start value to check against.

} else {
// just end span
span.end();
Expand All @@ -185,7 +192,7 @@ export class DocumentLoadInstrumentation extends InstrumentationBase<unknown> {
if (span) {
span.setAttribute(SemanticAttributes.HTTP_URL, resource.name);
addSpanNetworkEvents(span, resource);
this._endSpan(span, PTN.RESPONSE_END, resource);
this._endSpan(span, PTN.RESPONSE_END, PTN.FETCH_START, resource);
}
}

Expand Down
49 changes: 49 additions & 0 deletions plugins/web/opentelemetry-instrumentation-document-load/test/documentLoad.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,32 @@ const resourcesNoSecureConnectionStart = [
serverTiming: [],
},
];
const resourcesFetchAfterEnd = [
{
name: 'http://localhost:8090/bundle.js',
entryType: 'resource',
startTime: 20.985000010114163,
duration: 90.94999998342246,
initiatorType: 'script',
nextHopProtocol: 'http/1.1',
workerStart: 0,
redirectStart: 0,
redirectEnd: 0,
fetchStart: 120.985000010114163,
domainLookupStart: 20.985000010114163,
domainLookupEnd: 20.985000010114163,
connectStart: 20.985000010114163,
connectEnd: 20.985000010114163,
secureConnectionStart: 0,
requestStart: 29.28999997675419,
responseStart: 31.88999998383224,
responseEnd: 32.93499999353662,
transferSize: 1446645,
encodedBodySize: 1446396,
decodedBodySize: 1446396,
serverTiming: [],
},
];
const entries = {
name: 'http://localhost:8090/',
entryType: 'navigation',
Expand Down Expand Up @@ -321,6 +347,29 @@ describe('DocumentLoad Instrumentation', () => {
});
});

describe('when navigation entries have bad timings', () => {
let spyEntries: sinon.SinonStub;
beforeEach(() => {
spyEntries = sandbox.stub(window.performance, 'getEntriesByType');
spyEntries.withArgs('navigation').returns([entries]);
spyEntries.withArgs('resource').returns(resourcesFetchAfterEnd);
spyEntries.withArgs('paint').returns(paintEntries);
});
afterEach(() => {
spyEntries.restore();
});

it('should never have negative duration', done => {
plugin.enable();

setTimeout(() => {
const fetchSpan = exporter.getFinishedSpans()[1] as ReadableSpan;
assert.deepEqual(fetchSpan.duration, [0,0]);
done();
});
});
});

describe('when navigation entries types are available', () => {
let spyEntries: sinon.SinonStub;
beforeEach(() => {
Expand Down