chore(http): remove x-opentelemetry-outgoing-request header #1547

Author: vmarchaud

packages/opentelemetry-exporter-collector/src/platform/node/CollectorExporterNodeBase.ts

@@ -31,9 +31,7 @@ export abstract class CollectorExporterNodeBase<
   ExportItem,
   ServiceRequest
 > {
-  DEFAULT_HEADERS: Record<string, string> = {
-    [collectorTypes.OT_REQUEST_HEADER]: '1',
-  };
+  DEFAULT_HEADERS: Record<string, string> = {};
   headers: Record<string, string>;
   constructor(config: CollectorExporterConfigBase = {}) {
     super(config);

packages/opentelemetry-exporter-collector/src/types.ts

@@ -17,9 +17,6 @@
 import { SpanKind, Logger, Attributes } from '@opentelemetry/api';
 import * as api from '@opentelemetry/api';
 
-// header to prevent instrumentation on request
-export const OT_REQUEST_HEADER = 'x-opentelemetry-outgoing-request';
-
 /* eslint-disable @typescript-eslint/no-namespace */
 export namespace opentelemetryProto {
   export namespace collector {

packages/opentelemetry-exporter-jaeger/src/jaeger.ts

@@ -20,7 +20,6 @@ import { ReadableSpan, SpanExporter } from '@opentelemetry/tracing';
 import { Socket } from 'dgram';
 import { spanToThrift } from './transform';
 import * as jaegerTypes from './types';
-import { OT_REQUEST_HEADER } from './utils';
 
 /**
  * Format and sends span information to Jaeger Exporter.
@@ -54,7 +53,6 @@ export class JaegerExporter implements SpanExporter {
     localConfig.host = localConfig.host || process.env.JAEGER_AGENT_HOST;
     if (localConfig.endpoint) {
       this._sender = new jaegerTypes.HTTPSender(localConfig);
-      this._sender._httpOptions.headers[OT_REQUEST_HEADER] = 1;
     } else {
       this._sender = localConfig.endpoint = new jaegerTypes.UDPSender(
         localConfig

packages/opentelemetry-exporter-jaeger/src/utils.ts

@@ -22,7 +22,6 @@ import { ThriftProcess } from '../src/types';
 import { ReadableSpan } from '@opentelemetry/tracing';
 import { TraceFlags } from '@opentelemetry/api';
 import { Resource } from '@opentelemetry/resources';
-import { OT_REQUEST_HEADER } from '../src/utils';
 import * as nock from 'nock';
 
 describe('JaegerExporter', () => {
@@ -156,12 +155,11 @@ describe('JaegerExporter', () => {
       });
     });
 
-    it('should use httpSender if config.endpoint is setten and set x-opentelemetry-outgoing-request header', done => {
+    it('should use httpSender if config.endpoint is setten', done => {
       const mockedEndpoint = 'http://testendpoint';
       nock(mockedEndpoint)
         .post('/')
         .reply(function () {
-          assert.strictEqual(this.req.headers[OT_REQUEST_HEADER], 1);
           assert.strictEqual(
             this.req.headers['content-type'],
             'application/x-thrift'
@@ -174,10 +172,6 @@ describe('JaegerExporter', () => {
         endpoint: mockedEndpoint,
       });
       assert.strictEqual(exporter['_sender'].constructor.name, 'HTTPSender');
-      assert.strictEqual(
-        exporter['_sender']._httpOptions.headers[OT_REQUEST_HEADER],
-        1
-      );
       const spanContext = {
         traceId: 'd4cda95b652f4a1592b449d5929fda1b',
         spanId: '6e0c63257de34c92',

packages/opentelemetry-exporter-jaeger/test/jaeger.test.ts

@@ -17,7 +17,6 @@
 import * as api from '@opentelemetry/api';
 import { ExportResult } from '@opentelemetry/core';
 import * as zipkinTypes from '../../types';
-import { OT_REQUEST_HEADER } from '../../utils';
 
 /**
  * Prepares send function that will send spans to the remote Zipkin service.
@@ -33,7 +32,6 @@ export function prepareSend(
     xhrHeaders = {
       Accept: 'application/json',
       'Content-Type': 'application/json',
-      [OT_REQUEST_HEADER]: '1',
       ...headers,
     };
   }

packages/opentelemetry-exporter-zipkin/src/platform/browser/util.ts

@@ -20,7 +20,6 @@ import * as http from 'http';
 import * as https from 'https';
 import * as url from 'url';
 import * as zipkinTypes from '../../types';
-import { OT_REQUEST_HEADER } from '../../utils';
 
 /**
  * Prepares send function that will send spans to the remote Zipkin service.
@@ -37,7 +36,6 @@ export function prepareSend(
       method: 'POST',
       headers: {
         'Content-Type': 'application/json',
-        [OT_REQUEST_HEADER]: 1,
         ...headers,
       },
     },

packages/opentelemetry-exporter-zipkin/src/platform/node/util.ts

@@ -26,7 +26,6 @@ import * as api from '@opentelemetry/api';
 import { Resource } from '@opentelemetry/resources';
 import { ZipkinExporter } from '../../src';
 import * as zipkinTypes from '../../src/types';
-import { OT_REQUEST_HEADER } from '../../src/utils';
 import { TraceFlags } from '@opentelemetry/api';
 import { SERVICE_RESOURCE } from '@opentelemetry/resources';
 
@@ -254,26 +253,6 @@ describe('Zipkin Exporter - node', () => {
       });
     });
 
-    it(`should send '${OT_REQUEST_HEADER}' header`, () => {
-      const scope = nock('https://localhost:9411')
-        .post('/api/v2/spans')
-        .reply(function (uri, requestBody, cb) {
-          assert.ok(this.req.headers[OT_REQUEST_HEADER]);
-          cb(null, [200, 'Ok']);
-        });
-
-      const exporter = new ZipkinExporter({
-        serviceName: 'my-service',
-        logger: new NoopLogger(),
-        url: 'https://localhost:9411/api/v2/spans',
-      });
-
-      exporter.export([getReadableSpan()], (result: ExportResult) => {
-        scope.done();
-        assert.strictEqual(result, ExportResult.SUCCESS);
-      });
-    });
-
     it('should return FailedNonRetryable with 4xx', () => {
       const scope = nock('http://localhost:9411')
         .post('/api/v2/spans')

packages/opentelemetry-exporter-zipkin/src/utils.ts

@@ -769,22 +769,6 @@ export const runTests = (
           done();
         });
       });
-
-      methodList.map(method => {
-        const metadata = new grpc.Metadata();
-        metadata.set('x-opentelemetry-outgoing-request', '1');
-        describe(`Test should not create spans for grpc remote method ${method.description} when metadata has otel header`, () => {
-          before(() => {
-            method.metadata = metadata;
-          });
-
-          after(() => {
-            delete method.metadata;
-          });
-
-          runTest(method, provider, false);
-        });
-      });
     });
 
     describe('Test filtering requests using options', () => {

packages/opentelemetry-exporter-zipkin/test/node/zipkin.test.ts

@@ -29,7 +29,6 @@ import {
   grpcStatusCodeToSpanStatus,
   grpcStatusCodeToCanonicalCode,
   CALL_SPAN_ENDED,
-  containsOtelMetadata,
   methodIsIgnored,
 } from '../utils';
 import { EventEmitter } from 'events';
@@ -77,9 +76,6 @@ export function getPatchedClientMethods(
       const name = `grpc.${original.path.replace('/', '')}`;
       const args = [...arguments];
       const metadata = getMetadata.call(plugin, original, args);
-      if (containsOtelMetadata(metadata)) {
-        return original.apply(this, args);
-      }
       const span = plugin.tracer.startSpan(name, {
         kind: SpanKind.CLIENT,
       });

packages/opentelemetry-grpc-utils/test/grpcUtils.test.ts

@@ -33,7 +33,7 @@ import {
 import { RpcAttribute } from '@opentelemetry/semantic-conventions';
 import { clientStreamAndUnaryHandler } from './clientStreamAndUnary';
 import { serverStreamAndBidiHandler } from './serverStreamAndBidi';
-import { containsOtelMetadata, methodIsIgnored } from '../utils';
+import { methodIsIgnored } from '../utils';
 
 type ServerRegisterFunction = typeof grpcJs.Server.prototype.register;
 
@@ -141,12 +141,9 @@ function shouldNotTraceServerCall(
   ignoreGrpcMethods?: IgnoreMatcher[]
 ): boolean {
   const parsedName = methodName.split('/');
-  return (
-    containsOtelMetadata(metadata) ||
-    methodIsIgnored(
-      parsedName[parsedName.length - 1] || methodName,
-      ignoreGrpcMethods
-    )
+  return methodIsIgnored(
+    parsedName[parsedName.length - 1] || methodName,
+    ignoreGrpcMethods
   );
 }
 

packages/opentelemetry-plugin-grpc-js/src/client/utils.ts

@@ -25,11 +25,6 @@ import { IgnoreMatcher } from './types';
  */
 export const CALL_SPAN_ENDED = Symbol('opentelemetry call span ended');
 
-/**
- * Metadata key used to denote an outgoing opentelemetry request.
- */
-const OTEL_OUTGOING_REQUEST_KEY = 'x-opentelemetry-outgoing-request';
-
 /**
  * Convert a grpc status code to an opentelemetry Canonical code. For now, the enums are exactly the same
  * @param status
@@ -51,14 +46,6 @@ export const grpcStatusCodeToSpanStatus = (status: number): Status => {
   return { code: status };
 };
 
-/**
- * Returns true if the metadata contains
- * the opentelemetry outgoing request header.
- */
-export const containsOtelMetadata = (metadata: grpcTypes.Metadata): boolean => {
-  return metadata.get(OTEL_OUTGOING_REQUEST_KEY).length > 0;
-};
-
 /**
  * Returns true if methodName matches pattern
  * @param methodName the name of the method

packages/opentelemetry-plugin-grpc-js/src/server/patchServer.ts

@@ -43,7 +43,6 @@ import {
   _grpcStatusCodeToCanonicalCode,
   _grpcStatusCodeToSpanStatus,
   _methodIsIgnored,
-  _containsOtelMetadata,
 } from './utils';
 import { VERSION } from './version';
 
@@ -237,12 +236,9 @@ export class GrpcPlugin extends BasePlugin<grpc> {
     name: string
   ): boolean {
     const parsedName = name.split('/');
-    return (
-      _containsOtelMetadata(call.metadata) ||
-      _methodIsIgnored(
-        parsedName[parsedName.length - 1] || name,
-        this._config.ignoreGrpcMethods
-      )
+    return _methodIsIgnored(
+      parsedName[parsedName.length - 1] || name,
+      this._config.ignoreGrpcMethods
     );
   }
 
@@ -391,9 +387,6 @@ export class GrpcPlugin extends BasePlugin<grpc> {
         const name = `grpc.${original.path.replace('/', '')}`;
         const args = Array.prototype.slice.call(arguments);
         const metadata = plugin._getMetadata(original, args);
-        if (_containsOtelMetadata(metadata)) {
-          return original.apply(this, args);
-        }
         const span = plugin._tracer.startSpan(name, {
           kind: SpanKind.CLIENT,
         });

packages/opentelemetry-plugin-grpc-js/src/utils.ts

@@ -18,11 +18,6 @@ import { CanonicalCode, Status } from '@opentelemetry/api';
 import * as grpcTypes from 'grpc'; // For types only
 import { IgnoreMatcher } from './types';
 
-/**
- * Metadata key used to denote an outgoing opentelemetry request.
- */
-const _otRequestHeader = 'x-opentelemetry-outgoing-request';
-
 // Equivalent to lodash _.findIndex
 export const findIndex: <T>(args: T[], fn: (arg: T) => boolean) => number = (
   args,
@@ -55,16 +50,6 @@ export const _grpcStatusCodeToSpanStatus = (status: number): Status => {
   return { code: status };
 };
 
-/**
- * Returns true if the metadata contains
- * the opentelemetry outgoing request header.
- */
-export const _containsOtelMetadata = (
-  metadata: grpcTypes.Metadata
-): boolean => {
-  return metadata.get(_otRequestHeader).length > 0;
-};
-
 /**
  * Returns true if methodName matches pattern
  * @param methodName the name of the method

packages/opentelemetry-plugin-grpc/src/grpc.ts

@@ -398,15 +398,7 @@ export class HttpPlugin extends BasePlugin<Http> {
         extraOptions
       );
 
-      if (utils.isOpenTelemetryRequest(optionsParsed)) {
-        // clone the headers so delete will not modify the user's object
-        optionsParsed.headers = Object.assign({}, optionsParsed.headers);
-        delete optionsParsed.headers[utils.OT_REQUEST_HEADER];
-        return original.apply(this, [optionsParsed, ...args]);
-      }
-
       if (
-        utils.isOpenTelemetryRequest(optionsParsed) ||
         utils.isIgnored(
           origin + pathname,
           plugin._config.ignoreOutgoingUrls,

packages/opentelemetry-plugin-grpc/src/utils.ts

@@ -35,12 +35,6 @@ import {
   SpecialHttpStatusCodeMapping,
 } from './types';
 
-/**
- * Specific header used by exporters to "mark" outgoing request to avoid creating
- * spans for request that export them which would create a infinite loop.
- */
-export const OT_REQUEST_HEADER = 'x-opentelemetry-outgoing-request';
-
 export const HTTP_STATUS_SPECIAL_CASES: SpecialHttpStatusCodeMapping = {
   401: CanonicalCode.UNAUTHENTICATED,
   403: CanonicalCode.PERMISSION_DENIED,
@@ -301,24 +295,6 @@ export const isValidOptionsType = (options: unknown): boolean => {
   return type === 'string' || (type === 'object' && !Array.isArray(options));
 };
 
-/**
- * Check whether the given request should be ignored
- * Use case: Typically, exporter `SpanExporter` can use http module to send spans.
- * This will also generate spans (from the http-plugin) that will be sended through the exporter
- * and here we have loop.
- *
- * TODO: Refactor this logic when a solution is found in
- * https://github.com/open-telemetry/opentelemetry-specification/issues/530
- *
- *
- * @param {RequestOptions} options
- */
-export const isOpenTelemetryRequest = (
-  options: RequestOptions
-): options is { headers: {} } & RequestOptions => {
-  return !!(options && options.headers && options.headers[OT_REQUEST_HEADER]);
-};
-
 /**
  * Returns outgoing request attributes scoped to the options passed to the request
  * @param {ParsedRequestOptions} requestOptions the same options used to make the request