Skip to content

Commit

Permalink
fix(sdk-logs): hide internal methods with internal shared state (#3865)
Browse files Browse the repository at this point in the history 
Co-authored-by: Marc Pichler <[email protected]>
  • Loading branch information
@legendecas @pichlermarc
legendecas and pichlermarc authored Sep 26, 2023
1 parent f2fc0d8 commit 2b9832e
Show file tree
Hide file tree
Showing 15 changed files with 246 additions and 274 deletions.
1 change: 1 addition & 0 deletions experimental/CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ All notable changes to experimental packages in this project will be documented
* fix(sdk-node): remove explicit dependency on @opentelemetry/exporter-jaeger
* '@opentelemetry/exporter-jaeger' is no longer be a dependency of this package. To continue using '@opentelemetry/exporter-jaeger', please install it manually.
* NOTE: `@opentelemetry/exporter-jaeger` is deprecated, consider switching to one of the alternatives described [here](https://www.npmjs.com/package/@opentelemetry/exporter-jaeger)
* fix(sdk-logs): hide internal methods with internal shared state [#3865](https://github.com/open-telemetry/opentelemetry-js/pull/3865) @legendecas

### :rocket: (Enhancement)

Expand Down
23 changes: 14 additions & 9 deletions experimental/packages/sdk-logs/src/LogRecord.ts
View file
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
View file
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
View file
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 readonly _sharedState: LoggerProviderSharedState;

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();
}
}
8 changes: 5 additions & 3 deletions experimental/packages/sdk-logs/src/MultiLogRecordProcessor.ts
View file
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
View file
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 limits configuration
*/
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

0 comments on commit 2b9832e

Please sign in to comment.