Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(sdk-trace-base): fix spanLimits attribute length/count to consider env values #3068

Merged
Merged
Show file tree
Hide file tree
Changes from 21 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
229cc75
chore(sdk-trace-base): update span attribute length and count to cons…
svetlanabrennan Jun 28, 2022
98b96bc
chore(sdk-trace-base): fix envs loading before tests
svetlanabrennan Jun 29, 2022
9cd974b
chore(sdk-trace-base): remove duplicate process.env from used in brow…
svetlanabrennan Jun 29, 2022
51b0715
chore(sdk-trace-base): add changelog
svetlanabrennan Jun 29, 2022
51fb4b6
Merge branch 'main' into fix-attribute-length-issue
svetlanabrennan Jun 30, 2022
5b4e6d1
Merge branch 'main' into fix-attribute-length-issue
svetlanabrennan Jul 6, 2022
d3a6bb0
Update packages/opentelemetry-sdk-trace-base/test/common/BasicTracerP…
svetlanabrennan Jul 22, 2022
10ce6fc
Update packages/opentelemetry-sdk-trace-base/test/common/BasicTracerP…
svetlanabrennan Jul 22, 2022
41ccdba
chore(sdk-trace-base): add comments
svetlanabrennan Jul 22, 2022
65d4c24
Merge branch 'fix-attribute-length-issue' of github.com:svetlanabrenn…
svetlanabrennan Jul 22, 2022
9075be2
Update pr name in changelog
svetlanabrennan Jul 22, 2022
5cf481a
Merge branch 'main' into fix-attribute-length-issue
svetlanabrennan Jul 22, 2022
b42ac6a
fix(sdk-trace-base): refactor if statement to nullish check
svetlanabrennan Jul 25, 2022
49619ff
fix(sdk-trace-base): refactor if statement to nullish check
svetlanabrennan Jul 25, 2022
9336daf
fix(sdk-trace-base): add more test cases
svetlanabrennan Jul 25, 2022
c2fc454
Merge branch 'fix-attribute-length-issue' of github.com:svetlanabrenn…
svetlanabrennan Jul 25, 2022
1ff5be9
fix(sdk-trace-base): remove calling process in browser
svetlanabrennan Jul 25, 2022
705d27e
Merge branch 'main' into fix-attribute-length-issue
svetlanabrennan Jul 25, 2022
fc1e3e9
fix(sdk-trace-base): move parsing env to core package
svetlanabrennan Jul 26, 2022
2e77d0f
fix(sdk-trace-base): fix lint issue
svetlanabrennan Jul 26, 2022
771f1f4
Merge branch 'main' into fix-attribute-length-issue
svetlanabrennan Jul 26, 2022
aaec7cc
fix(sdk-trace-base): rename function
svetlanabrennan Jul 26, 2022
b959b72
Merge branch 'main' into fix-attribute-length-issue
svetlanabrennan Jul 27, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ All notable changes to this project will be documented in this file.

