fix: add license header, rename enum files and refactoring

refactor: make integration tests mandatory for ci only remove duplicate tests remove dead comments Signed-off-by: Olivier Albertini <[email protected]>
open-telemetry · Aug 30, 2019 · 5b5dced · 5b5dced
1 parent d612642
commit 5b5dced
Show file tree

Hide file tree

Showing 10 changed files with 75 additions and 135 deletions.
diff --git a/...y-plugin-http/src/enums/attributeNames.ts → ...y-plugin-http/src/enums/AttributeNames.ts b/...y-plugin-http/src/enums/attributeNames.ts → ...y-plugin-http/src/enums/AttributeNames.ts
@@ -20,8 +20,6 @@
  */
 export enum AttributeNames {
   HTTP_HOSTNAME = 'http.hostname',
-  // NOT ON OFFICIAL SPEC
-  ERROR = 'error',
   COMPONENT = 'component',
   HTTP_METHOD = 'http.method',
   HTTP_PATH = 'http.path',

diff --git a/...telemetry-plugin-http/src/enums/format.ts → ...telemetry-plugin-http/src/enums/Format.ts b/...telemetry-plugin-http/src/enums/format.ts → ...telemetry-plugin-http/src/enums/Format.ts
@@ -14,7 +14,6 @@
  * limitations under the License.
  */
 
-// Should we export this to the types package in order to expose all values ?
 export enum Format {
   HTTP = 'HttpTraceContext',
 }
diff --git a/packages/opentelemetry-plugin-http/src/http.ts b/packages/opentelemetry-plugin-http/src/http.ts
@@ -34,8 +34,8 @@ import {
   ParsedRequestOptions,
   HttpRequestArgs,
 } from './types';
-import { Format } from './enums/format';
-import { AttributeNames } from './enums/attributeNames';
+import { Format } from './enums/Format';
+import { AttributeNames } from './enums/AttributeNames';
 import { Utils } from './utils';
 
 /**

diff --git a/packages/opentelemetry-plugin-http/src/index.ts b/packages/opentelemetry-plugin-http/src/index.ts
@@ -1,5 +1,21 @@
+/**
+ * Copyright 2019, OpenTelemetry Authors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
 export * from './http';
 export * from './types';
 export * from './utils';
-export * from './enums/attributeNames';
-export * from './enums/format';
+export * from './enums/AttributeNames';
+export * from './enums/Format';
diff --git a/packages/opentelemetry-plugin-http/src/utils.ts b/packages/opentelemetry-plugin-http/src/utils.ts
@@ -22,7 +22,7 @@ import {
   IncomingHttpHeaders,
 } from 'http';
 import { IgnoreMatcher } from './types';
-import { AttributeNames } from './enums/attributeNames';
+import { AttributeNames } from './enums/AttributeNames';
 import * as url from 'url';
 
 /**
@@ -140,7 +140,6 @@ export class Utils {
   static setSpanOnError(span: Span, obj: IncomingMessage | ClientRequest) {
     obj.on('error', error => {
       span.setAttributes({
-        [AttributeNames.ERROR]: true,
         [AttributeNames.HTTP_ERROR_NAME]: error.name,
         [AttributeNames.HTTP_ERROR_MESSAGE]: error.message,
       });

diff --git a/packages/opentelemetry-plugin-http/test/functionals/http-enable.test.ts b/packages/opentelemetry-plugin-http/test/functionals/http-enable.test.ts
@@ -26,7 +26,6 @@ import { DummyPropagation } from '../utils/DummyPropagation';
 import { httpRequest } from '../utils/httpRequest';
 import { TracerTest } from '../utils/TracerTest';
 import { SpanAuditProcessor } from '../utils/SpanAuditProcessor';
-import * as url from 'url';
 
 let server: http.Server;
 const serverPort = 12345;
@@ -279,127 +278,5 @@ describe('HttpPlugin', () => {
         assert.strictEqual(spans.length, 0);
       });
     }
-
-    it('should create a rootSpan for GET requests and add propagation headers', async () => {
-      nock.enableNetConnect();
-      const spans = audit.processSpans();
-      assert.strictEqual(spans.length, 0);
-      await httpRequest.get(`http://google.fr/?query=test`).then(result => {
-        const spans = audit.processSpans();
-        assert.strictEqual(spans.length, 1);
-        assert.ok(spans[0].name.indexOf('GET /') >= 0);
-
-        const span = spans[0];
-        const validations = {
-          hostname: 'google.fr',
-          httpStatusCode: result.statusCode!,
-          httpMethod: 'GET',
-          pathname: '/',
-          path: '/?query=test',
-          resHeaders: result.resHeaders,
-          reqHeaders: result.reqHeaders,
-        };
-        assertSpan(span, SpanKind.CLIENT, validations);
-      });
-      nock.disableNetConnect();
-    });
-
-    it('custom attributes should show up on client spans', async () => {
-      nock.enableNetConnect();
-
-      await httpRequest.get(`http://google.fr/`).then(result => {
-        const spans = audit.processSpans();
-        assert.strictEqual(spans.length, 1);
-        assert.ok(spans[0].name.indexOf('GET /') >= 0);
-
-        const span = spans[0];
-        const validations = {
-          hostname: 'google.fr',
-          httpStatusCode: result.statusCode!,
-          httpMethod: 'GET',
-          pathname: '/',
-          resHeaders: result.resHeaders,
-          reqHeaders: result.reqHeaders,
-        };
-        assert.strictEqual(span.attributes['span kind'], SpanKind.CLIENT);
-        assertSpan(span, SpanKind.CLIENT, validations);
-      });
-      nock.disableNetConnect();
-    });
-
-    it('should create a span for GET requests and add propagation headers with Expect headers', async () => {
-      nock.enableNetConnect();
-      const spans = audit.processSpans();
-      assert.strictEqual(spans.length, 0);
-      const options = Object.assign(
-        { headers: { Expect: '100-continue' } },
-        url.parse('http://google.fr/')
-      );
-      await httpRequest.get(options).then(result => {
-        const spans = audit.processSpans();
-        assert.strictEqual(spans.length, 1);
-        assert.ok(spans[0].name.indexOf('GET /') >= 0);
-
-        const span = spans[0];
-        const validations = {
-          hostname: 'google.fr',
-          httpStatusCode: 301,
-          httpMethod: 'GET',
-          pathname: '/',
-          resHeaders: result.resHeaders,
-          reqHeaders: result.reqHeaders,
-        };
-        assertSpan(span, SpanKind.CLIENT, validations);
-      });
-      nock.disableNetConnect();
-    });
-    for (const headers of [
-      { Expect: '100-continue', 'user-agent': 'http-plugin-test' },
-      { 'user-agent': 'http-plugin-test' },
-    ]) {
-      it(`should create a span for GET requests and add propagation when using the following signature: http.get(url, options, callback) and following headers: ${JSON.stringify(
-        headers
-      )}`, done => {
-        nock.enableNetConnect();
-        const spans = audit.processSpans();
-        assert.strictEqual(spans.length, 0);
-        const options = { headers };
-        http.get('http://google.fr/', options, (resp: http.IncomingMessage) => {
-          const res = (resp as unknown) as http.IncomingMessage & {
-            req: http.IncomingMessage;
-          };
-          let data = '';
-          resp.on('data', chunk => {
-            data += chunk;
-          });
-          resp.on('end', () => {
-            const spans = audit.processSpans();
-            assert.strictEqual(spans.length, 1);
-            assert.ok(spans[0].name.indexOf('GET /') >= 0);
-            const validations = {
-              hostname: 'google.fr',
-              httpStatusCode: 301,
-              httpMethod: 'GET',
-              pathname: '/',
-              resHeaders: resp.headers,
-              /* tslint:disable:no-any */
-              reqHeaders: (res.req as any).getHeaders
-                ? (res.req as any).getHeaders()
-                : (res.req as any)._headers,
-              /* tslint:enable:no-any */
-            };
-            assert.ok(data);
-            assert.ok(
-              validations.reqHeaders[DummyPropagation.TRACE_CONTEXT_KEY]
-            );
-            assert.ok(
-              validations.reqHeaders[DummyPropagation.SPAN_CONTEXT_KEY]
-            );
-            done();
-          });
-        });
-        nock.disableNetConnect();
-      });
-    }
   });
 });
