fix: review code

jufab · Sep 14, 2020 · 8b1b7b5 · 8b1b7b5
1 parent 06cb6de
commit 8b1b7b5
Show file tree

Hide file tree

Showing 3 changed files with 70 additions and 82 deletions.
diff --git a/projects/opentelemetry-interceptor/__mocks__/data/config.mock.ts b/projects/opentelemetry-interceptor/__mocks__/data/config.mock.ts
@@ -15,6 +15,19 @@ export const otelcolExporterConfig: OpenTelemetryConfig = {
   },
 };
 
+/**
+ * @ignore
+ */
+export const otelcolExporterProductionConfig: OpenTelemetryConfig = {
+  commonConfig: {
+    serviceName: 'test',
+    production : true,
+  },
+  otelcolConfig: {
+    url: 'http://localhost',
+  },
+};
+
 /**
  * @ignore
  */

diff --git a/...ects/opentelemetry-interceptor/src/lib/interceptor/opentelemetry-http.interceptor.spec.ts b/...ects/opentelemetry-interceptor/src/lib/interceptor/opentelemetry-http.interceptor.spec.ts
@@ -9,12 +9,13 @@ import {
   HttpHeaders,
 } from '@angular/common/http';
 import { OpenTelemetryHttpInterceptor } from './opentelemetry-http.interceptor';
-import { OpenTelemetryInjectConfig } from '../configuration/opentelemetry-config';
+import { OpenTelemetryConfig, OpenTelemetryInjectConfig } from '../configuration/opentelemetry-config';
 import {
   otelcolExporterConfig,
   otelcolExporterWithProbabilitySamplerAndCompositeConfig,
   otelcolExporterWithProbabilitySamplerAtZeroAndCompositeConfig,
-  otelcolExporterWithProbabilitySamplerAtTwoConfig
+  otelcolExporterWithProbabilitySamplerAtTwoConfig,
+  otelcolExporterProductionConfig,
 } from '../../../__mocks__/data/config.mock';
 import { of } from 'rxjs';
 import { ConsoleSpanExporterModule } from '../services/exporter/console/console-span-exporter.module';
