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-logs): hide internal methods with internal shared state #3865

Merged
merged 8 commits into from
Sep 26, 2023
Merged
Show file tree
Hide file tree
Changes from 4 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
1 change: 1 addition & 0 deletions experimental/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@ All notable changes to experimental packages in this project will be documented
* chore(instrumentation-grpc): add 'grpc' deprecation notice postinstall script [#3833](https://github.com/open-telemetry/opentelemetry-js/pull/3833) @pichlermarc
* Support for telemetry generation for the [`grpc`](https://www.npmjs.com/package/grpc) module will be dropped in the next release as the package has been
deprecated for over 1 year, please migrate to [`@grpc/grpc-js`](https://www.npmjs.com/package/@grpc/grpc-js) to continue receiving telemetry.
* fix(sdk-logs): hide internal methods with internal shared state [#3865](https://github.com/open-telemetry/opentelemetry-js/pull/3865) @legendecas
pichlermarc marked this conversation as resolved.
Show resolved Hide resolved

### :rocket: (Enhancement)

Expand Down
23 changes: 14 additions & 9 deletions experimental/packages/sdk-logs/src/LogRecord.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,8 @@ import type { IResource } from '@opentelemetry/resources';

import type { ReadableLogRecord } from './export/ReadableLogRecord';
import type { LogRecordLimits } from './types';
import { Logger } from './Logger';
import { LogAttributes } from '@opentelemetry/api-logs';
import { LoggerProviderSharedState } from './internal/LoggerProviderSharedState';

export class LogRecord implements ReadableLogRecord {
readonly hrTime: api.HrTime;
Expand All @@ -41,7 +41,7 @@ export class LogRecord implements ReadableLogRecord {
private _body?: string;

private _isReadonly: boolean = false;
private readonly _logRecordLimits: LogRecordLimits;
private readonly _logRecordLimits: Required<LogRecordLimits>;

set severityText(severityText: string | undefined) {
if (this._isLogRecordReadonly()) {
Expand Down Expand Up @@ -73,7 +73,11 @@ export class LogRecord implements ReadableLogRecord {
return this._body;
}

constructor(logger: Logger, logRecord: logsAPI.LogRecord) {
constructor(
_sharedState: LoggerProviderSharedState,
instrumentationScope: InstrumentationScope,
logRecord: logsAPI.LogRecord
) {
const {
timestamp,
observedTimestamp,
Expand All @@ -97,9 +101,9 @@ export class LogRecord implements ReadableLogRecord {
this.severityNumber = severityNumber;
this.severityText = severityText;
this.body = body;
this.resource = logger.resource;
this.instrumentationScope = logger.instrumentationScope;
this._logRecordLimits = logger.getLogRecordLimits();
this.resource = _sharedState.resource;
this.instrumentationScope = instrumentationScope;
this._logRecordLimits = _sharedState.logRecordLimits;
this.setAttributes(attributes);
}

Expand Down Expand Up @@ -127,7 +131,7 @@ export class LogRecord implements ReadableLogRecord {
}
if (
Object.keys(this.attributes).length >=
this._logRecordLimits.attributeCountLimit! &&
this._logRecordLimits.attributeCountLimit &&
!Object.prototype.hasOwnProperty.call(this.attributes, key)
) {
return this;
Expand Down Expand Up @@ -159,15 +163,16 @@ export class LogRecord implements ReadableLogRecord {
}

/**
* @internal
* A LogRecordProcessor may freely modify logRecord for the duration of the OnEmit call.
* If logRecord is needed after OnEmit returns (i.e. for asynchronous processing) only reads are permitted.
*/
public makeReadonly() {
_makeReadonly() {
this._isReadonly = true;
}

private _truncateToSize(value: AttributeValue): AttributeValue {
const limit = this._logRecordLimits.attributeValueLengthLimit || 0;
const limit = this._logRecordLimits.attributeValueLengthLimit;
// Check limit
if (limit <= 0) {
// Negative values are invalid, so do not truncate
Expand Down
44 changes: 13 additions & 31 deletions experimental/packages/sdk-logs/src/Logger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,28 +15,17 @@
*/

import type * as logsAPI from '@opentelemetry/api-logs';
import type { IResource } from '@opentelemetry/resources';
import type { InstrumentationScope } from '@opentelemetry/core';
import { context } from '@opentelemetry/api';

import type { LoggerConfig, LogRecordLimits } from './types';
import { LogRecord } from './LogRecord';
import { LoggerProvider } from './LoggerProvider';
import { mergeConfig } from './config';
import { LogRecordProcessor } from './LogRecordProcessor';
import { LoggerProviderSharedState } from './internal/LoggerProviderSharedState';

export class Logger implements logsAPI.Logger {
public readonly resource: IResource;
private readonly _loggerConfig: Required<LoggerConfig>;

constructor(
public readonly instrumentationScope: InstrumentationScope,
config: LoggerConfig,
private _loggerProvider: LoggerProvider
) {
this._loggerConfig = mergeConfig(config);
this.resource = _loggerProvider.resource;
}
private _sharedState: LoggerProviderSharedState
) {}

public emit(logRecord: logsAPI.LogRecord): void {
const currentContext = logRecord.context || context.active();
Expand All @@ -45,30 +34,23 @@ export class Logger implements logsAPI.Logger {
* the LogRecords it emits MUST automatically include the Trace Context from the active Context,
* if Context has not been explicitly set.
*/
const logRecordInstance = new LogRecord(this, {
context: currentContext,
...logRecord,
});
const logRecordInstance = new LogRecord(
this._sharedState,
this.instrumentationScope,
{
context: currentContext,
...logRecord,
}
);
/**
* the explicitly passed Context,
* the current Context, or an empty Context if the Logger was obtained with include_trace_context=false
*/
this.getActiveLogRecordProcessor().onEmit(
logRecordInstance,
currentContext
);
this._sharedState.activeProcessor.onEmit(logRecordInstance, currentContext);
/**
* A LogRecordProcessor may freely modify logRecord for the duration of the OnEmit call.
* If logRecord is needed after OnEmit returns (i.e. for asynchronous processing) only reads are permitted.
*/
logRecordInstance.makeReadonly();
}

public getLogRecordLimits(): LogRecordLimits {
return this._loggerConfig.logRecordLimits;
}

public getActiveLogRecordProcessor(): LogRecordProcessor {
return this._loggerProvider.getActiveLogRecordProcessor();
logRecordInstance._makeReadonly();
}
}
67 changes: 22 additions & 45 deletions experimental/packages/sdk-logs/src/LoggerProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,47 +16,34 @@
import { diag } from '@opentelemetry/api';
import type * as logsAPI from '@opentelemetry/api-logs';
import { NOOP_LOGGER } from '@opentelemetry/api-logs';
import { IResource, Resource } from '@opentelemetry/resources';
import { Resource } from '@opentelemetry/resources';
import { BindOnceFuture, merge } from '@opentelemetry/core';

import type { LoggerProviderConfig } from './types';
import type { LogRecordProcessor } from './LogRecordProcessor';
import { Logger } from './Logger';
import { loadDefaultConfig, reconfigureLimits } from './config';
import { MultiLogRecordProcessor } from './MultiLogRecordProcessor';
import { NoopLogRecordProcessor } from './export/NoopLogRecordProcessor';
import { LoggerProviderSharedState } from './internal/LoggerProviderSharedState';

export const DEFAULT_LOGGER_NAME = 'unknown';

export class LoggerProvider implements logsAPI.LoggerProvider {
public readonly resource: IResource;

private readonly _loggers: Map<string, Logger> = new Map();
private _activeProcessor: MultiLogRecordProcessor;
private readonly _registeredLogRecordProcessors: LogRecordProcessor[] = [];
private readonly _config: LoggerProviderConfig;
private _shutdownOnce: BindOnceFuture<void>;
private _sharedState: LoggerProviderSharedState;
pichlermarc marked this conversation as resolved.
Show resolved Hide resolved

constructor(config: LoggerProviderConfig = {}) {
const {
resource = Resource.empty(),
resource = Resource.default(),
logRecordLimits,
forceFlushTimeoutMillis,
} = merge({}, loadDefaultConfig(), reconfigureLimits(config));
this.resource = Resource.default().merge(resource);
this._config = {
logRecordLimits,
resource: this.resource,
} = merge({}, loadDefaultConfig(), config);
this._sharedState = new LoggerProviderSharedState(
resource,
forceFlushTimeoutMillis,
};

this._shutdownOnce = new BindOnceFuture(this._shutdown, this);

// add a default processor: NoopLogRecordProcessor
this._activeProcessor = new MultiLogRecordProcessor(
[new NoopLogRecordProcessor()],
forceFlushTimeoutMillis
reconfigureLimits(logRecordLimits)
);
this._shutdownOnce = new BindOnceFuture(this._shutdown, this);
}

/**
Expand All @@ -77,30 +64,28 @@ export class LoggerProvider implements logsAPI.LoggerProvider {
}
const loggerName = name || DEFAULT_LOGGER_NAME;
const key = `${loggerName}@${version || ''}:${options?.schemaUrl || ''}`;
if (!this._loggers.has(key)) {
this._loggers.set(
if (!this._sharedState.loggers.has(key)) {
this._sharedState.loggers.set(
key,
new Logger(
{ name: loggerName, version, schemaUrl: options?.schemaUrl },
{
logRecordLimits: this._config.logRecordLimits,
},
this
this._sharedState
)
);
}
return this._loggers.get(key)!;
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
return this._sharedState.loggers.get(key)!;
}

/**
* Adds a new {@link LogRecordProcessor} to this logger.
* @param processor the new LogRecordProcessor to be added.
*/
public addLogRecordProcessor(processor: LogRecordProcessor) {
if (this._registeredLogRecordProcessors.length === 0) {
if (this._sharedState.registeredLogRecordProcessors.length === 0) {
// since we might have enabled by default a batchProcessor, we disable it
// before adding the new one
this._activeProcessor
this._sharedState.activeProcessor
.shutdown()
.catch(err =>
diag.error(
Expand All @@ -109,10 +94,10 @@ export class LoggerProvider implements logsAPI.LoggerProvider {
)
);
}
this._registeredLogRecordProcessors.push(processor);
this._activeProcessor = new MultiLogRecordProcessor(
this._registeredLogRecordProcessors,
this._config.forceFlushTimeoutMillis!
this._sharedState.registeredLogRecordProcessors.push(processor);
this._sharedState.activeProcessor = new MultiLogRecordProcessor(
this._sharedState.registeredLogRecordProcessors,
this._sharedState.forceFlushTimeoutMillis
);
}

Expand All @@ -127,7 +112,7 @@ export class LoggerProvider implements logsAPI.LoggerProvider {
diag.warn('invalid attempt to force flush after LoggerProvider shutdown');
return this._shutdownOnce.promise;
}
return this._activeProcessor.forceFlush();
return this._sharedState.activeProcessor.forceFlush();
}

/**
Expand All @@ -144,15 +129,7 @@ export class LoggerProvider implements logsAPI.LoggerProvider {
return this._shutdownOnce.call();
}

public getActiveLogRecordProcessor(): MultiLogRecordProcessor {
return this._activeProcessor;
}

public getActiveLoggers(): Map<string, Logger> {
return this._loggers;
}

private _shutdown(): Promise<void> {
return this._activeProcessor.shutdown();
return this._sharedState.activeProcessor.shutdown();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
*/

import { callWithTimeout } from '@opentelemetry/core';

import type { Context } from '@opentelemetry/api';
import type { LogRecordProcessor } from './LogRecordProcessor';
import type { LogRecord } from './LogRecord';

Expand All @@ -38,8 +38,10 @@ export class MultiLogRecordProcessor implements LogRecordProcessor {
);
}

public onEmit(logRecord: LogRecord): void {
this.processors.forEach(processors => processors.onEmit(logRecord));
public onEmit(logRecord: LogRecord, context?: Context): void {
this.processors.forEach(processors =>
processors.onEmit(logRecord, context)
);
}

public async shutdown(): Promise<void> {
Expand Down
67 changes: 23 additions & 44 deletions experimental/packages/sdk-logs/src/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import {
getEnv,
getEnvWithoutDefaults,
} from '@opentelemetry/core';
import { LoggerConfig } from './types';
import { LogRecordLimits } from './types';

export function loadDefaultConfig() {
return {
Expand All @@ -37,50 +37,29 @@ export function loadDefaultConfig() {
/**
* When general limits are provided and model specific limits are not,
* configures the model specific limits by using the values from the general ones.
* @param userConfig User provided tracer configuration
* @param logRecordLimits User provided tracer configuration
pichlermarc marked this conversation as resolved.
Show resolved Hide resolved
*/
export function reconfigureLimits(userConfig: LoggerConfig): LoggerConfig {
const logRecordLimits = Object.assign({}, userConfig.logRecordLimits);

export function reconfigureLimits(
logRecordLimits: LogRecordLimits
): Required<LogRecordLimits> {
const parsedEnvConfig = getEnvWithoutDefaults();

/**
* Reassign log record attribute count limit to use first non null value defined by user or use default value
*/
logRecordLimits.attributeCountLimit =
userConfig.logRecordLimits?.attributeCountLimit ??
parsedEnvConfig.OTEL_LOGRECORD_ATTRIBUTE_COUNT_LIMIT ??
parsedEnvConfig.OTEL_ATTRIBUTE_COUNT_LIMIT ??
DEFAULT_ATTRIBUTE_COUNT_LIMIT;

/**
* Reassign log record attribute value length limit to use first non null value defined by user or use default value
*/
logRecordLimits.attributeValueLengthLimit =
userConfig.logRecordLimits?.attributeValueLengthLimit ??
parsedEnvConfig.OTEL_LOGRECORD_ATTRIBUTE_VALUE_LENGTH_LIMIT ??
parsedEnvConfig.OTEL_ATTRIBUTE_VALUE_LENGTH_LIMIT ??
DEFAULT_ATTRIBUTE_VALUE_LENGTH_LIMIT;

return Object.assign({}, userConfig, { logRecordLimits });
}

/**
* Function to merge Default configuration (as specified in './config') with
* user provided configurations.
*/
export function mergeConfig(userConfig: LoggerConfig): Required<LoggerConfig> {
const DEFAULT_CONFIG = loadDefaultConfig();

const target = Object.assign({}, DEFAULT_CONFIG, userConfig);

target.logRecordLimits = Object.assign(
{},
DEFAULT_CONFIG.logRecordLimits,
userConfig.logRecordLimits || {}
);

return target;
return {
/**
* Reassign log record attribute count limit to use first non null value defined by user or use default value
*/
attributeCountLimit:
logRecordLimits.attributeCountLimit ??
parsedEnvConfig.OTEL_LOGRECORD_ATTRIBUTE_COUNT_LIMIT ??
parsedEnvConfig.OTEL_ATTRIBUTE_COUNT_LIMIT ??
DEFAULT_ATTRIBUTE_COUNT_LIMIT,
/**
* Reassign log record attribute value length limit to use first non null value defined by user or use default value
*/
attributeValueLengthLimit:
logRecordLimits.attributeValueLengthLimit ??
parsedEnvConfig.OTEL_LOGRECORD_ATTRIBUTE_VALUE_LENGTH_LIMIT ??
parsedEnvConfig.OTEL_ATTRIBUTE_VALUE_LENGTH_LIMIT ??
DEFAULT_ATTRIBUTE_VALUE_LENGTH_LIMIT,
};
}

export const DEFAULT_EVENT_DOMAIN = 'default';
Loading