diff --git a/packages/opentelemetry-plugin-http/test/integrations/http-enable.test.ts b/packages/opentelemetry-plugin-http/test/integrations/http-enable.test.ts
@@ -26,6 +26,7 @@ import { httpRequest } from '../utils/httpRequest';
 import { TracerTest } from '../utils/TracerTest';
 import { SpanAuditProcessor } from '../utils/SpanAuditProcessor';
 import * as url from 'url';
+import { Utils } from '../utils/Utils';
 
 const serverPort = 12345;
 const hostname = 'localhost';
@@ -37,6 +38,22 @@ export const customAttributeFunction = (span: Span): void => {
 
 describe('HttpPlugin Integration tests', () => {
   describe('enable()', () => {
+    before(function(done) {
+      // mandatory
+      if (process.env.CI) {
+        done();
+        return;
+      }
+
+      Utils.checkInternet(isConnected => {
+        if (!isConnected) {
+          this.skip();
+          // don't disturbe people
+        }
+        done();
+      });
+    });
+
     const scopeManager = new AsyncHooksScopeManager();
     const httpTextFormat = new DummyPropagation();
     const logger = new NoopLogger();
@@ -134,7 +151,13 @@ describe('HttpPlugin Integration tests', () => {
           resHeaders: result.resHeaders,
           reqHeaders: result.reqHeaders,
         };
-        assertSpan(span, SpanKind.CLIENT, validations);
+        try {
+          assertSpan(span, SpanKind.CLIENT, validations);
+        } catch (error) {
+          // temporary redirect is also correct
+          validations.httpStatusCode = 307;
+          assertSpan(span, SpanKind.CLIENT, validations);
+        }
       });
     });
     for (const headers of [

diff --git a/packages/opentelemetry-plugin-http/test/utils/DummyPropagation.ts b/packages/opentelemetry-plugin-http/test/utils/DummyPropagation.ts
@@ -14,7 +14,6 @@
  * limitations under the License.
  */
 import { SpanContext, HttpTextFormat } from '@opentelemetry/types';
-// import * as http from 'http';
 
 export class DummyPropagation implements HttpTextFormat {
   static TRACE_CONTEXT_KEY = 'x-dummy-trace-id';

diff --git a/packages/opentelemetry-plugin-http/test/utils/Utils.ts b/packages/opentelemetry-plugin-http/test/utils/Utils.ts
@@ -0,0 +1,29 @@
+/**
+ * Copyright 2019, OpenTelemetry Authors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import * as dns from 'dns';
+
+export class Utils {
+  static checkInternet(cb: (isConnected: boolean) => void) {
+    dns.lookup('google.com', err => {
+      if (err && err.code === 'ENOTFOUND') {
+        cb(false);
+      } else {
+        cb(true);
+      }
+    });
+  }
+}
diff --git a/packages/opentelemetry-plugin-http/test/utils/assertSpan.ts b/packages/opentelemetry-plugin-http/test/utils/assertSpan.ts
@@ -17,7 +17,7 @@
 import { SpanKind } from '@opentelemetry/types';
 import * as assert from 'assert';
 import * as http from 'http';
-import { AttributeNames } from '../../src/enums/attributeNames';
+import { AttributeNames } from '../../src/enums/AttributeNames';
 import { HttpPlugin } from '../../src/http';
 import { Utils } from '../../src/utils';
 import { DummyPropagation } from './DummyPropagation';