http-instrumentation: fix propagation errors when using Expect header #481 (#482)

* http-instrumentation: fix propagation errors when using Expect header #481

* chore: more details for the Expect header logic

* chore: address PR comments

Author: vmarchaud

packages/opencensus-instrumentation-http/src/http.ts

@@ -360,12 +360,28 @@ export class HttpPlugin extends BasePlugin {
 
       const setter: HeaderSetter = {
         setHeader(name: string, value: string) {
-          request.setHeader(name, value);
+          // If outgoing request headers contain the "Expect" header, the
+          // returned ClientRequest will throw an error if any new headers are
+          // added. We need to set the header directly in the headers object
+          // which has been cloned earlier.
+          if (plugin.hasExpectHeader(options) && options.headers) {
+            options.headers[name] = value;
+          } else {
+            request.setHeader(name, value);
+          }
         }
       };
 
       const propagation = plugin.tracer.propagation;
       if (propagation) {
+        // If outgoing request headers contain the "Expect" header, the returned
+        // ClientRequest will throw an error if any new headers are added.
+        // So we need to clone the options object to be able to inject new
+        // header.
+        if (plugin.hasExpectHeader(options)) {
+          options = Object.assign({}, options) as RequestOptions;
+          options.headers = Object.assign({}, options.headers);
+        }
         propagation.inject(setter, span.spanContext);
       }
 
@@ -485,6 +501,16 @@ export class HttpPlugin extends BasePlugin {
     } catch (ignore) {
     }
   }
+
+  /**
+   * Returns whether the Expect header is on the given options object.
+   * @param options Options for http.request.
+   */
+  hasExpectHeader(options: RequestOptions|url.URL): boolean {
+    return !!(
+        (options as RequestOptions).headers &&
+        (options as RequestOptions).headers!.Expect);
+  }
 }
 
 const plugin = new HttpPlugin('http');

packages/opencensus-instrumentation-http/test/test-http.ts

@@ -19,6 +19,7 @@ import * as assert from 'assert';
 import * as http from 'http';
 import * as nock from 'nock';
 import * as shimmer from 'shimmer';
+import * as url from 'url';
 
 import {HttpPlugin, plugin} from '../src/';
 import * as stats from '../src/http-stats';
@@ -374,6 +375,25 @@ describe('HttpPlugin', () => {
          });
          nock.disableNetConnect();
        });
+
+    it('should create a rootSpan for GET requests and add propagation headers with Expect headers',
+       async () => {
+         nock.enableNetConnect();
+         assert.strictEqual(rootSpanVerifier.endedRootSpans.length, 0);
+         const options = Object.assign(
+             {headers: {Expect: '100-continue'}},
+             url.parse('http://google.fr/'));
+         await httpRequest.get(options).then((result) => {
+           assert.strictEqual(rootSpanVerifier.endedRootSpans.length, 1);
+           assert.ok(
+               rootSpanVerifier.endedRootSpans[0].name.indexOf('GET /') >= 0);
+
+           const span = rootSpanVerifier.endedRootSpans[0];
+           assertSpanAttributes(span, 301, 'GET', 'google.fr', '/');
+           assertClientStats(testExporter, 301, 'GET');
+         });
+         nock.disableNetConnect();
+       });
   });