* fix(resources): fix browser compatibility for host and os detectors [#3004](https://github.com/open-telemetry/opentelemetry-js/pull/3004) @legendecas
* fix(sdk-trace-base): fix crash on environments without global document [#3000](https://github.com/open-telemetry/opentelemetry-js/pull/3000) @legendecas
* fix(sdk-trace-base): fix spanLimits attribute length/count to consider env values [#3068](https://github.com/open-telemetry/opentelemetry-js/pull/3068) @svetlanabrennan

### :house: (Internal)

Expand Down
11 changes: 11 additions & 0 deletions packages/opentelemetry-core/src/utils/environment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

import { DiagLogLevel } from '@opentelemetry/api';
import { TracesSamplerValues } from './sampling';
import { _globalThis } from '../platform/browser/globalThis';

const DEFAULT_LIST_SEPARATOR = ',';

Expand Down Expand Up @@ -283,3 +284,13 @@ export function parseEnvironment(values: RAW_ENVIRONMENT): ENVIRONMENT {

return environment;
}

/**
* Parse environment in node or browser without
* populating default values.
*/
export function parseEnvWithoutDefaults(): ENVIRONMENT {
svetlanabrennan marked this conversation as resolved.
Show resolved Hide resolved
return typeof process !== 'undefined' ?
parseEnvironment(process.env as RAW_ENVIRONMENT) :
parseEnvironment(_globalThis as typeof globalThis & RAW_ENVIRONMENT);
}
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ import {
} from '@opentelemetry/core';
import { Resource } from '@opentelemetry/resources';
import { SpanProcessor, Tracer } from '.';
import { DEFAULT_CONFIG } from './config';
import { loadDefaultConfig } from './config';
import { MultiSpanProcessor } from './MultiSpanProcessor';
import { NoopSpanProcessor } from './export/NoopSpanProcessor';
import { SDKRegistrationConfig, TracerConfig } from './types';
Expand Down Expand Up @@ -74,7 +74,7 @@ export class BasicTracerProvider implements TracerProvider {
readonly resource: Resource;

constructor(config: TracerConfig = {}) {
const mergedConfig = merge({}, DEFAULT_CONFIG, reconfigureLimits(config));
const mergedConfig = merge({}, loadDefaultConfig(), reconfigureLimits(config));
this.resource = mergedConfig.resource ?? Resource.empty();
this.resource = Resource.default().merge(this.resource);
this._config = Object.assign({}, mergedConfig, {
Expand Down
35 changes: 20 additions & 15 deletions packages/opentelemetry-sdk-trace-base/src/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,25 +30,30 @@ const FALLBACK_OTEL_TRACES_SAMPLER = TracesSamplerValues.AlwaysOn;
const DEFAULT_RATIO = 1;

/**
* Default configuration. For fields with primitive values, any user-provided
* Load default configuration. For fields with primitive values, any user-provided
* value will override the corresponding default value. For fields with
* non-primitive values (like `spanLimits`), the user-provided value will be
* used to extend the default value.
*/
export const DEFAULT_CONFIG = {
sampler: buildSamplerFromEnv(env),
forceFlushTimeoutMillis: 30000,
generalLimits: {
attributeValueLengthLimit: getEnv().OTEL_ATTRIBUTE_VALUE_LENGTH_LIMIT,
attributeCountLimit: getEnv().OTEL_ATTRIBUTE_COUNT_LIMIT,
},
spanLimits: {
attributeValueLengthLimit: getEnv().OTEL_SPAN_ATTRIBUTE_VALUE_LENGTH_LIMIT,
attributeCountLimit: getEnv().OTEL_SPAN_ATTRIBUTE_COUNT_LIMIT,
linkCountLimit: getEnv().OTEL_SPAN_LINK_COUNT_LIMIT,
eventCountLimit: getEnv().OTEL_SPAN_EVENT_COUNT_LIMIT,
},
};

// object needs to be wrapped in this function and called when needed otherwise
// envs are parsed before tests are ran - causes tests using these envs to fail
export function loadDefaultConfig() {
return {
sampler: buildSamplerFromEnv(env),
forceFlushTimeoutMillis: 30000,
generalLimits: {
attributeValueLengthLimit: getEnv().OTEL_ATTRIBUTE_VALUE_LENGTH_LIMIT,
attributeCountLimit: getEnv().OTEL_ATTRIBUTE_COUNT_LIMIT,
},
spanLimits: {
attributeValueLengthLimit: getEnv().OTEL_SPAN_ATTRIBUTE_VALUE_LENGTH_LIMIT,
attributeCountLimit: getEnv().OTEL_SPAN_ATTRIBUTE_COUNT_LIMIT,
linkCountLimit: getEnv().OTEL_SPAN_LINK_COUNT_LIMIT,
eventCountLimit: getEnv().OTEL_SPAN_EVENT_COUNT_LIMIT,
},
};
}

/**
* Based on environment, builds a sampler, complies with specification.
Expand Down
35 changes: 24 additions & 11 deletions packages/opentelemetry-sdk-trace-base/src/utility.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,13 @@
*/

import { Sampler } from '@opentelemetry/api';
import { buildSamplerFromEnv, DEFAULT_CONFIG } from './config';
import { buildSamplerFromEnv, loadDefaultConfig } from './config';
import { SpanLimits, TracerConfig, GeneralLimits } from './types';
import {
DEFAULT_ATTRIBUTE_COUNT_LIMIT,
DEFAULT_ATTRIBUTE_VALUE_LENGTH_LIMIT,
parseEnvWithoutDefaults,
} from '@opentelemetry/core';

/**
* Function to merge Default configuration (as specified in './config') with
Expand All @@ -31,6 +36,8 @@ export function mergeConfig(userConfig: TracerConfig): TracerConfig & {
sampler: buildSamplerFromEnv(),
};

const DEFAULT_CONFIG = loadDefaultConfig();

const target = Object.assign(
{},
DEFAULT_CONFIG,
Expand Down Expand Up @@ -61,21 +68,27 @@ export function mergeConfig(userConfig: TracerConfig): TracerConfig & {
export function reconfigureLimits(userConfig: TracerConfig): TracerConfig {
const spanLimits = Object.assign({}, userConfig.spanLimits);

const parsedEnvConfig = parseEnvWithoutDefaults();

/**
* When span attribute count limit is not defined, but general attribute count limit is defined
* Then, span attribute count limit will be same as general one
* Reassign span attribute count limit to use first non null value defined by user or use default value
*/
if (spanLimits.attributeCountLimit == null && userConfig.generalLimits?.attributeCountLimit != null) {
spanLimits.attributeCountLimit = userConfig.generalLimits.attributeCountLimit;
}
spanLimits.attributeCountLimit =
userConfig.spanLimits?.attributeCountLimit ??
userConfig.generalLimits?.attributeCountLimit ??
parsedEnvConfig.OTEL_SPAN_ATTRIBUTE_COUNT_LIMIT ??
parsedEnvConfig.OTEL_ATTRIBUTE_COUNT_LIMIT ??
DEFAULT_ATTRIBUTE_COUNT_LIMIT;

/**
* When span attribute value length limit is not defined, but general attribute value length limit is defined
* Then, span attribute value length limit will be same as general one
* Reassign span attribute value length limit to use first non null value defined by user or use default value
*/
if (spanLimits.attributeValueLengthLimit == null && userConfig.generalLimits?.attributeValueLengthLimit != null) {
spanLimits.attributeValueLengthLimit = userConfig.generalLimits.attributeValueLengthLimit;
}
spanLimits.attributeValueLengthLimit =
userConfig.spanLimits?.attributeValueLengthLimit ??
userConfig.generalLimits?.attributeValueLengthLimit ??
parsedEnvConfig.OTEL_SPAN_ATTRIBUTE_VALUE_LENGTH_LIMIT ??
parsedEnvConfig.OTEL_ATTRIBUTE_VALUE_LENGTH_LIMIT ??
DEFAULT_ATTRIBUTE_VALUE_LENGTH_LIMIT;

return Object.assign({}, userConfig, { spanLimits });
}
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,108 @@ describe('BasicTracerProvider', () => {
});
});

describe('when attribute value length limit is defined via env', () => {
it('should have general attribute value length limits value as defined with env', () => {
envSource.OTEL_ATTRIBUTE_VALUE_LENGTH_LIMIT = '115';
const tracer = new BasicTracerProvider().getTracer('default');
const generalLimits = tracer.getGeneralLimits();
assert.strictEqual(generalLimits.attributeValueLengthLimit, 115);
delete envSource.OTEL_ATTRIBUTE_VALUE_LENGTH_LIMIT;
});
it('should have span attribute value length limit value same as general limit value', () => {
envSource.OTEL_ATTRIBUTE_VALUE_LENGTH_LIMIT = '125';
const tracer = new BasicTracerProvider().getTracer('default');
const generalLimits = tracer.getGeneralLimits();
const spanLimits = tracer.getSpanLimits();
assert.strictEqual(generalLimits.attributeValueLengthLimit, 125);
assert.strictEqual(spanLimits.attributeValueLengthLimit, 125);
delete envSource.OTEL_ATTRIBUTE_VALUE_LENGTH_LIMIT;
});
it('should have span and general attribute value length limits as defined in env', () => {
envSource.OTEL_ATTRIBUTE_VALUE_LENGTH_LIMIT = '125';
envSource.OTEL_SPAN_ATTRIBUTE_VALUE_LENGTH_LIMIT = '109';
const tracer = new BasicTracerProvider().getTracer('default');
const spanLimits = tracer.getSpanLimits();
const generalLimits = tracer.getGeneralLimits();
assert.strictEqual(generalLimits.attributeValueLengthLimit, 125);
assert.strictEqual(spanLimits.attributeValueLengthLimit, 109);
delete envSource.OTEL_ATTRIBUTE_VALUE_LENGTH_LIMIT;
delete envSource.OTEL_SPAN_ATTRIBUTE_VALUE_LENGTH_LIMIT;
});
it('should have span attribute value length limit as deafult of Infinity', () => {
envSource.OTEL_ATTRIBUTE_VALUE_LENGTH_LIMIT = '125';
envSource.OTEL_SPAN_ATTRIBUTE_VALUE_LENGTH_LIMIT = 'Infinity';
const tracer = new BasicTracerProvider().getTracer('default');
const spanLimits = tracer.getSpanLimits();
const generalLimits = tracer.getGeneralLimits();
assert.strictEqual(generalLimits.attributeValueLengthLimit, 125);
assert.strictEqual(spanLimits.attributeValueLengthLimit, Infinity);
delete envSource.OTEL_ATTRIBUTE_VALUE_LENGTH_LIMIT;
delete envSource.OTEL_SPAN_ATTRIBUTE_VALUE_LENGTH_LIMIT;
});
});

describe('when attribute value length limit is not defined via env', () => {
it('should use default value of Infinity', () => {
const tracer = new BasicTracerProvider().getTracer('default');
const spanLimits = tracer.getSpanLimits();
const generalLimits = tracer.getGeneralLimits();
assert.strictEqual(generalLimits.attributeValueLengthLimit, Infinity);
assert.strictEqual(spanLimits.attributeValueLengthLimit, Infinity);
});
});

describe('when attribute count limit is defined via env', () => {
it('should general attribute count limit as defined with env', () => {
envSource.OTEL_ATTRIBUTE_COUNT_LIMIT = '25';
const tracer = new BasicTracerProvider({}).getTracer('default');
const generalLimits = tracer.getGeneralLimits();
assert.strictEqual(generalLimits.attributeCountLimit, 25);
delete envSource.OTEL_ATTRIBUTE_COUNT_LIMIT;
});
it('should have span attribute count limit value same as general limit value', () => {
envSource.OTEL_ATTRIBUTE_COUNT_LIMIT = '20';
const tracer = new BasicTracerProvider().getTracer('default');
const generalLimits = tracer.getGeneralLimits();
const spanLimits = tracer.getSpanLimits();
assert.strictEqual(generalLimits.attributeCountLimit, 20);
assert.strictEqual(spanLimits.attributeCountLimit, 20);
delete envSource.OTEL_ATTRIBUTE_COUNT_LIMIT;
});
it('should have span and general attribute count limits as defined in env', () => {
envSource.OTEL_ATTRIBUTE_COUNT_LIMIT = '20';
envSource.OTEL_SPAN_ATTRIBUTE_COUNT_LIMIT = '35';
const tracer = new BasicTracerProvider().getTracer('default');
const spanLimits = tracer.getSpanLimits();
const generalLimits = tracer.getGeneralLimits();
assert.strictEqual(generalLimits.attributeCountLimit, 20);
assert.strictEqual(spanLimits.attributeCountLimit, 35);
delete envSource.OTEL_ATTRIBUTE_COUNT_LIMIT;
delete envSource.OTEL_SPAN_ATTRIBUTE_COUNT_LIMIT;
});
it('should have span attribute count limit as default of 128', () => {
envSource.OTEL_ATTRIBUTE_COUNT_LIMIT = '20';
envSource.OTEL_SPAN_ATTRIBUTE_COUNT_LIMIT = '128';
const tracer = new BasicTracerProvider().getTracer('default');
const spanLimits = tracer.getSpanLimits();
const generalLimits = tracer.getGeneralLimits();
assert.strictEqual(generalLimits.attributeCountLimit, 20);
assert.strictEqual(spanLimits.attributeCountLimit, 128);
delete envSource.OTEL_ATTRIBUTE_COUNT_LIMIT;
delete envSource.OTEL_SPAN_ATTRIBUTE_COUNT_LIMIT;
});
});

describe('when attribute count limit is not defined via env', () => {
it('should use default value of 128', () => {
const tracer = new BasicTracerProvider().getTracer('default');
const spanLimits = tracer.getSpanLimits();
const generalLimits = tracer.getGeneralLimits();
assert.strictEqual(generalLimits.attributeCountLimit, 128);
assert.strictEqual(spanLimits.attributeCountLimit, 128);
});
});

describe('when "eventCountLimit" is defined', () => {
it('should have tracer with defined value', () => {
const tracer = new BasicTracerProvider({
Expand Down