From 73ad589fe582b4ac7395dc13291f28e7e1879ce2 Mon Sep 17 00:00:00 2001 From: vmarchaud Date: Sat, 19 Sep 2020 18:14:36 +0200 Subject: [PATCH] chore(http): remove `x-opentelemetry-outgoing-request` header #1547 --- .../node/CollectorExporterNodeBase.ts | 4 +- .../src/types.ts | 3 -- .../src/jaeger.ts | 2 - .../src/utils.ts | 17 -------- .../test/jaeger.test.ts | 8 +--- .../src/platform/browser/util.ts | 2 - .../src/platform/node/util.ts | 2 - .../src/utils.ts | 17 -------- .../test/node/zipkin.test.ts | 21 ---------- .../opentelemetry-plugin-http/src/http.ts | 8 ---- .../opentelemetry-plugin-http/src/utils.ts | 24 ----------- .../test/functionals/http-enable.test.ts | 42 ------------------- .../test/functionals/utils.test.ts | 27 ------------ .../test/functionals/https-enable.test.ts | 17 -------- 14 files changed, 2 insertions(+), 192 deletions(-) delete mode 100644 packages/opentelemetry-exporter-jaeger/src/utils.ts delete mode 100644 packages/opentelemetry-exporter-zipkin/src/utils.ts diff --git a/packages/opentelemetry-exporter-collector/src/platform/node/CollectorExporterNodeBase.ts b/packages/opentelemetry-exporter-collector/src/platform/node/CollectorExporterNodeBase.ts index a8835915658..bcca2508785 100644 --- a/packages/opentelemetry-exporter-collector/src/platform/node/CollectorExporterNodeBase.ts +++ b/packages/opentelemetry-exporter-collector/src/platform/node/CollectorExporterNodeBase.ts @@ -31,9 +31,7 @@ export abstract class CollectorExporterNodeBase< ExportItem, ServiceRequest > { - DEFAULT_HEADERS: Record = { - [collectorTypes.OT_REQUEST_HEADER]: '1', - }; + DEFAULT_HEADERS: Record = {}; headers: Record; constructor(config: CollectorExporterConfigBase = {}) { super(config); diff --git a/packages/opentelemetry-exporter-collector/src/types.ts b/packages/opentelemetry-exporter-collector/src/types.ts index 23ff4da1a55..eab6b6b1938 100644 --- a/packages/opentelemetry-exporter-collector/src/types.ts +++ b/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 { diff --git a/packages/opentelemetry-exporter-jaeger/src/jaeger.ts b/packages/opentelemetry-exporter-jaeger/src/jaeger.ts index 1b5cd3cefa1..def95ddcf77 100644 --- a/packages/opentelemetry-exporter-jaeger/src/jaeger.ts +++ b/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 diff --git a/packages/opentelemetry-exporter-jaeger/src/utils.ts b/packages/opentelemetry-exporter-jaeger/src/utils.ts deleted file mode 100644 index ff29a5a5253..00000000000 --- a/packages/opentelemetry-exporter-jaeger/src/utils.ts +++ /dev/null @@ -1,17 +0,0 @@ -/* - * Copyright The 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 const OT_REQUEST_HEADER = 'x-opentelemetry-outgoing-request'; diff --git a/packages/opentelemetry-exporter-jaeger/test/jaeger.test.ts b/packages/opentelemetry-exporter-jaeger/test/jaeger.test.ts index 7e8baa4e884..1018b781265 100644 --- a/packages/opentelemetry-exporter-jaeger/test/jaeger.test.ts +++ b/packages/opentelemetry-exporter-jaeger/test/jaeger.test.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', diff --git a/packages/opentelemetry-exporter-zipkin/src/platform/browser/util.ts b/packages/opentelemetry-exporter-zipkin/src/platform/browser/util.ts index a954fb2d421..d2782dad012 100644 --- a/packages/opentelemetry-exporter-zipkin/src/platform/browser/util.ts +++ b/packages/opentelemetry-exporter-zipkin/src/platform/browser/util.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, }; } diff --git a/packages/opentelemetry-exporter-zipkin/src/platform/node/util.ts b/packages/opentelemetry-exporter-zipkin/src/platform/node/util.ts index 89f740b8e2b..668d11bb480 100644 --- a/packages/opentelemetry-exporter-zipkin/src/platform/node/util.ts +++ b/packages/opentelemetry-exporter-zipkin/src/platform/node/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, }, }, diff --git a/packages/opentelemetry-exporter-zipkin/src/utils.ts b/packages/opentelemetry-exporter-zipkin/src/utils.ts deleted file mode 100644 index ff29a5a5253..00000000000 --- a/packages/opentelemetry-exporter-zipkin/src/utils.ts +++ /dev/null @@ -1,17 +0,0 @@ -/* - * Copyright The 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 const OT_REQUEST_HEADER = 'x-opentelemetry-outgoing-request'; diff --git a/packages/opentelemetry-exporter-zipkin/test/node/zipkin.test.ts b/packages/opentelemetry-exporter-zipkin/test/node/zipkin.test.ts index 803f88838d2..df2604f5328 100644 --- a/packages/opentelemetry-exporter-zipkin/test/node/zipkin.test.ts +++ b/packages/opentelemetry-exporter-zipkin/test/node/zipkin.test.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') diff --git a/packages/opentelemetry-plugin-http/src/http.ts b/packages/opentelemetry-plugin-http/src/http.ts index 7f5e45b2583..6e94af1c6ec 100644 --- a/packages/opentelemetry-plugin-http/src/http.ts +++ b/packages/opentelemetry-plugin-http/src/http.ts @@ -398,15 +398,7 @@ export class HttpPlugin extends BasePlugin { 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, diff --git a/packages/opentelemetry-plugin-http/src/utils.ts b/packages/opentelemetry-plugin-http/src/utils.ts index cfc9b43cbc4..cfe8d9515e3 100644 --- a/packages/opentelemetry-plugin-http/src/utils.ts +++ b/packages/opentelemetry-plugin-http/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 diff --git a/packages/opentelemetry-plugin-http/test/functionals/http-enable.test.ts b/packages/opentelemetry-plugin-http/test/functionals/http-enable.test.ts index 60d34fa45ce..b78f504304e 100644 --- a/packages/opentelemetry-plugin-http/test/functionals/http-enable.test.ts +++ b/packages/opentelemetry-plugin-http/test/functionals/http-enable.test.ts @@ -36,7 +36,6 @@ import * as nock from 'nock'; import * as path from 'path'; import { HttpPlugin, plugin } from '../../src/http'; import { Http, HttpPluginConfig } from '../../src/types'; -import { OT_REQUEST_HEADER } from '../../src/utils'; import { assertSpan } from '../utils/assertSpan'; import { DummyPropagation } from '../utils/DummyPropagation'; import { httpRequest } from '../utils/httpRequest'; @@ -185,27 +184,6 @@ describe('HttpPlugin', () => { serverPort ); }); - - it(`should not trace requests with '${OT_REQUEST_HEADER}' header`, async () => { - const testPath = '/outgoing/do-not-trace'; - doNock(hostname, testPath, 200, 'Ok'); - - const options = { - host: hostname, - path: testPath, - headers: { [OT_REQUEST_HEADER]: 1 }, - }; - - const result = await httpRequest.get(options); - assert( - result.reqHeaders[OT_REQUEST_HEADER] === undefined, - 'custom header should be stripped' - ); - const spans = memoryExporter.getFinishedSpans(); - assert.strictEqual(result.data, 'Ok'); - assert.strictEqual(spans.length, 0); - assert.strictEqual(options.headers[OT_REQUEST_HEADER], 1); - }); }); describe('with good plugin options', () => { beforeEach(() => { @@ -303,26 +281,6 @@ describe('HttpPlugin', () => { }); }); - it(`should not trace requests with '${OT_REQUEST_HEADER}' header`, async () => { - const testPath = '/outgoing/do-not-trace'; - doNock(hostname, testPath, 200, 'Ok'); - - const options = { - host: hostname, - path: testPath, - headers: { [OT_REQUEST_HEADER]: 1 }, - }; - - const result = await httpRequest.get(options); - assert( - result.reqHeaders[OT_REQUEST_HEADER] === undefined, - 'custom header should be stripped' - ); - const spans = memoryExporter.getFinishedSpans(); - assert.strictEqual(result.data, 'Ok'); - assert.strictEqual(spans.length, 0); - }); - const httpErrorCodes = [ 400, 401, diff --git a/packages/opentelemetry-plugin-http/test/functionals/utils.test.ts b/packages/opentelemetry-plugin-http/test/functionals/utils.test.ts index 2ecc2806046..e69d6b72520 100644 --- a/packages/opentelemetry-plugin-http/test/functionals/utils.test.ts +++ b/packages/opentelemetry-plugin-http/test/functionals/utils.test.ts @@ -271,33 +271,6 @@ describe('Utility', () => { } }); }); - describe('isOpenTelemetryRequest()', () => { - [ - {}, - { headers: {} }, - url.parse('http://url.com'), - { headers: { [utils.OT_REQUEST_HEADER]: 0 } }, - { headers: { [utils.OT_REQUEST_HEADER]: false } }, - ].forEach(options => { - it(`should return false with the following value: ${JSON.stringify( - options - )}`, () => { - /* tslint:disable-next-line:no-any */ - assert.strictEqual(utils.isOpenTelemetryRequest(options as any), false); - }); - }); - for (const options of [ - { headers: { [utils.OT_REQUEST_HEADER]: 1 } }, - { headers: { [utils.OT_REQUEST_HEADER]: true } }, - ]) { - it(`should return true with the following value: ${JSON.stringify( - options - )}`, () => { - /* tslint:disable-next-line:no-any */ - assert.strictEqual(utils.isOpenTelemetryRequest(options as any), true); - }); - } - }); describe('isValidOptionsType()', () => { ['', false, true, 1, 0, []].forEach(options => { diff --git a/packages/opentelemetry-plugin-https/test/functionals/https-enable.test.ts b/packages/opentelemetry-plugin-https/test/functionals/https-enable.test.ts index 22c459d6b12..5bb2420da10 100644 --- a/packages/opentelemetry-plugin-https/test/functionals/https-enable.test.ts +++ b/packages/opentelemetry-plugin-https/test/functionals/https-enable.test.ts @@ -26,7 +26,6 @@ import { NodeTracerProvider } from '@opentelemetry/node'; import { Http, HttpPluginConfig, - OT_REQUEST_HEADER, } from '@opentelemetry/plugin-http'; import { AsyncHooksContextManager } from '@opentelemetry/context-async-hooks'; import { ContextManager } from '@opentelemetry/context-base'; @@ -186,22 +185,6 @@ describe('HttpsPlugin', () => { serverPort ); }); - - it(`should not trace requests with '${OT_REQUEST_HEADER}' header`, async () => { - const testPath = '/outgoing/do-not-trace'; - doNock(hostname, testPath, 200, 'Ok'); - - const options = { - host: hostname, - path: testPath, - headers: { [OT_REQUEST_HEADER]: 1 }, - }; - - const result = await httpsRequest.get(options); - const spans = memoryExporter.getFinishedSpans(); - assert.strictEqual(result.data, 'Ok'); - assert.strictEqual(spans.length, 0); - }); }); describe('with good plugin options', () => { beforeEach(() => {