From 9420bccef143b24bda95532362ff2bbacbcf4a65 Mon Sep 17 00:00:00 2001
From: legendecas <legendecas@gmail.com>
Date: Mon, 4 Apr 2022 16:49:33 +0800
Subject: [PATCH 1/3] fix: sanitize attributes inputs
---
CHANGELOG.md | 2 +
.../src/common/attributes.ts | 20 ++--
.../src/trace/sampler/ParentBasedSampler.ts | 4 +-
.../src/transform.ts | 4 +-
.../opentelemetry-resources/src/Resource.ts | 2 +-
.../opentelemetry-sdk-trace-base/src/Span.ts | 21 ++--
.../src/TimedEvent.ts | 4 +-
.../src/Tracer.ts | 13 ++-
.../src/export/ReadableSpan.ts | 4 +-
.../test/common/Span.test.ts | 100 ++++++++----------
.../test/common/Tracer.test.ts | 42 +++++++-
.../test/common/util.ts | 33 ++++++
.../src/shim.ts | 18 ++--
.../test/Shim.test.ts | 4 +-
14 files changed, 173 insertions(+), 98 deletions(-)
create mode 100644 packages/opentelemetry-sdk-trace-base/test/common/util.ts
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 02999c34c9c..6349f7efab7 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -10,6 +10,8 @@ All notable changes to this project will be documented in this file.
### :bug: (Bug Fix)
+* [#2881](https://github.com/open-telemetry/opentelemetry-js/pull/2881) fix: sanitize attributes inputs ([@legendecas](https://githuc.com/legendecas))
+
### :books: (Refine Doc)
* [#2860](https://github.com/open-telemetry/opentelemetry-js/pull/2860) docs(sdk): update earliest support node version ([@svetlanabrennan](https://github.com/svetlanabrennan))
diff --git a/packages/opentelemetry-core/src/common/attributes.ts b/packages/opentelemetry-core/src/common/attributes.ts
index 34fe493d9d8..3eefcd1cfdb 100644
--- a/packages/opentelemetry-core/src/common/attributes.ts
+++ b/packages/opentelemetry-core/src/common/attributes.ts
@@ -13,17 +13,19 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
-import { SpanAttributeValue, SpanAttributes } from '@opentelemetry/api';
+import { AttributeValue, Attributes } from '@opentelemetry/api';
-export function sanitizeAttributes(attributes: unknown): SpanAttributes {
- const out: SpanAttributes = {};
+export function sanitizeAttributes(attributes: unknown): Attributes {
+ const out: Attributes = {};
- if (attributes == null || typeof attributes !== 'object') {
+ if (typeof attributes !== 'object' || attributes == null) {
return out;
}
- // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
- for (const [k, v] of Object.entries(attributes!)) {
+ for (const [k, v] of Object.entries(attributes)) {
+ if (!isAttributeKey(k)) {
+ continue;
+ }
if (isAttributeValue(v)) {
if (Array.isArray(v)) {
out[k] = v.slice();
@@ -36,7 +38,11 @@ export function sanitizeAttributes(attributes: unknown): SpanAttributes {
return out;
}
-export function isAttributeValue(val: unknown): val is SpanAttributeValue {
+export function isAttributeKey(key: unknown): key is string {
+ return typeof key === 'string' && key.length > 0;
+}
+
+export function isAttributeValue(val: unknown): val is AttributeValue {
if (val == null) {
return true;
}
diff --git a/packages/opentelemetry-core/src/trace/sampler/ParentBasedSampler.ts b/packages/opentelemetry-core/src/trace/sampler/ParentBasedSampler.ts
index 6cd10b1484d..aba16a18b79 100644
--- a/packages/opentelemetry-core/src/trace/sampler/ParentBasedSampler.ts
+++ b/packages/opentelemetry-core/src/trace/sampler/ParentBasedSampler.ts
@@ -15,7 +15,7 @@
*/
import {
- SpanAttributes,
+ Attributes,
Context,
isSpanContextValid,
Link,
@@ -64,7 +64,7 @@ export class ParentBasedSampler implements Sampler {
traceId: string,
spanName: string,
spanKind: SpanKind,
- attributes: SpanAttributes,
+ attributes: Attributes,
links: Link[]
): SamplingResult {
const parentContext = trace.getSpanContext(context);
diff --git a/packages/opentelemetry-exporter-zipkin/src/transform.ts b/packages/opentelemetry-exporter-zipkin/src/transform.ts
index 9ffd48ecce7..1b3a444a678 100644
--- a/packages/opentelemetry-exporter-zipkin/src/transform.ts
+++ b/packages/opentelemetry-exporter-zipkin/src/transform.ts
@@ -66,9 +66,9 @@ export function toZipkinSpan(
return zipkinSpan;
}
-/** Converts OpenTelemetry SpanAttributes and SpanStatus to Zipkin Tags format. */
+/** Converts OpenTelemetry Attributes and SpanStatus to Zipkin Tags format. */
export function _toZipkinTags(
- attributes: api.SpanAttributes,
+ attributes: api.Attributes,
status: api.SpanStatus,
statusCodeTagName: string,
statusErrorTagName: string,
diff --git a/packages/opentelemetry-resources/src/Resource.ts b/packages/opentelemetry-resources/src/Resource.ts
index ac368d812ee..55853ac98ea 100644
--- a/packages/opentelemetry-resources/src/Resource.ts
+++ b/packages/opentelemetry-resources/src/Resource.ts
@@ -68,7 +68,7 @@ export class Resource {
merge(other: Resource | null): Resource {
if (!other || !Object.keys(other.attributes).length) return this;
- // SpanAttributes from resource overwrite attributes from other resource.
+ // Attributes from resource overwrite attributes from other resource.
const mergedAttributes = Object.assign(
{},
this.attributes,
diff --git a/packages/opentelemetry-sdk-trace-base/src/Span.ts b/packages/opentelemetry-sdk-trace-base/src/Span.ts
index 79ceae7ffcf..343ceebf030 100644
--- a/packages/opentelemetry-sdk-trace-base/src/Span.ts
+++ b/packages/opentelemetry-sdk-trace-base/src/Span.ts
@@ -22,6 +22,7 @@ import {
InstrumentationLibrary,
isTimeInput,
timeInputToHrTime,
+ sanitizeAttributes,
} from '@opentelemetry/core';
import { Resource } from '@opentelemetry/resources';
import { SemanticAttributes } from '@opentelemetry/semantic-conventions';
@@ -30,7 +31,7 @@ import { TimedEvent } from './TimedEvent';
import { Tracer } from './Tracer';
import { SpanProcessor } from './SpanProcessor';
import { SpanLimits } from './types';
-import { SpanAttributeValue, Context } from '@opentelemetry/api';
+import { AttributeValue, Context } from '@opentelemetry/api';
import { ExceptionEventName } from './enums';
/**
@@ -42,7 +43,7 @@ export class Span implements api.Span, ReadableSpan {
private readonly _spanContext: api.SpanContext;
readonly kind: api.SpanKind;
readonly parentSpanId?: string;
- readonly attributes: api.SpanAttributes = {};
+ readonly attributes: api.Attributes = {};
readonly links: api.Link[] = [];
readonly events: TimedEvent[] = [];
readonly startTime: api.HrTime;
@@ -88,7 +89,7 @@ export class Span implements api.Span, ReadableSpan {
return this._spanContext;
}
- setAttribute(key: string, value?: SpanAttributeValue): this;
+ setAttribute(key: string, value?: AttributeValue): this;
setAttribute(key: string, value: unknown): this {
if (value == null || this._isSpanEnded()) return this;
if (key.length === 0) {
@@ -111,7 +112,7 @@ export class Span implements api.Span, ReadableSpan {
return this;
}
- setAttributes(attributes: api.SpanAttributes): this {
+ setAttributes(attributes: api.Attributes): this {
for (const [k, v] of Object.entries(attributes)) {
this.setAttribute(k, v);
}
@@ -127,7 +128,7 @@ export class Span implements api.Span, ReadableSpan {
*/
addEvent(
name: string,
- attributesOrStartTime?: api.SpanAttributes | api.TimeInput,
+ attributesOrStartTime?: api.Attributes | api.TimeInput,
startTime?: api.TimeInput
): this {
if (this._isSpanEnded()) return this;
@@ -148,9 +149,11 @@ export class Span implements api.Span, ReadableSpan {
if (typeof startTime === 'undefined') {
startTime = hrTime();
}
+
+ const attributes = sanitizeAttributes(attributesOrStartTime);
this.events.push({
name,
- attributes: attributesOrStartTime as api.SpanAttributes,
+ attributes,
time: timeInputToHrTime(startTime),
});
return this;
@@ -193,7 +196,7 @@ export class Span implements api.Span, ReadableSpan {
}
recordException(exception: api.Exception, time: api.TimeInput = hrTime()): void {
- const attributes: api.SpanAttributes = {};
+ const attributes: api.Attributes = {};
if (typeof exception === 'string') {
attributes[SemanticAttributes.EXCEPTION_MESSAGE] = exception;
} else if (exception) {
@@ -217,7 +220,7 @@ export class Span implements api.Span, ReadableSpan {
attributes[SemanticAttributes.EXCEPTION_TYPE] ||
attributes[SemanticAttributes.EXCEPTION_MESSAGE]
) {
- this.addEvent(ExceptionEventName, attributes as api.SpanAttributes, time);
+ this.addEvent(ExceptionEventName, attributes, time);
} else {
api.diag.warn(`Failed to record an exception ${exception}`);
}
@@ -260,7 +263,7 @@ export class Span implements api.Span, ReadableSpan {
* @param value Attribute value
* @returns truncated attribute value if required, otherwise same value
*/
- private _truncateToSize(value: SpanAttributeValue): SpanAttributeValue {
+ private _truncateToSize(value: AttributeValue): AttributeValue {
const limit = this._attributeValueLengthLimit;
// Check limit
if (limit <= 0) {
diff --git a/packages/opentelemetry-sdk-trace-base/src/TimedEvent.ts b/packages/opentelemetry-sdk-trace-base/src/TimedEvent.ts
index 93cb9b47f3f..862a14201dd 100644
--- a/packages/opentelemetry-sdk-trace-base/src/TimedEvent.ts
+++ b/packages/opentelemetry-sdk-trace-base/src/TimedEvent.ts
@@ -14,7 +14,7 @@
* limitations under the License.
*/
-import { HrTime, SpanAttributes } from '@opentelemetry/api';
+import { HrTime, Attributes } from '@opentelemetry/api';
/**
* Represents a timed event.
@@ -25,5 +25,5 @@ export interface TimedEvent {
/** The name of the event. */
name: string;
/** The attributes of the event. */
- attributes?: SpanAttributes;
+ attributes?: Attributes;
}
diff --git a/packages/opentelemetry-sdk-trace-base/src/Tracer.ts b/packages/opentelemetry-sdk-trace-base/src/Tracer.ts
index 5d85373a461..aa4321d5e50 100644
--- a/packages/opentelemetry-sdk-trace-base/src/Tracer.ts
+++ b/packages/opentelemetry-sdk-trace-base/src/Tracer.ts
@@ -92,7 +92,12 @@ export class Tracer implements api.Tracer {
}
const spanKind = options.kind ?? api.SpanKind.INTERNAL;
- const links = options.links ?? [];
+ const links = (options.links ?? []).map(link => {
+ return {
+ context: link.context,
+ attributes: sanitizeAttributes(link.attributes),
+ };
+ });
const attributes = sanitizeAttributes(options.attributes);
// make sampling decision
const samplingResult = this._sampler.shouldSample(
@@ -124,8 +129,10 @@ export class Tracer implements api.Tracer {
links,
options.startTime
);
- // Set default attributes
- span.setAttributes(Object.assign(attributes, samplingResult.attributes));
+ // Set initial span attributes. The attributes object may have been mutated
+ // by the sampler, so we sanitize the merged attributes before setting them.
+ const initAttributes = sanitizeAttributes(Object.assign(attributes, samplingResult.attributes));
+ span.setAttributes(initAttributes);
return span;
}
diff --git a/packages/opentelemetry-sdk-trace-base/src/export/ReadableSpan.ts b/packages/opentelemetry-sdk-trace-base/src/export/ReadableSpan.ts
index 8552134a569..08631e9d6d9 100644
--- a/packages/opentelemetry-sdk-trace-base/src/export/ReadableSpan.ts
+++ b/packages/opentelemetry-sdk-trace-base/src/export/ReadableSpan.ts
@@ -17,7 +17,7 @@
import {
SpanKind,
SpanStatus,
- SpanAttributes,
+ Attributes,
HrTime,
Link,
SpanContext,
@@ -34,7 +34,7 @@ export interface ReadableSpan {
readonly startTime: HrTime;
readonly endTime: HrTime;
readonly status: SpanStatus;
- readonly attributes: SpanAttributes;
+ readonly attributes: Attributes;
readonly links: Link[];
readonly events: TimedEvent[];
readonly duration: HrTime;
diff --git a/packages/opentelemetry-sdk-trace-base/test/common/Span.test.ts b/packages/opentelemetry-sdk-trace-base/test/common/Span.test.ts
index 2a81b4c42f8..ebb40a33c18 100644
--- a/packages/opentelemetry-sdk-trace-base/test/common/Span.test.ts
+++ b/packages/opentelemetry-sdk-trace-base/test/common/Span.test.ts
@@ -22,6 +22,8 @@ import {
SpanKind,
TraceFlags,
HrTime,
+ Attributes,
+ AttributeValue,
} from '@opentelemetry/api';
import {
DEFAULT_ATTRIBUTE_COUNT_LIMIT,
@@ -35,6 +37,7 @@ import { SemanticAttributes } from '@opentelemetry/semantic-conventions';
import * as assert from 'assert';
import * as sinon from 'sinon';
import { BasicTracerProvider, Span, SpanProcessor } from '../../src';
+import { invalidAttributes, validAttributes } from './util';
const performanceTimeOrigin: HrTime = [1, 1];
@@ -237,28 +240,14 @@ describe('Span', () => {
SpanKind.CLIENT
);
- span.setAttribute('string', 'string');
- span.setAttribute('number', 0);
- span.setAttribute('bool', true);
- span.setAttribute('array<string>', ['str1', 'str2']);
- span.setAttribute('array<number>', [1, 2]);
- span.setAttribute('array<bool>', [true, false]);
-
- //@ts-expect-error invalid attribute type object
- span.setAttribute('object', { foo: 'bar' });
- //@ts-expect-error invalid attribute inhomogenous array
- span.setAttribute('non-homogeneous-array', [0, '']);
- // This empty length attribute should not be set
- span.setAttribute('', 'empty-key');
+ for (const [k, v] of Object.entries(validAttributes)) {
+ span.setAttribute(k, v);
+ }
+ for (const [k, v] of Object.entries(invalidAttributes)) {
+ span.setAttribute(k, v as unknown as AttributeValue);
+ }
- assert.deepStrictEqual(span.attributes, {
- string: 'string',
- number: 0,
- bool: true,
- 'array<string>': ['str1', 'str2'],
- 'array<number>': [1, 2],
- 'array<bool>': [true, false],
- });
+ assert.deepStrictEqual(span.attributes, validAttributes);
});
it('should be able to overwrite attributes', () => {
@@ -623,43 +612,42 @@ describe('Span', () => {
SpanKind.CLIENT
);
- span.setAttributes({
- string: 'string',
- number: 0,
- bool: true,
- 'array<string>': ['str1', 'str2'],
- 'array<number>': [1, 2],
- 'array<bool>': [true, false],
- //@ts-expect-error invalid attribute type object
- object: { foo: 'bar' },
- //@ts-expect-error invalid attribute inhomogenous array
- 'non-homogeneous-array': [0, ''],
- // This empty length attribute should not be set
- '': 'empty-key',
- });
+ span.setAttributes(validAttributes);
+ span.setAttributes(invalidAttributes as unknown as Attributes);
- assert.deepStrictEqual(span.attributes, {
- string: 'string',
- number: 0,
- bool: true,
- 'array<string>': ['str1', 'str2'],
- 'array<number>': [1, 2],
- 'array<bool>': [true, false],
- });
+ assert.deepStrictEqual(span.attributes, validAttributes);
});
});
- it('should set an event', () => {
- const span = new Span(
- tracer,
- ROOT_CONTEXT,
- name,
- spanContext,
- SpanKind.CLIENT
- );
- span.addEvent('sent');
- span.addEvent('rev', { attr1: 'value', attr2: 123, attr3: true });
- span.end();
+ describe('addEvent', () => {
+ it('should add an event', () => {
+ const span = new Span(
+ tracer,
+ ROOT_CONTEXT,
+ name,
+ spanContext,
+ SpanKind.CLIENT
+ );
+ span.addEvent('sent');
+ span.addEvent('rev', { attr1: 'value', attr2: 123, attr3: true });
+ span.end();
+ });
+
+ it('should sanitize attribute values', () => {
+ const span = new Span(
+ tracer,
+ ROOT_CONTEXT,
+ name,
+ spanContext,
+ SpanKind.CLIENT
+ );
+ span.addEvent('rev', { ...validAttributes, ...invalidAttributes } as unknown as Attributes);
+ span.end();
+
+ assert.strictEqual(span.events.length, 1);
+ assert.deepStrictEqual(span.events[0].name, 'rev');
+ assert.deepStrictEqual(span.events[0].attributes, validAttributes);
+ });
});
it('should set a link', () => {
@@ -836,14 +824,14 @@ describe('Span', () => {
assert.strictEqual(span.events.length, 1);
const [event] = span.events;
assert.deepStrictEqual(event.name, 'sent');
- assert.ok(!event.attributes);
+ assert.deepStrictEqual(event.attributes, {});
assert.ok(event.time[0] > 0);
span.addEvent('rev', { attr1: 'value', attr2: 123, attr3: true });
assert.strictEqual(span.events.length, 2);
const [event1, event2] = span.events;
assert.deepStrictEqual(event1.name, 'sent');
- assert.ok(!event1.attributes);
+ assert.deepStrictEqual(event1.attributes, {});
assert.ok(event1.time[0] > 0);
assert.deepStrictEqual(event2.name, 'rev');
assert.deepStrictEqual(event2.attributes, {
diff --git a/packages/opentelemetry-sdk-trace-base/test/common/Tracer.test.ts b/packages/opentelemetry-sdk-trace-base/test/common/Tracer.test.ts
index 2962d8d7e6e..573346548ee 100644
--- a/packages/opentelemetry-sdk-trace-base/test/common/Tracer.test.ts
+++ b/packages/opentelemetry-sdk-trace-base/test/common/Tracer.test.ts
@@ -14,9 +14,12 @@
* limitations under the License.
*/
import {
+ Attributes,
+ Context,
context,
createContextKey,
INVALID_TRACEID,
+ Link,
ROOT_CONTEXT,
Sampler,
SamplingDecision,
@@ -30,12 +33,14 @@ import {
AlwaysOffSampler,
AlwaysOnSampler,
InstrumentationLibrary,
+ sanitizeAttributes,
suppressTracing
} from '@opentelemetry/core';
import * as assert from 'assert';
import { BasicTracerProvider, Span, SpanProcessor, Tracer } from '../../src';
import { TestStackContextManager } from './export/TestStackContextManager';
import * as sinon from 'sinon';
+import { invalidAttributes, validAttributes } from './util';
describe('Tracer', () => {
const tracerProvider = new BasicTracerProvider();
@@ -44,12 +49,19 @@ describe('Tracer', () => {
: process.env) as any;
class TestSampler implements Sampler {
- shouldSample() {
+ shouldSample(_context: Context, _traceId: string, _spanName: string, _spanKind: SpanKind, attributes: Attributes, links: Link[]) {
+ // The attributes object should be valid.
+ assert.deepStrictEqual(sanitizeAttributes(attributes), attributes);
+ links.forEach(link => {
+ assert.deepStrictEqual(sanitizeAttributes(link.attributes), link.attributes);
+ });
return {
decision: SamplingDecision.RECORD_AND_SAMPLED,
attributes: {
testAttribute: 'foobar',
- },
+ // invalid attributes should be sanitized.
+ ...invalidAttributes,
+ } as unknown as Attributes,
};
}
}
@@ -312,7 +324,6 @@ describe('Tracer', () => {
});
it('should start an active span with name, options and function args', () => {
-
const tracer = new Tracer(
{ name: 'default', version: '0.0.1' },
{ sampler: new TestSampler() },
@@ -356,4 +367,29 @@ describe('Tracer', () => {
}
}), 1);
});
+
+ it('should sample with valid attributes', () => {
+ const tracer = new Tracer(
+ { name: 'default', version: '0.0.1' },
+ { sampler: new TestSampler() },
+ tracerProvider
+ );
+
+ const attributes = { ...validAttributes, ...invalidAttributes } as unknown as Attributes;
+ const links = [{
+ context: {
+ traceId: 'b3cda95b652f4a1592b449d5929fda1b',
+ spanId: '6e0c63257de34c92',
+ traceFlags: TraceFlags.SAMPLED
+ },
+ attributes: { ...attributes },
+ }];
+ // TestSampler should validate the attributes and links.
+ const span = tracer.startSpan('my-span', { attributes, links }) as Span;
+ span.end();
+
+ assert.deepStrictEqual(span.attributes, { ...validAttributes, testAttribute: 'foobar' });
+ assert.strictEqual(span.links.length, 1);
+ assert.deepStrictEqual(span.links[0].attributes, validAttributes);
+ });
});
diff --git a/packages/opentelemetry-sdk-trace-base/test/common/util.ts b/packages/opentelemetry-sdk-trace-base/test/common/util.ts
new file mode 100644
index 00000000000..6b27471c285
--- /dev/null
+++ b/packages/opentelemetry-sdk-trace-base/test/common/util.ts
@@ -0,0 +1,33 @@
+/*
+ * 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 validAttributes = {
+ string: 'string',
+ number: 0,
+ bool: true,
+ 'array<string>': ['str1', 'str2'],
+ 'array<number>': [1, 2],
+ 'array<bool>': [true, false],
+};
+
+export const invalidAttributes = {
+ // invalid attribute type object
+ object: { foo: 'bar' },
+ // invalid attribute inhomogenous array
+ 'non-homogeneous-array': [0, ''],
+ // This empty length attribute should not be set
+ '': 'empty-key',
+};
diff --git a/packages/opentelemetry-shim-opentracing/src/shim.ts b/packages/opentelemetry-shim-opentracing/src/shim.ts
index b9fee53adce..5d303ca3121 100644
--- a/packages/opentelemetry-shim-opentracing/src/shim.ts
+++ b/packages/opentelemetry-shim-opentracing/src/shim.ts
@@ -15,7 +15,7 @@
*/
import * as api from '@opentelemetry/api';
-import { SpanAttributes, SpanAttributeValue, SpanStatusCode, TextMapPropagator } from '@opentelemetry/api';
+import { Attributes, AttributeValue, SpanStatusCode, TextMapPropagator } from '@opentelemetry/api';
import * as opentracing from 'opentracing';
import { SemanticAttributes } from '@opentelemetry/semantic-conventions';
@@ -276,7 +276,7 @@ export class SpanShim extends opentracing.Span {
* @param eventName name of the event.
* @param payload an arbitrary object to be attached to the event.
*/
- override logEvent(eventName: string, payload?: SpanAttributes): void {
+ override logEvent(eventName: string, payload?: Attributes): void {
this._logInternal(eventName, payload);
}
@@ -286,7 +286,7 @@ export class SpanShim extends opentracing.Span {
* @param keyValuePairs a set of key-value pairs to be used as event attributes
* @param timestamp optional timestamp for the event
*/
- override log(keyValuePairs: SpanAttributes, timestamp?: number): this {
+ override log(keyValuePairs: Attributes, timestamp?: number): this {
const entries = Object.entries(keyValuePairs);
const eventEntry = entries.find(([key, _]) => key === 'event');
const eventName = eventEntry?.[1] || 'log';
@@ -296,7 +296,7 @@ export class SpanShim extends opentracing.Span {
return this;
}
- private _logInternal(eventName: string, attributes: SpanAttributes | undefined, timestamp?: number): void {
+ private _logInternal(eventName: string, attributes: Attributes | undefined, timestamp?: number): void {
if (attributes && eventName === 'error') {
const entries = Object.entries(attributes);
const errorEntry = entries.find(([key]) => key === 'error.object');
@@ -306,7 +306,7 @@ export class SpanShim extends opentracing.Span {
return;
}
- const mappedAttributes: api.SpanAttributes = {};
+ const mappedAttributes: api.Attributes = {};
for (const [k, v] of entries) {
switch (k) {
case 'error.kind': {
@@ -337,7 +337,7 @@ export class SpanShim extends opentracing.Span {
* Adds a set of tags to the span.
* @param keyValueMap set of KV pairs representing tags
*/
- override addTags(keyValueMap: SpanAttributes): this {
+ override addTags(keyValueMap: Attributes): this {
for (const [key, value] of Object.entries(keyValueMap)) {
if (this._setErrorAsSpanStatusCode(key, value)) {
continue;
@@ -355,7 +355,7 @@ export class SpanShim extends opentracing.Span {
* @param key key for the tag
* @param value value for the tag
*/
- override setTag(key: string, value: SpanAttributeValue): this {
+ override setTag(key: string, value: AttributeValue): this {
if (this._setErrorAsSpanStatusCode(key, value)) {
return this;
}
@@ -383,7 +383,7 @@ export class SpanShim extends opentracing.Span {
private _setErrorAsSpanStatusCode(
key: string,
- value: SpanAttributeValue | undefined
+ value: AttributeValue | undefined
): boolean {
if (key === opentracing.Tags.ERROR) {
const statusCode = SpanShim._mapErrorTag(value);
@@ -394,7 +394,7 @@ export class SpanShim extends opentracing.Span {
}
private static _mapErrorTag(
- value: SpanAttributeValue | undefined
+ value: AttributeValue | undefined
): SpanStatusCode {
switch (value) {
case true:
diff --git a/packages/opentelemetry-shim-opentracing/test/Shim.test.ts b/packages/opentelemetry-shim-opentracing/test/Shim.test.ts
index 3c38bd98888..3ed62d4aad2 100644
--- a/packages/opentelemetry-shim-opentracing/test/Shim.test.ts
+++ b/packages/opentelemetry-shim-opentracing/test/Shim.test.ts
@@ -401,7 +401,7 @@ describe('OpenTracing Shim', () => {
span.log(kvLogs, tomorrow);
assert.strictEqual(otSpan.events[0].name, 'fun-time');
assert.strictEqual(otSpan.events[0].time[0], Math.trunc(tomorrow / 1000));
- assert.strictEqual(otSpan.events[0].attributes, kvLogs);
+ assert.deepStrictEqual(otSpan.events[0].attributes, kvLogs);
});
it('names event log, as a fallback', () => {
@@ -409,7 +409,7 @@ describe('OpenTracing Shim', () => {
span.log(kvLogs, tomorrow);
assert.strictEqual(otSpan.events[0].name, 'log');
assert.strictEqual(otSpan.events[0].time[0], Math.trunc(tomorrow / 1000));
- assert.strictEqual(otSpan.events[0].attributes, kvLogs);
+ assert.deepStrictEqual(otSpan.events[0].attributes, kvLogs);
});
it('records an exception', () => {
From cabbee64e23d308b8d4702d3a018d7b002b71261 Mon Sep 17 00:00:00 2001
From: legendecas <legendecas@gmail.com>
Date: Wed, 6 Apr 2022 22:27:11 +0800
Subject: [PATCH 2/3] fixup!
---
CHANGELOG.md | 2 +-
.../src/common/attributes.ts | 18 +++++++++++-------
2 files changed, 12 insertions(+), 8 deletions(-)
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 6349f7efab7..eac9adbc0e6 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -10,7 +10,7 @@ All notable changes to this project will be documented in this file.
### :bug: (Bug Fix)
-* [#2881](https://github.com/open-telemetry/opentelemetry-js/pull/2881) fix: sanitize attributes inputs ([@legendecas](https://githuc.com/legendecas))
+* fix: sanitize attributes inputs #2881 @legendecas
### :books: (Refine Doc)
diff --git a/packages/opentelemetry-core/src/common/attributes.ts b/packages/opentelemetry-core/src/common/attributes.ts
index 3eefcd1cfdb..8e39bb6c54b 100644
--- a/packages/opentelemetry-core/src/common/attributes.ts
+++ b/packages/opentelemetry-core/src/common/attributes.ts
@@ -13,7 +13,8 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
-import { AttributeValue, Attributes } from '@opentelemetry/api';
+
+import { diag, AttributeValue, Attributes } from '@opentelemetry/api';
export function sanitizeAttributes(attributes: unknown): Attributes {
const out: Attributes = {};
@@ -24,14 +25,17 @@ export function sanitizeAttributes(attributes: unknown): Attributes {
for (const [k, v] of Object.entries(attributes)) {
if (!isAttributeKey(k)) {
+ diag.warn(`Invalid attribute key: ${k}`);
continue;
}
- if (isAttributeValue(v)) {
- if (Array.isArray(v)) {
- out[k] = v.slice();
- } else {
- out[k] = v;
- }
+ if (!isAttributeValue(v)) {
+ diag.warn(`Invalid attribute value set for key: ${k}`);
+ continue;
+ }
+ if (Array.isArray(v)) {
+ out[k] = v.slice();
+ } else {
+ out[k] = v;
}
}
From fca32af8660f0836ac1587e9130e222bddf5f41e Mon Sep 17 00:00:00 2001
From: legendecas <legendecas@gmail.com>
Date: Mon, 11 Apr 2022 19:18:34 +0800
Subject: [PATCH 3/3] fixup!
---
.../opentelemetry-core/src/common/attributes.ts | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/packages/opentelemetry-core/src/common/attributes.ts b/packages/opentelemetry-core/src/common/attributes.ts
index 8e39bb6c54b..6e07557b2eb 100644
--- a/packages/opentelemetry-core/src/common/attributes.ts
+++ b/packages/opentelemetry-core/src/common/attributes.ts
@@ -23,19 +23,19 @@ export function sanitizeAttributes(attributes: unknown): Attributes {
return out;
}
- for (const [k, v] of Object.entries(attributes)) {
- if (!isAttributeKey(k)) {
- diag.warn(`Invalid attribute key: ${k}`);
+ for (const [key, val] of Object.entries(attributes)) {
+ if (!isAttributeKey(key)) {
+ diag.warn(`Invalid attribute key: ${key}`);
continue;
}
- if (!isAttributeValue(v)) {
- diag.warn(`Invalid attribute value set for key: ${k}`);
+ if (!isAttributeValue(val)) {
+ diag.warn(`Invalid attribute value set for key: ${key}`);
continue;
}
- if (Array.isArray(v)) {
- out[k] = v.slice();
+ if (Array.isArray(val)) {
+ out[key] = val.slice();
} else {
- out[k] = v;
+ out[key] = val;
}
}