From 806eabd0bc8cc8121d75bba23c832b9ef686633c Mon Sep 17 00:00:00 2001 From: Mark Wolff Date: Tue, 23 Jun 2020 22:46:42 -0400 Subject: [PATCH] refactor: deprecate attribute.component (#1220) * refactor: deprecate attribute.component * fix(tests): shift test attribute indexes Co-authored-by: Naseem Co-authored-by: Mayur Kale --- .../opentelemetry-plugin-grpc/src/grpc.ts | 14 +-- .../test/utils/assertionUtils.ts | 6 - .../opentelemetry-plugin-http/src/http.ts | 5 +- .../test/utils/assertSpan.ts | 4 - .../test/utils/assertSpan.ts | 4 - .../src/xhr.ts | 6 +- .../test/xhr.test.ts | 107 +++++++----------- .../src/trace/general.ts | 8 +- 8 files changed, 53 insertions(+), 101 deletions(-) diff --git a/packages/opentelemetry-plugin-grpc/src/grpc.ts b/packages/opentelemetry-plugin-grpc/src/grpc.ts index f0072ea569..c9cbe1fa33 100644 --- a/packages/opentelemetry-plugin-grpc/src/grpc.ts +++ b/packages/opentelemetry-plugin-grpc/src/grpc.ts @@ -23,10 +23,7 @@ import { SpanOptions, Status, } from '@opentelemetry/api'; -import { - GeneralAttribute, - RpcAttribute, -} from '@opentelemetry/semantic-conventions'; +import { RpcAttribute } from '@opentelemetry/semantic-conventions'; import { BasePlugin } from '@opentelemetry/core'; import * as events from 'events'; import * as grpcTypes from 'grpc'; @@ -177,7 +174,6 @@ export class GrpcPlugin extends BasePlugin { .startSpan(spanName, spanOptions) .setAttributes({ [RpcAttribute.GRPC_KIND]: spanOptions.kind, - [GeneralAttribute.COMPONENT]: GrpcPlugin.component, }); plugin._tracer.withSpan(span, () => { @@ -353,11 +349,9 @@ export class GrpcPlugin extends BasePlugin { return function clientMethodTrace(this: grpcTypes.Client) { const name = `grpc.${original.path.replace('/', '')}`; const args = Array.prototype.slice.call(arguments); - const span = plugin._tracer - .startSpan(name, { - kind: SpanKind.CLIENT, - }) - .setAttribute(GeneralAttribute.COMPONENT, GrpcPlugin.component); + const span = plugin._tracer.startSpan(name, { + kind: SpanKind.CLIENT, + }); return plugin._tracer.withSpan(span, () => plugin._makeGrpcClientRemoteCall(original, args, this, plugin)(span) ); diff --git a/packages/opentelemetry-plugin-grpc/test/utils/assertionUtils.ts b/packages/opentelemetry-plugin-grpc/test/utils/assertionUtils.ts index 38b0069d1e..f1f0a44fa7 100644 --- a/packages/opentelemetry-plugin-grpc/test/utils/assertionUtils.ts +++ b/packages/opentelemetry-plugin-grpc/test/utils/assertionUtils.ts @@ -16,14 +16,12 @@ import { SpanKind } from '@opentelemetry/api'; import * as assert from 'assert'; -import { GrpcPlugin } from '../../src/grpc'; import * as grpc from 'grpc'; import { ReadableSpan } from '@opentelemetry/tracing'; import { hrTimeToMilliseconds, hrTimeToMicroseconds, } from '@opentelemetry/core'; -import { GeneralAttribute } from '@opentelemetry/semantic-conventions'; export const assertSpan = ( span: ReadableSpan, @@ -34,10 +32,6 @@ export const assertSpan = ( assert.strictEqual(span.spanContext.spanId.length, 16); assert.strictEqual(span.kind, kind); - assert.strictEqual( - span.attributes[GeneralAttribute.COMPONENT], - GrpcPlugin.component - ); assert.ok(span.endTime); assert.strictEqual(span.links.length, 0); assert.strictEqual(span.events.length, 1); diff --git a/packages/opentelemetry-plugin-http/src/http.ts b/packages/opentelemetry-plugin-http/src/http.ts index 83b2792e9e..435de21195 100644 --- a/packages/opentelemetry-plugin-http/src/http.ts +++ b/packages/opentelemetry-plugin-http/src/http.ts @@ -29,7 +29,6 @@ import { NoRecordingSpan, getExtractedSpanContext, } from '@opentelemetry/core'; -import { GeneralAttribute } from '@opentelemetry/semantic-conventions'; import { ClientRequest, IncomingMessage, @@ -461,9 +460,7 @@ export class HttpPlugin extends BasePlugin { // https://github.com/open-telemetry/opentelemetry-specification/issues/530 span = new NoRecordingSpan(spanContext); } else { - span = this._tracer - .startSpan(name, options) - .setAttribute(GeneralAttribute.COMPONENT, this.component); + span = this._tracer.startSpan(name, options); } this._spanNotEnded.add(span); return span; diff --git a/packages/opentelemetry-plugin-http/test/utils/assertSpan.ts b/packages/opentelemetry-plugin-http/test/utils/assertSpan.ts index 77fbf04c12..0b02fc702d 100644 --- a/packages/opentelemetry-plugin-http/test/utils/assertSpan.ts +++ b/packages/opentelemetry-plugin-http/test/utils/assertSpan.ts @@ -48,10 +48,6 @@ export const assertSpan = ( span.name, `${validations.httpMethod} ${validations.pathname}` ); - assert.strictEqual( - span.attributes[GeneralAttribute.COMPONENT], - validations.component - ); assert.strictEqual( span.attributes[HttpAttribute.HTTP_ERROR_MESSAGE], span.status.message diff --git a/packages/opentelemetry-plugin-https/test/utils/assertSpan.ts b/packages/opentelemetry-plugin-https/test/utils/assertSpan.ts index 019ecbd466..d96db011c5 100644 --- a/packages/opentelemetry-plugin-https/test/utils/assertSpan.ts +++ b/packages/opentelemetry-plugin-https/test/utils/assertSpan.ts @@ -48,10 +48,6 @@ export const assertSpan = ( span.name, `${validations.httpMethod} ${validations.pathname}` ); - assert.strictEqual( - span.attributes[GeneralAttribute.COMPONENT], - validations.component - ); assert.strictEqual( span.attributes[HttpAttribute.HTTP_ERROR_MESSAGE], span.status.message diff --git a/packages/opentelemetry-plugin-xml-http-request/src/xhr.ts b/packages/opentelemetry-plugin-xml-http-request/src/xhr.ts index 59b59cc03d..c28d658f7b 100644 --- a/packages/opentelemetry-plugin-xml-http-request/src/xhr.ts +++ b/packages/opentelemetry-plugin-xml-http-request/src/xhr.ts @@ -22,10 +22,7 @@ import { isWrapped, otperformance, } from '@opentelemetry/core'; -import { - HttpAttribute, - GeneralAttribute, -} from '@opentelemetry/semantic-conventions'; +import { HttpAttribute } from '@opentelemetry/semantic-conventions'; import { addSpanNetworkEvents, getResource, @@ -279,7 +276,6 @@ export class XMLHttpRequestPlugin extends BasePlugin { const currentSpan = this._tracer.startSpan(url, { kind: api.SpanKind.CLIENT, attributes: { - [GeneralAttribute.COMPONENT]: this.component, [HttpAttribute.HTTP_METHOD]: method, [HttpAttribute.HTTP_URL]: url, }, diff --git a/packages/opentelemetry-plugin-xml-http-request/test/xhr.test.ts b/packages/opentelemetry-plugin-xml-http-request/test/xhr.test.ts index 339499f9f2..7887339acb 100644 --- a/packages/opentelemetry-plugin-xml-http-request/test/xhr.test.ts +++ b/packages/opentelemetry-plugin-xml-http-request/test/xhr.test.ts @@ -26,10 +26,7 @@ import { } from '@opentelemetry/core'; import { ZoneContextManager } from '@opentelemetry/context-zone'; import * as tracing from '@opentelemetry/tracing'; -import { - HttpAttribute, - GeneralAttribute, -} from '@opentelemetry/semantic-conventions'; +import { HttpAttribute } from '@opentelemetry/semantic-conventions'; import { PerformanceTimingNames as PTN, WebTracerProvider, @@ -283,45 +280,41 @@ describe('xhr', () => { const attributes = span.attributes; const keys = Object.keys(attributes); - assert.ok( - attributes[keys[0]] !== '', - `attributes ${GeneralAttribute.COMPONENT} is not defined` - ); assert.strictEqual( - attributes[keys[1]], + attributes[keys[0]], 'GET', `attributes ${HttpAttribute.HTTP_METHOD} is wrong` ); assert.strictEqual( - attributes[keys[2]], + attributes[keys[1]], url, `attributes ${HttpAttribute.HTTP_URL} is wrong` ); assert.strictEqual( - attributes[keys[3]], + attributes[keys[2]], 200, `attributes ${HttpAttribute.HTTP_STATUS_CODE} is wrong` ); assert.strictEqual( - attributes[keys[4]], + attributes[keys[3]], 'OK', `attributes ${HttpAttribute.HTTP_STATUS_TEXT} is wrong` ); assert.strictEqual( - attributes[keys[5]], + attributes[keys[4]], parseUrl(url).host, `attributes ${HttpAttribute.HTTP_HOST} is wrong` ); assert.ok( - attributes[keys[6]] === 'http' || attributes[keys[6]] === 'https', + attributes[keys[5]] === 'http' || attributes[keys[5]] === 'https', `attributes ${HttpAttribute.HTTP_SCHEME} is wrong` ); assert.ok( - attributes[keys[7]] !== '', + attributes[keys[6]] !== '', `attributes ${HttpAttribute.HTTP_USER_AGENT} is not defined` ); - assert.strictEqual(keys.length, 8, 'number of attributes is wrong'); + assert.strictEqual(keys.length, 7, 'number of attributes is wrong'); }); it('span should have correct events', () => { @@ -670,7 +663,7 @@ describe('xhr', () => { const keys = Object.keys(attributes); assert.strictEqual( - attributes[keys[2]], + attributes[keys[1]], secondUrl, `attribute ${HttpAttribute.HTTP_URL} is wrong` ); @@ -757,45 +750,41 @@ describe('xhr', () => { const attributes = span.attributes; const keys = Object.keys(attributes); - assert.ok( - attributes[keys[0]] !== '', - `attributes ${GeneralAttribute.COMPONENT} is not defined` - ); assert.strictEqual( - attributes[keys[1]], + attributes[keys[0]], 'GET', `attributes ${HttpAttribute.HTTP_METHOD} is wrong` ); assert.strictEqual( - attributes[keys[2]], + attributes[keys[1]], url, `attributes ${HttpAttribute.HTTP_URL} is wrong` ); assert.strictEqual( - attributes[keys[3]], + attributes[keys[2]], 400, `attributes ${HttpAttribute.HTTP_STATUS_CODE} is wrong` ); assert.strictEqual( - attributes[keys[4]], + attributes[keys[3]], 'Bad Request', `attributes ${HttpAttribute.HTTP_STATUS_TEXT} is wrong` ); assert.strictEqual( - attributes[keys[5]], + attributes[keys[4]], 'raw.githubusercontent.com', `attributes ${HttpAttribute.HTTP_HOST} is wrong` ); assert.ok( - attributes[keys[6]] === 'http' || attributes[keys[6]] === 'https', + attributes[keys[5]] === 'http' || attributes[keys[5]] === 'https', `attributes ${HttpAttribute.HTTP_SCHEME} is wrong` ); assert.ok( - attributes[keys[7]] !== '', + attributes[keys[6]] !== '', `attributes ${HttpAttribute.HTTP_USER_AGENT} is not defined` ); - assert.strictEqual(keys.length, 8, 'number of attributes is wrong'); + assert.strictEqual(keys.length, 7, 'number of attributes is wrong'); }); it('span should have correct events', () => { @@ -888,45 +877,41 @@ describe('xhr', () => { const attributes = span.attributes; const keys = Object.keys(attributes); - assert.ok( - attributes[keys[0]] !== '', - `attributes ${GeneralAttribute.COMPONENT} is not defined` - ); assert.strictEqual( - attributes[keys[1]], + attributes[keys[0]], 'GET', `attributes ${HttpAttribute.HTTP_METHOD} is wrong` ); assert.strictEqual( - attributes[keys[2]], + attributes[keys[1]], url, `attributes ${HttpAttribute.HTTP_URL} is wrong` ); assert.strictEqual( - attributes[keys[3]], + attributes[keys[2]], 0, `attributes ${HttpAttribute.HTTP_STATUS_CODE} is wrong` ); assert.strictEqual( - attributes[keys[4]], + attributes[keys[3]], '', `attributes ${HttpAttribute.HTTP_STATUS_TEXT} is wrong` ); assert.strictEqual( - attributes[keys[5]], + attributes[keys[4]], 'raw.githubusercontent.com', `attributes ${HttpAttribute.HTTP_HOST} is wrong` ); assert.ok( - attributes[keys[6]] === 'http' || attributes[keys[6]] === 'https', + attributes[keys[5]] === 'http' || attributes[keys[5]] === 'https', `attributes ${HttpAttribute.HTTP_SCHEME} is wrong` ); assert.ok( - attributes[keys[7]] !== '', + attributes[keys[6]] !== '', `attributes ${HttpAttribute.HTTP_USER_AGENT} is not defined` ); - assert.strictEqual(keys.length, 8, 'number of attributes is wrong'); + assert.strictEqual(keys.length, 7, 'number of attributes is wrong'); }); it('span should have correct events', () => { @@ -981,45 +966,41 @@ describe('xhr', () => { const attributes = span.attributes; const keys = Object.keys(attributes); - assert.ok( - attributes[keys[0]] !== '', - `attributes ${GeneralAttribute.COMPONENT} is not defined` - ); assert.strictEqual( - attributes[keys[1]], + attributes[keys[0]], 'GET', `attributes ${HttpAttribute.HTTP_METHOD} is wrong` ); assert.strictEqual( - attributes[keys[2]], + attributes[keys[1]], url, `attributes ${HttpAttribute.HTTP_URL} is wrong` ); assert.strictEqual( - attributes[keys[3]], + attributes[keys[2]], 0, `attributes ${HttpAttribute.HTTP_STATUS_CODE} is wrong` ); assert.strictEqual( - attributes[keys[4]], + attributes[keys[3]], '', `attributes ${HttpAttribute.HTTP_STATUS_TEXT} is wrong` ); assert.strictEqual( - attributes[keys[5]], + attributes[keys[4]], 'raw.githubusercontent.com', `attributes ${HttpAttribute.HTTP_HOST} is wrong` ); assert.ok( - attributes[keys[6]] === 'http' || attributes[keys[6]] === 'https', + attributes[keys[5]] === 'http' || attributes[keys[5]] === 'https', `attributes ${HttpAttribute.HTTP_SCHEME} is wrong` ); assert.ok( - attributes[keys[7]] !== '', + attributes[keys[6]] !== '', `attributes ${HttpAttribute.HTTP_USER_AGENT} is not defined` ); - assert.strictEqual(keys.length, 8, 'number of attributes is wrong'); + assert.strictEqual(keys.length, 7, 'number of attributes is wrong'); }); it('span should have correct events', () => { @@ -1076,45 +1057,41 @@ describe('xhr', () => { const attributes = span.attributes; const keys = Object.keys(attributes); - assert.ok( - attributes[keys[0]] !== '', - `attributes ${GeneralAttribute.COMPONENT} is not defined` - ); assert.strictEqual( - attributes[keys[1]], + attributes[keys[0]], 'GET', `attributes ${HttpAttribute.HTTP_METHOD} is wrong` ); assert.strictEqual( - attributes[keys[2]], + attributes[keys[1]], url, `attributes ${HttpAttribute.HTTP_URL} is wrong` ); assert.strictEqual( - attributes[keys[3]], + attributes[keys[2]], 0, `attributes ${HttpAttribute.HTTP_STATUS_CODE} is wrong` ); assert.strictEqual( - attributes[keys[4]], + attributes[keys[3]], '', `attributes ${HttpAttribute.HTTP_STATUS_TEXT} is wrong` ); assert.strictEqual( - attributes[keys[5]], + attributes[keys[4]], 'raw.githubusercontent.com', `attributes ${HttpAttribute.HTTP_HOST} is wrong` ); assert.ok( - attributes[keys[6]] === 'http' || attributes[keys[6]] === 'https', + attributes[keys[5]] === 'http' || attributes[keys[5]] === 'https', `attributes ${HttpAttribute.HTTP_SCHEME} is wrong` ); assert.ok( - attributes[keys[7]] !== '', + attributes[keys[6]] !== '', `attributes ${HttpAttribute.HTTP_USER_AGENT} is not defined` ); - assert.strictEqual(keys.length, 8, 'number of attributes is wrong'); + assert.strictEqual(keys.length, 7, 'number of attributes is wrong'); }); it('span should have correct events', () => { diff --git a/packages/opentelemetry-semantic-conventions/src/trace/general.ts b/packages/opentelemetry-semantic-conventions/src/trace/general.ts index d52f4ff449..281f9045ac 100644 --- a/packages/opentelemetry-semantic-conventions/src/trace/general.ts +++ b/packages/opentelemetry-semantic-conventions/src/trace/general.ts @@ -13,10 +13,12 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -export const GeneralAttribute = { - // Not in spec - COMPONENT: 'component', +/** + * General purpose networking attributes defined by the OpenTelemetry Semantic Conventions Specification + * https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/trace/semantic_conventions/span-general.md + */ +export const GeneralAttribute = { NET_PEER_IP: 'net.peer.ip', NET_PEER_ADDRESS: 'net.peer.address', NET_PEER_HOSTNAME: 'net.peer.host',