Skip to content

Commit 3c040c4

Browse files
chancancodechancancode
authored
fix(inst-fetch,inst-xhr) Ignore network events with zero-timing (#5332)
1 parent 1c6c39b commit 3c040c4

File tree

7 files changed

+208
-151
lines changed

7 files changed

+208
-151
lines changed

‎experimental/CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ All notable changes to experimental packages in this project will be documented
2525

2626
* fix(instrumentation-grpc): monitor error events with events.errorMonitor [#5369](https://github.com/open-telemetry/opentelemetry-js/pull/5369) @cjihrig
2727
* fix(exporter-metrics-otlp-http): browser OTLPMetricExporter was not passing config to OTLPMetricExporterBase super class [#5331](https://github.com/open-telemetry/opentelemetry-js/pull/5331) @trentm
28+
* fix(instrumentation-fetch, instrumentation-xhr): Ignore network events with zero-timings [#5332](https://github.com/open-telemetry/opentelemetry-js/pull/5332) @chancancode
2829

2930
### :books: (Refine Doc)
3031

‎experimental/packages/opentelemetry-instrumentation-fetch/test/fetch.test.ts‎

Lines changed: 27 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -90,27 +90,27 @@ const textToReadableStream = (msg: string): ReadableStream => {
9090

9191
const CUSTOM_ATTRIBUTE_KEY = 'span kind';
9292
const defaultResource = {
93-
connectEnd: 15,
94-
connectStart: 13,
95-
decodedBodySize: 0,
96-
domainLookupEnd: 12,
97-
domainLookupStart: 11,
98-
encodedBodySize: 0,
99-
fetchStart: 10.1,
93+
entryType: 'resource',
94+
name: '',
10095
initiatorType: 'fetch',
101-
nextHopProtocol: '',
102-
redirectEnd: 0,
96+
startTime: 10.1,
10397
redirectStart: 0,
98+
redirectEnd: 0,
99+
workerStart: 0,
100+
fetchStart: 10.1,
101+
domainLookupStart: 11,
102+
domainLookupEnd: 12,
103+
connectStart: 13,
104+
secureConnectionStart: 0,
105+
connectEnd: 15,
104106
requestStart: 16,
105-
responseEnd: 20.5,
106107
responseStart: 17,
107-
secureConnectionStart: 14,
108+
responseEnd: 20.5,
109+
duration: 10.4,
110+
decodedBodySize: 30,
111+
encodedBodySize: 30,
108112
transferSize: 0,
109-
workerStart: 0,
110-
duration: 0,
111-
entryType: '',
112-
name: '',
113-
startTime: 0,
113+
nextHopProtocol: '',
114114
};
115115

116116
function createResource(resource = {}): PerformanceResourceTiming {
@@ -124,7 +124,7 @@ function createResource(resource = {}): PerformanceResourceTiming {
124124
function createMainResource(resource = {}): PerformanceResourceTiming {
125125
const mainResource: any = createResource(resource);
126126
Object.keys(mainResource).forEach((key: string) => {
127-
if (typeof mainResource[key] === 'number') {
127+
if (typeof mainResource[key] === 'number' && mainResource[key] !== 0) {
128128
mainResource[key] = mainResource[key] + 30;
129129
}
130130
});
@@ -139,8 +139,14 @@ function createFakePerformanceObs(url: string) {
139139
const resources: PerformanceObserverEntryList = {
140140
getEntries(): PerformanceEntryList {
141141
return [
142-
createResource({ name: absoluteUrl }) as any,
143-
createMainResource({ name: absoluteUrl }) as any,
142+
createResource({
143+
name: absoluteUrl,
144+
[PTN.SECURE_CONNECTION_START]: url.startsWith('https:') ? 14 : 0,
145+
}) as any,
146+
createMainResource({
147+
name: absoluteUrl,
148+
[PTN.SECURE_CONNECTION_START]: url.startsWith('https:') ? 14 : 0,
149+
}) as any,
144150
];
145151
},
146152
getEntriesByName(): PerformanceEntryList {
@@ -458,7 +464,7 @@ describe('fetch', () => {
458464
] as number;
459465
assert.strictEqual(
460466
responseContentLength,
461-
30,
467+
60,
462468
`attributes ${SEMATTRS_HTTP_RESPONSE_CONTENT_LENGTH} is <= 0`
463469
);
464470

@@ -1217,7 +1223,7 @@ describe('fetch', () => {
12171223
] as number;
12181224
assert.strictEqual(
12191225
responseContentLength,
1220-
30,
1226+
60,
12211227
`attributes ${SEMATTRS_HTTP_RESPONSE_CONTENT_LENGTH} is <= 0`
12221228
);
12231229
});

‎experimental/packages/opentelemetry-instrumentation-xml-http-request/test/xhr.test.ts‎

Lines changed: 31 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -134,28 +134,37 @@ const postData = (
134134

135135
function createResource(resource = {}): PerformanceResourceTiming {
136136
const defaultResource = {
137-
connectEnd: 15,
138-
connectStart: 13,
139-
decodedBodySize: 0,
140-
domainLookupEnd: 12,
141-
domainLookupStart: 11,
142-
encodedBodySize: 0,
143-
fetchStart: 10.1,
137+
entryType: 'resource',
138+
name: '',
144139
initiatorType: 'xmlhttprequest',
145-
nextHopProtocol: '',
146-
redirectEnd: 0,
140+
startTime: 10.1,
147141
redirectStart: 0,
142+
redirectEnd: 0,
143+
workerStart: 0,
144+
fetchStart: 10.1,
145+
domainLookupStart: 11,
146+
domainLookupEnd: 12,
147+
connectStart: 13,
148+
secureConnectionStart: 0,
149+
connectEnd: 15,
148150
requestStart: 16,
149-
responseEnd: 20.5,
150151
responseStart: 17,
151-
secureConnectionStart: 14,
152+
responseEnd: 20.5,
153+
duration: 10.4,
154+
decodedBodySize: 30,
155+
encodedBodySize: 30,
152156
transferSize: 0,
153-
workerStart: 0,
154-
duration: 0,
155-
entryType: '',
156-
name: '',
157-
startTime: 0,
157+
nextHopProtocol: '',
158158
};
159+
160+
if (
161+
'name' in resource &&
162+
typeof resource.name === 'string' &&
163+
resource.name.startsWith('https:')
164+
) {
165+
defaultResource.secureConnectionStart = 14;
166+
}
167+
159168
return Object.assign(
160169
{},
161170
defaultResource,
@@ -166,7 +175,7 @@ function createResource(resource = {}): PerformanceResourceTiming {
166175
function createMainResource(resource = {}): PerformanceResourceTiming {
167176
const mainResource: any = createResource(resource);
168177
Object.keys(mainResource).forEach((key: string) => {
169-
if (typeof mainResource[key] === 'number') {
178+
if (typeof mainResource[key] === 'number' && mainResource[key] !== 0) {
170179
mainResource[key] = mainResource[key] + 30;
171180
}
172181
});
@@ -424,7 +433,7 @@ describe('xhr', () => {
424433
] as number;
425434
assert.strictEqual(
426435
responseContentLength,
427-
30,
436+
60,
428437
`attributes ${SEMATTRS_HTTP_RESPONSE_CONTENT_LENGTH} <= 0`
429438
);
430439
assert.strictEqual(
@@ -868,7 +877,7 @@ describe('xhr', () => {
868877
] as number;
869878
assert.strictEqual(
870879
responseContentLength,
871-
30,
880+
60,
872881
`attributes ${SEMATTRS_HTTP_RESPONSE_CONTENT_LENGTH} is <= 0`
873882
);
874883
});
@@ -1052,7 +1061,7 @@ describe('xhr', () => {
10521061
] as number;
10531062
assert.strictEqual(
10541063
responseContentLength,
1055-
0,
1064+
30,
10561065
`attributes ${SEMATTRS_HTTP_RESPONSE_CONTENT_LENGTH} <= 0`
10571066
);
10581067
assert.strictEqual(
@@ -1622,7 +1631,7 @@ describe('xhr', () => {
16221631
] as number;
16231632
assert.strictEqual(
16241633
responseContentLength,
1625-
30,
1634+
60,
16261635
`attributes ${SEMATTRS_HTTP_RESPONSE_CONTENT_LENGTH} <= 0`
16271636
);
16281637
assert.strictEqual(
@@ -2236,7 +2245,7 @@ describe('xhr', () => {
22362245
] as number;
22372246
assert.strictEqual(
22382247
responseContentLength,
2239-
0,
2248+
30,
22402249
`attributes ${SEMATTRS_HTTP_RESPONSE_CONTENT_LENGTH} <= 0`
22412250
);
22422251
assert.strictEqual(

‎packages/opentelemetry-sdk-trace-web/src/enums/PerformanceTimingNames.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ export enum PerformanceTimingNames {
3535
RESPONSE_END = 'responseEnd',
3636
RESPONSE_START = 'responseStart',
3737
SECURE_CONNECTION_START = 'secureConnectionStart',
38+
START_TIME = 'startTime',
3839
UNLOAD_EVENT_END = 'unloadEventEnd',
3940
UNLOAD_EVENT_START = 'unloadEventStart',
4041
}

‎packages/opentelemetry-sdk-trace-web/src/types.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ export type PerformanceEntries = {
3535
[PerformanceTimingNames.RESPONSE_END]?: number;
3636
[PerformanceTimingNames.RESPONSE_START]?: number;
3737
[PerformanceTimingNames.SECURE_CONNECTION_START]?: number;
38+
[PerformanceTimingNames.START_TIME]?: number;
3839
[PerformanceTimingNames.UNLOAD_EVENT_END]?: number;
3940
[PerformanceTimingNames.UNLOAD_EVENT_START]?: number;
4041
};

‎packages/opentelemetry-sdk-trace-web/src/utils.ts‎

Lines changed: 29 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -58,32 +58,22 @@ export function hasKey<O extends object>(
5858
* @param span
5959
* @param performanceName name of performance entry for time start
6060
* @param entries
61-
* @param refPerfName name of performance entry to use for reference
61+
* @param ignoreZeros
6262
*/
6363
export function addSpanNetworkEvent(
6464
span: api.Span,
6565
performanceName: string,
6666
entries: PerformanceEntries,
67-
refPerfName?: string
67+
ignoreZeros = true
6868
): api.Span | undefined {
69-
let perfTime: number | undefined;
70-
let refTime: number | undefined;
7169
if (
7270
hasKey(entries, performanceName) &&
73-
typeof entries[performanceName] === 'number'
71+
typeof entries[performanceName] === 'number' &&
72+
!(ignoreZeros && entries[performanceName] === 0)
7473
) {
75-
perfTime = entries[performanceName];
76-
}
77-
const refName = refPerfName || PTN.FETCH_START;
78-
// Use a reference time which is the earliest possible value so that the performance timings that are earlier should not be added
79-
// using FETCH START time in case no reference is provided
80-
if (hasKey(entries, refName) && typeof entries[refName] === 'number') {
81-
refTime = entries[refName];
82-
}
83-
if (perfTime !== undefined && refTime !== undefined && perfTime >= refTime) {
84-
span.addEvent(performanceName, perfTime);
85-
return span;
74+
return span.addEvent(performanceName, entries[performanceName]);
8675
}
76+
8777
return undefined;
8878
}
8979

@@ -92,32 +82,40 @@ export function addSpanNetworkEvent(
9282
* @param span
9383
* @param resource
9484
* @param ignoreNetworkEvents
85+
* @param ignoreZeros
9586
*/
9687
export function addSpanNetworkEvents(
9788
span: api.Span,
9889
resource: PerformanceEntries,
99-
ignoreNetworkEvents = false
90+
ignoreNetworkEvents = false,
91+
ignoreZeros?: boolean
10092
): void {
93+
if (ignoreZeros === undefined) {
94+
ignoreZeros = resource[PTN.START_TIME] !== 0;
95+
}
96+
10197
if (!ignoreNetworkEvents) {
102-
addSpanNetworkEvent(span, PTN.FETCH_START, resource);
103-
addSpanNetworkEvent(span, PTN.DOMAIN_LOOKUP_START, resource);
104-
addSpanNetworkEvent(span, PTN.DOMAIN_LOOKUP_END, resource);
105-
addSpanNetworkEvent(span, PTN.CONNECT_START, resource);
106-
if (
107-
hasKey(resource as PerformanceResourceTiming, 'name') &&
108-
(resource as PerformanceResourceTiming)['name'].startsWith('https:')
109-
) {
110-
addSpanNetworkEvent(span, PTN.SECURE_CONNECTION_START, resource);
111-
}
112-
addSpanNetworkEvent(span, PTN.CONNECT_END, resource);
113-
addSpanNetworkEvent(span, PTN.REQUEST_START, resource);
114-
addSpanNetworkEvent(span, PTN.RESPONSE_START, resource);
115-
addSpanNetworkEvent(span, PTN.RESPONSE_END, resource);
98+
addSpanNetworkEvent(span, PTN.FETCH_START, resource, ignoreZeros);
99+
addSpanNetworkEvent(span, PTN.DOMAIN_LOOKUP_START, resource, ignoreZeros);
100+
addSpanNetworkEvent(span, PTN.DOMAIN_LOOKUP_END, resource, ignoreZeros);
101+
addSpanNetworkEvent(span, PTN.CONNECT_START, resource, ignoreZeros);
102+
addSpanNetworkEvent(
103+
span,
104+
PTN.SECURE_CONNECTION_START,
105+
resource,
106+
ignoreZeros
107+
);
108+
addSpanNetworkEvent(span, PTN.CONNECT_END, resource, ignoreZeros);
109+
addSpanNetworkEvent(span, PTN.REQUEST_START, resource, ignoreZeros);
110+
addSpanNetworkEvent(span, PTN.RESPONSE_START, resource, ignoreZeros);
111+
addSpanNetworkEvent(span, PTN.RESPONSE_END, resource, ignoreZeros);
116112
}
113+
117114
const encodedLength = resource[PTN.ENCODED_BODY_SIZE];
118115
if (encodedLength !== undefined) {
119116
span.setAttribute(SEMATTRS_HTTP_RESPONSE_CONTENT_LENGTH, encodedLength);
120117
}
118+
121119
const decodedLength = resource[PTN.DECODED_BODY_SIZE];
122120
// Spec: Not set if transport encoding not used (in which case encoded and decoded sizes match)
123121
if (decodedLength !== undefined && encodedLength !== decodedLength) {

0 commit comments

Comments
 (0)