Skip to content

Commit

Permalink
chore(http): remove x-opentelemetry-outgoing-request header #1547
Browse files Browse the repository at this point in the history
  • Loading branch information
vmarchaud committed Sep 19, 2020
1 parent 3309ea4 commit fa9e376
Show file tree
Hide file tree
Showing 20 changed files with 6 additions and 268 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
3 changes: 0 additions & 3 deletions packages/opentelemetry-exporter-collector/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
2 changes: 0 additions & 2 deletions packages/opentelemetry-exporter-jaeger/src/jaeger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand Down
17 changes: 0 additions & 17 deletions packages/opentelemetry-exporter-jaeger/src/utils.ts

This file was deleted.

8 changes: 1 addition & 7 deletions packages/opentelemetry-exporter-jaeger/test/jaeger.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down Expand Up @@ -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'
Expand All @@ -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',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -33,7 +32,6 @@ export function prepareSend(
xhrHeaders = {
Accept: 'application/json',
'Content-Type': 'application/json',
[OT_REQUEST_HEADER]: '1',
...headers,
};
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -37,7 +36,6 @@ export function prepareSend(
method: 'POST',
headers: {
'Content-Type': 'application/json',
[OT_REQUEST_HEADER]: 1,
...headers,
},
},
Expand Down
17 changes: 0 additions & 17 deletions packages/opentelemetry-exporter-zipkin/src/utils.ts

This file was deleted.

21 changes: 0 additions & 21 deletions packages/opentelemetry-exporter-zipkin/test/node/zipkin.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -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')
Expand Down
16 changes: 0 additions & 16 deletions packages/opentelemetry-grpc-utils/test/grpcUtils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down
4 changes: 0 additions & 4 deletions packages/opentelemetry-plugin-grpc-js/src/client/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ import {
grpcStatusCodeToSpanStatus,
grpcStatusCodeToCanonicalCode,
CALL_SPAN_ENDED,
containsOtelMetadata,
methodIsIgnored,
} from '../utils';
import { EventEmitter } from 'events';
Expand Down Expand Up @@ -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,
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -142,7 +142,6 @@ function shouldNotTraceServerCall(
): boolean {
const parsedName = methodName.split('/');
return (
containsOtelMetadata(metadata) ||
methodIsIgnored(
parsedName[parsedName.length - 1] || methodName,
ignoreGrpcMethods
Expand Down
13 changes: 0 additions & 13 deletions packages/opentelemetry-plugin-grpc-js/src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
13 changes: 3 additions & 10 deletions packages/opentelemetry-plugin-grpc/src/grpc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ import {
_grpcStatusCodeToCanonicalCode,
_grpcStatusCodeToSpanStatus,
_methodIsIgnored,
_containsOtelMetadata,
} from './utils';
import { VERSION } from './version';

Expand Down Expand Up @@ -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
);
}

Expand Down Expand Up @@ -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,
});
Expand Down
15 changes: 0 additions & 15 deletions packages/opentelemetry-plugin-grpc/src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down
8 changes: 0 additions & 8 deletions packages/opentelemetry-plugin-http/src/http.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
24 changes: 0 additions & 24 deletions packages/opentelemetry-plugin-http/src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down
Loading

0 comments on commit fa9e376

Please sign in to comment.