[#57] Removes http header binary annotation in spans.

openzipkin · Jun 28, 2018 · 93620ad · 93620ad
1 parent 181ec15
commit 93620ad
Show file tree

Hide file tree

Showing 6 changed files with 34 additions and 56 deletions.
diff --git a/.gitignore b/.gitignore
@@ -2,3 +2,4 @@ node_modules
 lib
 npm-debug.log
 lerna-debug.log
+package-lock.json
diff --git a/packages/zipkin-instrumentation-express/test/wrapExpressHttpProxyIntegrationTest.js b/packages/zipkin-instrumentation-express/test/wrapExpressHttpProxyIntegrationTest.js
@@ -219,37 +219,33 @@ describe('express http proxy instrumentation - integration test', () => {
 
               expect(annotations[4].annotation.annotationType).to.equal('LocalAddr');
 
-              expect(annotations[5].annotation.annotationType).to.equal('BinaryAnnotation');
-              expect(annotations[5].annotation.key).to.equal('X-B3-Flags');
-              expect(annotations[5].annotation.value).to.equal('1');
-
-              expect(annotations[6].annotation.annotationType).to.equal('ServiceName');
-              expect(annotations[6].annotation.serviceName).to.equal('weather-app');
+              expect(annotations[5].annotation.annotationType).to.equal('ServiceName');
+              expect(annotations[5].annotation.serviceName).to.equal('weather-app');
 
-              expect(annotations[7].annotation.annotationType).to.equal('Rpc');
-              expect(annotations[7].annotation.name).to.equal('POST');
+              expect(annotations[6].annotation.annotationType).to.equal('Rpc');
+              expect(annotations[6].annotation.name).to.equal('POST');
 
-              expect(annotations[8].annotation.annotationType).to.equal('BinaryAnnotation');
-              expect(annotations[8].annotation.key).to.equal('http.url');
+              expect(annotations[7].annotation.annotationType).to.equal('BinaryAnnotation');
+              expect(annotations[7].annotation.key).to.equal('http.url');
               // express-http-proxy does not include protocol when intercepting request
               const apiUrlWithoutProtocol = `//127.0.0.1:${apiPort}/weather?index=10&count=300`;
-              expect(annotations[8].annotation.value).to.equal(apiUrlWithoutProtocol);
+              expect(annotations[7].annotation.value).to.equal(apiUrlWithoutProtocol);
 
-              expect(annotations[9].annotation.annotationType).to.equal('ClientSend');
+              expect(annotations[8].annotation.annotationType).to.equal('ClientSend');
 
-              expect(annotations[10].annotation.annotationType).to.equal('ServerAddr');
+              expect(annotations[9].annotation.annotationType).to.equal('ServerAddr');
 
-              expect(annotations[11].annotation.annotationType).to.equal('BinaryAnnotation');
-              expect(annotations[11].annotation.key).to.equal('http.status_code');
-              expect(annotations[11].annotation.value).to.equal('202');
+              expect(annotations[10].annotation.annotationType).to.equal('BinaryAnnotation');
+              expect(annotations[10].annotation.key).to.equal('http.status_code');
+              expect(annotations[10].annotation.value).to.equal('202');
 
-              expect(annotations[12].annotation.annotationType).to.equal('ClientRecv');
+              expect(annotations[11].annotation.annotationType).to.equal('ClientRecv');
 
-              expect(annotations[13].annotation.annotationType).to.equal('BinaryAnnotation');
-              expect(annotations[13].annotation.key).to.equal('http.status_code');
-              expect(annotations[13].annotation.value).to.equal('203');
+              expect(annotations[12].annotation.annotationType).to.equal('BinaryAnnotation');
+              expect(annotations[12].annotation.key).to.equal('http.status_code');
+              expect(annotations[12].annotation.value).to.equal('203');
 
-              expect(annotations[14].annotation.annotationType).to.equal('ServerSend');
+              expect(annotations[13].annotation.annotationType).to.equal('ServerSend');
 
               done();
             })

diff --git a/packages/zipkin-instrumentation-hapi/test/integrationTest.js b/packages/zipkin-instrumentation-hapi/test/integrationTest.js
@@ -55,14 +55,10 @@ describe('hapi middleware - integration test', () => {
         expect(annotations[4].annotation.annotationType).to.equal('LocalAddr');
 
         expect(annotations[5].annotation.annotationType).to.equal('BinaryAnnotation');
-        expect(annotations[5].annotation.key).to.equal('X-B3-Flags');
-        expect(annotations[5].annotation.value).to.equal('1');
-
-        expect(annotations[6].annotation.annotationType).to.equal('BinaryAnnotation');
-        expect(annotations[6].annotation.key).to.equal('http.status_code');
-        expect(annotations[6].annotation.value).to.equal('202');
+        expect(annotations[5].annotation.key).to.equal('http.status_code');
+        expect(annotations[5].annotation.value).to.equal('202');
 
-        expect(annotations[7].annotation.annotationType).to.equal('ServerSend');
+        expect(annotations[6].annotation.annotationType).to.equal('ServerSend');
 
         done();
       });

diff --git a/packages/zipkin-instrumentation-restify/test/integrationTest.js b/packages/zipkin-instrumentation-restify/test/integrationTest.js
@@ -61,12 +61,8 @@ describe('restify middleware - integration test', () => {
           expect(annotations[4].annotation.annotationType).to.equal('LocalAddr');
 
           expect(annotations[5].annotation.annotationType).to.equal('BinaryAnnotation');
-          expect(annotations[5].annotation.key).to.equal('X-B3-Flags');
-          expect(annotations[5].annotation.value).to.equal('1');
-
-          expect(annotations[6].annotation.annotationType).to.equal('BinaryAnnotation');
-          expect(annotations[6].annotation.key).to.equal('message');
-          expect(annotations[6].annotation.value).to.equal('hello from within app');
+          expect(annotations[5].annotation.key).to.equal('message');
+          expect(annotations[5].annotation.value).to.equal('hello from within app');
 
           expect(annotations[7].annotation.annotationType).to.equal('BinaryAnnotation');
           expect(annotations[7].annotation.key).to.equal('http.status_code');
@@ -189,22 +185,18 @@ describe('restify middleware - integration test', () => {
           expect(annotations[4].annotation.annotationType).to.equal('LocalAddr');
 
           expect(annotations[5].annotation.annotationType).to.equal('BinaryAnnotation');
-          expect(annotations[5].annotation.key).to.equal('X-B3-Flags');
-          expect(annotations[5].annotation.value).to.equal('1');
+          expect(annotations[5].annotation.key).to.equal('message');
+          expect(annotations[5].annotation.value).to.equal('testing error annotation recording');
 
           expect(annotations[6].annotation.annotationType).to.equal('BinaryAnnotation');
-          expect(annotations[6].annotation.key).to.equal('message');
-          expect(annotations[6].annotation.value).to.equal('testing error annotation recording');
+          expect(annotations[6].annotation.key).to.equal('http.status_code');
+          expect(annotations[6].annotation.value).to.equal('404');
 
           expect(annotations[7].annotation.annotationType).to.equal('BinaryAnnotation');
-          expect(annotations[7].annotation.key).to.equal('http.status_code');
+          expect(annotations[7].annotation.key).to.equal('error');
           expect(annotations[7].annotation.value).to.equal('404');
 
-          expect(annotations[8].annotation.annotationType).to.equal('BinaryAnnotation');
-          expect(annotations[8].annotation.key).to.equal('error');
-          expect(annotations[8].annotation.value).to.equal('404');
-
-          expect(annotations[9].annotation.annotationType).to.equal('ServerSend');
+          expect(annotations[8].annotation.annotationType).to.equal('ServerSend');
 
           done();
         })

diff --git a/packages/zipkin/src/instrumentation/httpServer.js b/packages/zipkin/src/instrumentation/httpServer.js
@@ -77,9 +77,6 @@ class HttpServerInstrumentation {
     this.tracer.recordAnnotation(new Annotation.ServerRecv());
     this.tracer.recordAnnotation(new Annotation.LocalAddr({port: this.port}));
 
-    if (id.flags !== 0 && id.flags != null) {
-      this.tracer.recordBinary(Header.Flags, id.flags.toString());
-    }
     return id;
   }
 

diff --git a/packages/zipkin/test/httpServerInstrumentation.test.js b/packages/zipkin/test/httpServerInstrumentation.test.js
@@ -93,18 +93,14 @@ describe('Http Server Instrumentation', () => {
     expect(annotations[4].annotation.annotationType).to.equal('LocalAddr');
 
     expect(annotations[5].annotation.annotationType).to.equal('BinaryAnnotation');
-    expect(annotations[5].annotation.key).to.equal('X-B3-Flags');
-    expect(annotations[5].annotation.value).to.equal('1');
+    expect(annotations[5].annotation.key).to.equal('message');
+    expect(annotations[5].annotation.value).to.equal('hello from within app');
 
     expect(annotations[6].annotation.annotationType).to.equal('BinaryAnnotation');
-    expect(annotations[6].annotation.key).to.equal('message');
-    expect(annotations[6].annotation.value).to.equal('hello from within app');
-
-    expect(annotations[7].annotation.annotationType).to.equal('BinaryAnnotation');
-    expect(annotations[7].annotation.key).to.equal('http.status_code');
-    expect(annotations[7].annotation.value).to.equal('202');
+    expect(annotations[6].annotation.key).to.equal('http.status_code');
+    expect(annotations[6].annotation.value).to.equal('202');
 
-    expect(annotations[8].annotation.annotationType).to.equal('ServerSend');
+    expect(annotations[7].annotation.annotationType).to.equal('ServerSend');
   });
 
   it('should properly report the URL with a query string', () => {