@@ -24,19 +25,7 @@ describe('OpenTelemetryHttpInterceptor', () => {
   let httpClient: HttpClient;
   let httpControllerMock: HttpTestingController;
   beforeEach(() => {
-    TestBed.configureTestingModule({
-      imports: [HttpClientTestingModule, ConsoleSpanExporterModule, HttpTraceContextPropagatorModule],
-      providers: [
-        { provide: OpenTelemetryInjectConfig, useValue: otelcolExporterConfig },
-        {
-          provide: HTTP_INTERCEPTORS,
-          useClass: OpenTelemetryHttpInterceptor,
-          multi: true,
-        },
-      ],
-    });
-    httpClient = TestBed.inject(HttpClient);
-    httpControllerMock = TestBed.inject(HttpTestingController);
+    ({ httpClient, httpControllerMock } = defineModuleTest(httpClient, httpControllerMock, otelcolExporterConfig));
   });
   it('should be created', () => {
     const interceptor = TestBed.inject(OpenTelemetryHttpInterceptor);
@@ -53,6 +42,18 @@ describe('OpenTelemetryHttpInterceptor', () => {
     httpControllerMock.verify();
   });
 
+  it('verify with production mode', () => {
+    ({ httpClient, httpControllerMock } = defineModuleTest(httpClient, httpControllerMock, otelcolExporterProductionConfig));
+
+    const url = 'http://url.test.com';
+    httpClient.get(url).subscribe();
+    const req = httpControllerMock.expectOne(url);
+    expect(req.request.headers).not.toBeNull();
+    expect(req.request.headers.get('traceparent')).not.toBeNull();
+    req.flush({});
+    httpControllerMock.verify();
+  });
+
   it('Add traceparent header on a given request with already presents headers', () => {
     const url = 'http://url.test.com';
     const headers: HttpHeaders = new HttpHeaders({
@@ -100,23 +101,7 @@ describe('OpenTelemetryHttpInterceptor', () => {
   });
 
   it('verify probability sampler to be add', () => {
-    TestBed.resetTestingModule();
-    TestBed.configureTestingModule({
-      imports: [HttpClientTestingModule, ConsoleSpanExporterModule, HttpTraceContextPropagatorModule],
-      providers: [
-        {
-          provide: OpenTelemetryInjectConfig,
-          useValue: otelcolExporterWithProbabilitySamplerAndCompositeConfig,
-        },
-        {
-          provide: HTTP_INTERCEPTORS,
-          useClass: OpenTelemetryHttpInterceptor,
-          multi: true,
-        },
-      ],
-    });
-    httpClient = TestBed.inject(HttpClient);
-    httpControllerMock = TestBed.inject(HttpTestingController);
+    ({ httpClient, httpControllerMock } = defineModuleTest(httpClient, httpControllerMock, otelcolExporterWithProbabilitySamplerAndCompositeConfig));
 
     const url = 'http://url.test.com';
     httpClient.get(url).subscribe();
@@ -127,23 +112,7 @@ describe('OpenTelemetryHttpInterceptor', () => {
   });
 
   it('verify probability sampler to be add at zero', () => {
-    TestBed.resetTestingModule();
-    TestBed.configureTestingModule({
-      imports: [HttpClientTestingModule, ConsoleSpanExporterModule, HttpTraceContextPropagatorModule],
-      providers: [
-        {
-          provide: OpenTelemetryInjectConfig,
-          useValue: otelcolExporterWithProbabilitySamplerAtZeroAndCompositeConfig,
-        },
-        {
-          provide: HTTP_INTERCEPTORS,
-          useClass: OpenTelemetryHttpInterceptor,
-          multi: true,
-        },
-      ],
-    });
-    httpClient = TestBed.inject(HttpClient);
-    httpControllerMock = TestBed.inject(HttpTestingController);
+    ({ httpClient, httpControllerMock } = defineModuleTest(httpClient, httpControllerMock, otelcolExporterWithProbabilitySamplerAtZeroAndCompositeConfig));
 
     const url = 'http://url.test.com';
     httpClient.get(url).subscribe();
@@ -153,23 +122,7 @@ describe('OpenTelemetryHttpInterceptor', () => {
     httpControllerMock.verify();
   });
   it('verify probability sampler to be add at one', () => {
-    TestBed.resetTestingModule();
-    TestBed.configureTestingModule({
-      imports: [HttpClientTestingModule, ConsoleSpanExporterModule, HttpTraceContextPropagatorModule],
-      providers: [
-        {
-          provide: OpenTelemetryInjectConfig,
-          useValue: otelcolExporterWithProbabilitySamplerAtTwoConfig,
-        },
-        {
-          provide: HTTP_INTERCEPTORS,
-          useClass: OpenTelemetryHttpInterceptor,
-          multi: true,
-        },
-      ],
-    });
-    httpClient = TestBed.inject(HttpClient);
-    httpControllerMock = TestBed.inject(HttpTestingController);
+    ({ httpClient, httpControllerMock } = defineModuleTest(httpClient, httpControllerMock, otelcolExporterWithProbabilitySamplerAtTwoConfig));
 
     const url = 'http://url.test.com';
     httpClient.get(url).subscribe();
@@ -179,3 +132,26 @@ describe('OpenTelemetryHttpInterceptor', () => {
     httpControllerMock.verify();
   });
 });
+
+
+function defineModuleTest(httpClient: HttpClient, httpControllerMock: HttpTestingController, otelcolConfig: OpenTelemetryConfig) {
+  TestBed.resetTestingModule();
+  TestBed.configureTestingModule({
+    imports: [HttpClientTestingModule, ConsoleSpanExporterModule, HttpTraceContextPropagatorModule],
+    providers: [
+      {
+        provide: OpenTelemetryInjectConfig,
+        useValue: otelcolConfig,
+      },
+      {
+        provide: HTTP_INTERCEPTORS,
+        useClass: OpenTelemetryHttpInterceptor,
+        multi: true,
+      },
+    ],
+  });
+  httpClient = TestBed.inject(HttpClient);
+  httpControllerMock = TestBed.inject(HttpTestingController);
+  return { httpClient, httpControllerMock };
+}
+
diff --git a/projects/opentelemetry-interceptor/src/lib/interceptor/opentelemetry-http.interceptor.ts b/projects/opentelemetry-interceptor/src/lib/interceptor/opentelemetry-http.interceptor.ts
@@ -102,22 +102,21 @@ export class OpenTelemetryHttpInterceptor implements HttpInterceptor {
           }
         },
         (event: HttpErrorResponse) => {
-          if (event instanceof HttpErrorResponse) {
-            span.setAttributes(
-              {
-                'http.status_text': event.statusText,
-                'http.status_code': event.status,
-              }
-            );
-            span.recordException({
-              name: event.name,
-              message: event.message,
-              stack: event.error
-            });
-            span.setStatus({
-              code: CanonicalCode.INTERNAL
-            });
-          }
+          span.setAttributes(
+            {
+              'http.status_text': event.statusText,
+              'http.status_code': event.status,
+            }
+          );
+          span.recordException({
+            name: event.name,
+            message: event.message,
+            stack: event.error
+          });
+          // TODO : To change after new spec...
+          span.setStatus({
+            code: CanonicalCode.INTERNAL
+          });
         }
       ),
       finalize(() => {