chore: require value in setAttribute (#1851)

Co-authored-by: Gerhard Stöbich <[email protected]> Co-authored-by: Valentin Marchaud <[email protected]>
open-telemetry · Jan 30, 2021 · 072e38c · 072e38c
1 parent f4e7115
commit 072e38c
Show file tree

Hide file tree

Showing 5 changed files with 23 additions and 12 deletions.
diff --git a/packages/opentelemetry-api/src/trace/span.ts b/packages/opentelemetry-api/src/trace/span.ts
@@ -50,7 +50,7 @@ export interface Span {
    * @param value the value for this attribute. Setting a value null or
    *              undefined is invalid and will result in undefined behavior.
    */
-  setAttribute(key: string, value?: AttributeValue): this;
+  setAttribute(key: string, value: AttributeValue): this;
 
   /**
    * Sets attributes to the span.

diff --git a/packages/opentelemetry-instrumentation-fetch/src/fetch.ts b/packages/opentelemetry-instrumentation-fetch/src/fetch.ts
@@ -112,7 +112,9 @@ export class FetchInstrumentation extends InstrumentationBase<
   ): void {
     const parsedUrl = web.parseUrl(response.url);
     span.setAttribute(HttpAttribute.HTTP_STATUS_CODE, response.status);
-    span.setAttribute(HttpAttribute.HTTP_STATUS_TEXT, response.statusText);
+    if (response.statusText != undefined) {
+      span.setAttribute(HttpAttribute.HTTP_STATUS_TEXT, response.statusText);
+    }
     span.setAttribute(HttpAttribute.HTTP_HOST, parsedUrl.host);
     span.setAttribute(
       HttpAttribute.HTTP_SCHEME,

diff --git a/packages/opentelemetry-instrumentation-xml-http-request/src/xhr.ts b/packages/opentelemetry-instrumentation-xml-http-request/src/xhr.ts
@@ -144,9 +144,12 @@ export class XMLHttpRequestInstrumentation extends InstrumentationBase<XMLHttpRe
   _addFinalSpanAttributes(span: api.Span, xhrMem: XhrMem, spanUrl?: string) {
     if (typeof spanUrl === 'string') {
       const parsedUrl = parseUrl(spanUrl);
-
-      span.setAttribute(HttpAttribute.HTTP_STATUS_CODE, xhrMem.status);
-      span.setAttribute(HttpAttribute.HTTP_STATUS_TEXT, xhrMem.statusText);
+      if (xhrMem.status !== undefined) {
+        span.setAttribute(HttpAttribute.HTTP_STATUS_CODE, xhrMem.status);
+      }
+      if (xhrMem.statusText !== undefined) {
+        span.setAttribute(HttpAttribute.HTTP_STATUS_TEXT, xhrMem.statusText);
+      }
       span.setAttribute(HttpAttribute.HTTP_HOST, parsedUrl.host);
       span.setAttribute(
         HttpAttribute.HTTP_SCHEME,

diff --git a/packages/opentelemetry-instrumentation-xml-http-request/test/xhr.test.ts b/packages/opentelemetry-instrumentation-xml-http-request/test/xhr.test.ts
@@ -789,29 +789,34 @@ describe('xhr', () => {
             );
             assert.strictEqual(
               attributes[keys[2]],
+              0,
+              `attributes ${HttpAttribute.HTTP_REQUEST_CONTENT_LENGTH} is wrong`
+            );
+            assert.strictEqual(
+              attributes[keys[3]],
               400,
               `attributes ${HttpAttribute.HTTP_STATUS_CODE} is wrong`
             );
             assert.strictEqual(
-              attributes[keys[3]],
+              attributes[keys[4]],
               'Bad Request',
               `attributes ${HttpAttribute.HTTP_STATUS_TEXT} is wrong`
             );
             assert.strictEqual(
-              attributes[keys[4]],
+              attributes[keys[5]],
               'raw.githubusercontent.com',
               `attributes ${HttpAttribute.HTTP_HOST} is wrong`
             );
             assert.ok(
-              attributes[keys[5]] === 'http' || attributes[keys[5]] === 'https',
+              attributes[keys[6]] === 'http' || attributes[keys[6]] === 'https',
               `attributes ${HttpAttribute.HTTP_SCHEME} is wrong`
             );
             assert.ok(
-              attributes[keys[6]] !== '',
+              attributes[keys[7]] !== '',
               `attributes ${HttpAttribute.HTTP_USER_AGENT} is not defined`
             );
 
-            assert.strictEqual(keys.length, 7, 'number of attributes is wrong');
+            assert.strictEqual(keys.length, 8, 'number of attributes is wrong');
           });
 
           it('span should have correct events', () => {

diff --git a/packages/opentelemetry-web/src/utils.ts b/packages/opentelemetry-web/src/utils.ts
@@ -80,10 +80,11 @@ export function addSpanNetworkEvents(
   addSpanNetworkEvent(span, PTN.REQUEST_START, resource);
   addSpanNetworkEvent(span, PTN.RESPONSE_START, resource);
   addSpanNetworkEvent(span, PTN.RESPONSE_END, resource);
-  if (resource[PTN.ENCODED_BODY_SIZE]) {
+  const contentLength = resource[PTN.ENCODED_BODY_SIZE];
+  if (contentLength !== undefined) {
     span.setAttribute(
       HttpAttribute.HTTP_RESPONSE_CONTENT_LENGTH,
-      resource[PTN.ENCODED_BODY_SIZE]
+      contentLength
     );
   }
 }