Skip to content
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
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
2 changes: 1 addition & 1 deletion codecov.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ ignore:
- "**/karma.*.js"
- "**/.eslintrc.js"
- "**/*.config.*"
- "scripts/"
- "**/scripts/**"
comment:
layout: "header, changes, diff, files"
behavior: default
Expand Down
2 changes: 0 additions & 2 deletions eslint.config.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,6 @@ export default tseslint.config(
'**/protos/**',
'**/.tmp/**',
'docs/**',
// Generated files committed back to the tree.
'experimental/packages/configuration/src/generated/**',
Comment thread
maryliag marked this conversation as resolved.
// tsd-style negative type-check fixtures, intentionally outside tsconfig.
'experimental/packages/configuration/test/fixtures/types/**',
'experimental/packages/otlp-transformer/src/generated/**',
Expand Down
2 changes: 2 additions & 0 deletions experimental/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ For notes on migrating to 2.x / 0.200.x see [the upgrade guide](doc/upgrade-to-2

### :boom: Breaking Changes

* chore(configuration)!: bump config schema to v1.1.0; rename `without_scope_info` → `scope_info_enabled` and `without_target_info/development` → `target_info_enabled/development` on the Prometheus pull exporter (semantics inverted), rename `with_resource_constant_labels` → `resource_constant_labels`. Validate `file_format` per the configuration versioning spec: accept any minor version of major `1` (e.g. `1.0`, `1.1`), warn when the minor version is newer than supported, and reject other major versions. [#6781](https://github.com/open-telemetry/opentelemetry-js/pull/6781) @MikeGoldsmith

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Q: is this breaking now? It might not be.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Technically it is a breaking change for anyone who was using prometheus/development config out of the parsed model. However, given that model and the whole config package is experimental and moving from an rc to a stable release, maybe could remove the breaking change flag?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks.

Per the configuration repo's versioning doc, anything */development isn't considered a semver breaking change.


### :rocket: Features

### :bug: Bug Fixes
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ const typescript = require('typescript');
// Get latest version by running:
// git tag -l --sort=version:refname | grep -v -- - | tail -1
// in git@github.com:open-telemetry/opentelemetry-configuration.git
const CONFIG_VERSION = 'v1.0.0';
const CONFIG_VERSION = 'v1.1.0';

const TOP = path.resolve(__dirname, '..');
const SCHEMA_PATH = path.join(
Expand Down Expand Up @@ -136,7 +136,6 @@ const validatorJsWithHeader = [
'// Pre-compiled ajv validator for the OpenTelemetry configuration schema',
`// Generated from opentelemetry-configuration.git ${CONFIG_VERSION}`,
'// Run `npm run generate:config` to regenerate',
'// eslint-disable-next-line',
'',
validatorJs,
].join('\n');
Expand All @@ -149,7 +148,6 @@ console.log(`Written pre-compiled validator to ${VALIDATOR_JS_PATH}`);
// ajv as a transitive dependency just for the .d.ts file.
const validatorDts = [
licenseHeader,
'/* eslint-disable */',
'// AUTO-GENERATED — do not edit',
'// Pre-compiled ajv validator for the OpenTelemetry configuration schema',
`// Generated from opentelemetry-configuration.git ${CONFIG_VERSION}`,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -453,8 +453,8 @@ export function setMeterProvider(config: ConfigurationModel): void {
host:
getStringFromEnv('OTEL_EXPORTER_PROMETHEUS_HOST') ?? 'localhost',
port: getNumberFromEnv('OTEL_EXPORTER_PROMETHEUS_PORT') ?? 9464,
without_scope_info: false,
'without_target_info/development': false,
scope_info_enabled: true,
'target_info_enabled/development': true,
},
},
};
Expand Down
48 changes: 41 additions & 7 deletions experimental/packages/configuration/src/FileConfigFactory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
* SPDX-License-Identifier: Apache-2.0
*/

import { diag } from '@opentelemetry/api';
import { getStringFromEnv } from '@opentelemetry/core';
import * as fs from 'fs';
import * as yaml from 'yaml';
Expand Down Expand Up @@ -31,21 +32,54 @@ export class FileConfigFactory implements ConfigFactory {
}
}

// The schema version this SDK targets. Per the configuration versioning spec,
// an implementation MUST reject a config file whose MAJOR version differs (a
// breaking-change boundary), accepts any MINOR version within that major, and
// SHOULD warn when the MINOR version is newer than it targets since unknown
// settings may be ignored. See
// https://github.com/open-telemetry/opentelemetry-configuration/blob/main/VERSIONING.md
const SUPPORTED_FILE_FORMAT_MAJOR = 1;
const SUPPORTED_FILE_FORMAT_MINOR = 1;

function validateFileFormat(configFile: string, fileFormat: unknown): void {
if (typeof fileFormat !== 'string' || fileFormat.length === 0) {
throw new Error(
`${configFile}: Unsupported file_format: "${fileFormat}". Expected a "MAJOR.MINOR" version string.`
);
}

// Drop any pre-release / meta tag, e.g. "1.0-rc.2" -> "1.0".
const parts = fileFormat.split('-', 1)[0].split('.');
const major = Number(parts[0]);
const minor = parts.length > 1 ? Number(parts[1]) : 0;
if (!Number.isInteger(major) || !Number.isInteger(minor)) {
throw new Error(
`${configFile}: Unsupported file_format: "${fileFormat}". Expected "MAJOR.MINOR" version numbers.`
);
}

if (major !== SUPPORTED_FILE_FORMAT_MAJOR) {
throw new Error(
`${configFile}: Unsupported file_format: "${fileFormat}". This SDK supports schema version ${SUPPORTED_FILE_FORMAT_MAJOR}.x.`
);
}

if (minor > SUPPORTED_FILE_FORMAT_MINOR) {
diag.warn(
`Configuration file_format "${fileFormat}" has a newer minor version than this SDK supports (${SUPPORTED_FILE_FORMAT_MAJOR}.${SUPPORTED_FILE_FORMAT_MINOR}); some settings may be ignored.`
Comment thread
MikeGoldsmith marked this conversation as resolved.
Outdated
);
}
}

export function parseConfigFile(): ConfigurationModel {
const supportedFileVersionPattern = /^1\.0$/;
const configFile = getStringFromEnv('OTEL_CONFIG_FILE') || '';
const file = fs.readFileSync(configFile, 'utf8');

const doc = yaml.parseDocument(file, { version: '1.2' });
substituteEnvVars(doc);
const processed = doc.toJS() as Record<string, unknown>;

const fileFormat = processed?.file_format;
if (!fileFormat || !supportedFileVersionPattern.test(String(fileFormat))) {
throw new Error(
`${configFile}: Unsupported file_format: "${fileFormat}". Must match ${supportedFileVersionPattern}.`
);
}
validateFileFormat(configFile, processed?.file_format);
Comment thread
trentm marked this conversation as resolved.

const valid = validateConfig(processed);
if (!valid) {
Expand Down
60 changes: 44 additions & 16 deletions experimental/packages/configuration/src/generated/types.ts
Comment thread
MikeGoldsmith marked this conversation as resolved.
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
*/
//
// AUTO-GENERATED — do not edit
// Generated from opentelemetry-configuration.git v1.0.0
// Generated from opentelemetry-configuration.git v1.1.0
// Run `npm run generate:config` to regenerate
//
/* eslint-disable @typescript-eslint/no-explicit-any, @typescript-eslint/no-empty-object-type */
Expand Down Expand Up @@ -231,6 +231,12 @@ export type ExperimentalOtlpFileExporter = {
*/
export type ConsoleExporter = {} | null;

/**
* Configure an event to span event bridge log record processor.
* If omitted, ignore.
*/
export type ExperimentalEventToSpanEventBridgeLogRecordProcessor = {} | null;

/**
* Configure exporter to be OTLP with HTTP transport.
* If omitted, ignore.
Expand Down Expand Up @@ -383,16 +389,16 @@ export type ExperimentalPrometheusMetricExporter = {
*/
port?: number | null;
/**
* Configure Prometheus Exporter to produce metrics without scope labels.
* If omitted or null, false is used.
* Configure Prometheus Exporter to produce metrics with scope labels.
* If omitted or null, true is used.
*/
without_scope_info?: boolean | null;
scope_info_enabled?: boolean | null;
/**
* Configure Prometheus Exporter to produce metrics without a target info metric for the resource.
* If omitted or null, false is used.
* Configure Prometheus Exporter to produce metrics with a target info metric for the resource.
* If omitted or null, true is used.
*/
'without_target_info/development'?: boolean | null;
with_resource_constant_labels?: IncludeExclude;
'target_info_enabled/development'?: boolean | null;
resource_constant_labels?: IncludeExclude;
translation_strategy?: ExperimentalPrometheusTranslationStrategy;
} | null;

Expand Down Expand Up @@ -652,6 +658,12 @@ export type TraceIdRatioBasedSampler = {
ratio?: number | null;
} | null;

/**
* Configure the ID generator to randomly generate TraceIds and SpanIds (spec default).
* If omitted, ignore.
*/
export type RandomIdGenerator = {} | null;

/**
* The attribute type.
* Values include:
Expand Down Expand Up @@ -766,6 +778,7 @@ export interface LoggerProvider {
export interface LogRecordProcessor {
batch?: BatchLogRecordProcessor;
simple?: SimpleLogRecordProcessor;
'event_to_span_event_bridge/development'?: ExperimentalEventToSpanEventBridgeLogRecordProcessor;
[k: string]: object | null | undefined;
}

Expand Down Expand Up @@ -887,7 +900,7 @@ export interface ExperimentalLoggerConfig {

export interface ExperimentalLoggerMatcherAndConfig {
/**
* Configure logger names to match, evaluated as follows:
* Configure logger names to match. Matching is case-sensitive, evaluated as follows:
*
* * If the logger name exactly matches.
* * If the logger name matches the wildcard pattern, where '?' matches any single character and '*' matches any number of characters including none.
Expand Down Expand Up @@ -943,6 +956,11 @@ export interface PeriodicMetricReader {
* If omitted or null, 30000 is used.
*/
timeout?: number | null;
/**
* Configure maximum export batch size.
* If omitted or null, no limit is used.
*/
'max_export_batch_size/development'?: number | null;
exporter: PushMetricExporter;
/**
* Configure metric producers.
Expand Down Expand Up @@ -1051,7 +1069,7 @@ export interface PullMetricExporter {
export interface IncludeExclude {
/**
* Configure list of value patterns to include.
* Values are evaluated to match as follows:
* Matching is case-sensitive. Values are evaluated to match as follows:
* * If the value exactly matches.
* * If the value matches the wildcard pattern, where '?' matches any single character and '*' matches any number of characters including none.
* If omitted, all values are included.
Expand All @@ -1061,7 +1079,7 @@ export interface IncludeExclude {
included?: string[];
/**
* Configure list of value patterns to exclude. Applies after .included (i.e. excluded has higher priority than included).
* Values are evaluated to match as follows:
* Matching is case-sensitive. Values are evaluated to match as follows:
* * If the value exactly matches.
* * If the value matches the wildcard pattern, where '?' matches any single character and '*' matches any number of characters including none.
* If omitted, .included attributes are included.
Expand Down Expand Up @@ -1176,7 +1194,7 @@ export interface ExperimentalMeterConfig {

export interface ExperimentalMeterMatcherAndConfig {
/**
* Configure meter names to match, evaluated as follows:
* Configure meter names to match. Matching is case-sensitive, evaluated as follows:
*
* * If the meter name exactly matches.
* * If the meter name matches the wildcard pattern, where '?' matches any single character and '*' matches any number of characters including none.
Expand Down Expand Up @@ -1230,6 +1248,7 @@ export interface TracerProvider {
processors: SpanProcessor[];
limits?: SpanLimits;
sampler?: Sampler;
id_generator?: IdGenerator;
'tracer_configurator/development'?: ExperimentalTracerConfigurator;
}

Expand Down Expand Up @@ -1437,7 +1456,7 @@ export interface ExperimentalComposableRuleBasedSamplerRuleAttributePatterns {
key: string;
/**
* Configure list of value patterns to include.
* Values are evaluated to match as follows:
* Matching is case-sensitive. Values are evaluated to match as follows:
* * If the value exactly matches.
* * If the value matches the wildcard pattern, where '?' matches any single character and '*' matches any number of characters including none.
* If omitted, all values are included.
Expand All @@ -1447,7 +1466,7 @@ export interface ExperimentalComposableRuleBasedSamplerRuleAttributePatterns {
included?: string[];
/**
* Configure list of value patterns to exclude. Applies after .included (i.e. excluded has higher priority than included).
* Values are evaluated to match as follows:
* Matching is case-sensitive. Values are evaluated to match as follows:
* * If the value exactly matches.
* * If the value matches the wildcard pattern, where '?' matches any single character and '*' matches any number of characters including none.
* If omitted, .included attributes are included.
Expand All @@ -1457,6 +1476,15 @@ export interface ExperimentalComposableRuleBasedSamplerRuleAttributePatterns {
excluded?: string[];
}

/**
* Configure the trace and span ID generator.
* If omitted, RandomIdGenerator is used.
*/
export interface IdGenerator {
random?: RandomIdGenerator;
[k: string]: object | null | undefined;
}

/**
* Configure tracers.
* If omitted, all tracers use default values as described in ExperimentalTracerConfig.
Expand Down Expand Up @@ -1486,7 +1514,7 @@ export interface ExperimentalTracerConfig {

export interface ExperimentalTracerMatcherAndConfig {
/**
* Configure tracer names to match, evaluated as follows:
* Configure tracer names to match. Matching is case-sensitive, evaluated as follows:
*
* * If the tracer name exactly matches.
* * If the tracer name matches the wildcard pattern, where '?' matches any single character and '*' matches any number of characters including none.
Expand Down Expand Up @@ -1531,7 +1559,7 @@ export interface AttributeNameValue {
/**
* The attribute value.
* The type of value must match .type.
* Property is required and must be non-null.
* Property must be present, but if null the entry is ignored.
*/
value: string | number | boolean | null | string[] | boolean[] | number[];
type?: AttributeType;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,9 @@
* SPDX-License-Identifier: Apache-2.0
*/

/* eslint-disable */
// AUTO-GENERATED — do not edit
// Pre-compiled ajv validator for the OpenTelemetry configuration schema
// Generated from opentelemetry-configuration.git v1.0.0
// Generated from opentelemetry-configuration.git v1.1.0
// Run `npm run generate:config` to regenerate

/** Minimal subset of ajv ErrorObject used by FileConfigFactory */
Expand Down

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -839,8 +839,8 @@ describe('EnvironmentConfigFactory', function () {
'prometheus/development': {
host: 'localhost',
port: 9464,
without_scope_info: false,
'without_target_info/development': false,
scope_info_enabled: true,
'target_info_enabled/development': true,
},
},
},
Expand Down Expand Up @@ -869,8 +869,8 @@ describe('EnvironmentConfigFactory', function () {
'prometheus/development': {
host: '0.0.0.0',
port: 8080,
without_scope_info: false,
'without_target_info/development': false,
scope_info_enabled: true,
'target_info_enabled/development': true,
},
},
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

import * as assert from 'assert';
import * as Sinon from 'sinon';
import { diag } from '@opentelemetry/api';
import type { ConfigurationModel } from '../src';
import { createConfigFactory } from '../src/ConfigFactory';
import { parseConfigFile } from '../src/FileConfigFactory';
Expand Down Expand Up @@ -152,9 +153,9 @@ const ksCardinality = {
const ksPromExporter = (strategy: string) => ({
host: 'localhost',
port: 9464,
without_scope_info: false,
'without_target_info/development': false,
with_resource_constant_labels: {
scope_info_enabled: true,
'target_info_enabled/development': true,
resource_constant_labels: {
included: ['service*'],
excluded: ['service.attr1'],
},
Expand Down Expand Up @@ -836,6 +837,33 @@ describe('FileConfigFactory', function () {
assert.throws(() => createConfigFactory(), /Unsupported file_format/);
});

it('should accept file_format 1.0 for backward compatibility', function () {
process.env.OTEL_CONFIG_FILE = 'test/fixtures/file-format-1.0.yaml';
assert.doesNotThrow(() => createConfigFactory());
});

it('should accept file_format 1.1', function () {
process.env.OTEL_CONFIG_FILE = 'test/fixtures/short-config.yml';
assert.doesNotThrow(() => createConfigFactory());
});

it('should accept a newer minor file_format with a warning', function () {
const warnStub = Sinon.stub(diag, 'warn');
process.env.OTEL_CONFIG_FILE =
'test/fixtures/file-format-future-minor.yaml';
assert.doesNotThrow(() => createConfigFactory());
Sinon.assert.calledWith(warnStub, Sinon.match(/newer minor version/));
warnStub.restore();
});

it('should throw for an unsupported major file_format version', function () {
process.env.OTEL_CONFIG_FILE = 'test/fixtures/file-format-unsupported.yaml';
assert.throws(
() => createConfigFactory(),
/Unsupported file_format.*supports schema version 1\.x/
);
});

it('should show multiple validation errors for invalid config', function () {
process.env.OTEL_CONFIG_FILE = 'test/fixtures/invalid-multiple-errors.yaml';
assert.throws(
Expand Down
Loading