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

feat(diag-logger): replace logger with diag logger #1925

Merged
merged 5 commits into from
Feb 17, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
Prev Previous commit
Next Next commit
fix: additional breaking changes and removal of config initialization
  • Loading branch information
MSNev committed Feb 17, 2021
commit f191b10e7941d34a4dcc0d36bc795aa1b2e917a9
105 changes: 40 additions & 65 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,10 @@ To request automatic tracing support for a module not on this list, please [file

The new global [```api.diag```](https://github.com/open-telemetry/opentelemetry-js/blob/main/packages/opentelemetry-api/src/api/diag.ts#L32) not only provides the ability to set a global diagnostic logger ```setLogger()``` and logging level ```setLogLevel()```, it is also a ```DiagLogger``` implementation and can be used directly to log diagnostic messages.

And the helper functions will fallback to this global DiagLogger when no logger is provided or configured.
All included logger references have been removed in preference to using the global ```api.diag``` directly, so you no longer need to pass around the logger instance via
function parameters or included as part of the configuration for a component.

- **Using and setting the global diagnostic logger**

```javascript
// Previously (Not supported)
Expand All @@ -277,68 +280,22 @@ import { diag, DiagLogLevel } from "@opentelemetry/api";
diag.setLogLevel(DiagLogLevel.ERROR);
```

- **Common helper for passing and fetching a diagnostic logger via optional configuration**

```javascript
// Previously
import { Logger } from "@opentelemetry/api";
interface MyConfig {
logger: Logger
}

// and usage
import { NoopLogger } from "@opentelemetry/api";
export function myFunction(config: MyConfig) {
let logger = config.logger || new NoopLogger();
}

// Now
import { DiagLoggerConfig } from "@opentelemetry/api";
interface MyConfig extends DiagLoggerConfig {
// DiagLoggerConfig defines and optional DiagLogger and DiagLogLevel
// So you can specify either or both and no need to define within
// your own configuration.
}

// And usage
import { getDiagLoggerFromConfig } from "@opentelemetry/api";
export function myFunction(config?: MyConfig) {
// Config can be optional and is handled by the helper
// Will default to api.diag instance if no logger is defined
// and any api.diag logging level will also obeyed
let logger = getDiagLoggerFromConfig(config);
}

// or
import { getDiagLoggerFromConfig, DiagConsoleLogger } from "@opentelemetry/api";
export function myFunction(config?: MyConfig) {
// Defaults to a Console logger rather than the api.diag if no logger is defined // Will use any defined the logging level - defaults to ALL!
let logger = getDiagLoggerFromConfig(config, () => new DiagConsoleLogger());
}

// if you want to fallback to the default api.diag logger and IGNORE the global
// logging level you need to provide a fallback function, if no log level is
// in the config it would default to ALL (So use ONLY during development)
import { getDiagLoggerFromConfig, diag } from "@opentelemetry/api";
export function myFunction(config?: MyConfig) {
let logger = getDiagLoggerFromConfig(config, () => diag.getLogger());
}
```

#### Direct update path for existing code

Without refactoring your existing code to use ```api.diag``` or ```getDiagLoggerFromConfig``` (recommended path), below is how you would directly upgrade to the newer version so you can get compiling and running.
Without refactoring your existing code to use ```api.diag``` (recommended path), below is how you would directly upgrade to the newer version so you can get compiling and running.

- **api.Logger**

The api.Logger has been renamed for clarity to highlight that this is for internal(OpenTelemetry and supporting components) diagnostic level logging and not for usage by a consuming application to send telemetry out of the executing environment.
The api.Logger has been renamed for clarity to highlight that this is for internal (OpenTelemetry and supporting components) diagnostic level logging and not for usage by a consuming application to send telemetry out of the executing environment.

The new ```DiagLogger``` provides the same set of functions as the previous Logger interface plus a few more, so direct replacement of the Logger is a simple case of changing to the new (renamed) type which is also exported by api.
The new ```DiagLogger``` provides the same set of functions as the previous Logger interface plus an additional verbose level, so direct replacement of the Logger is a simple case of changing to the new (renamed) type which is also exported by api.

```typescript
// Previously
import { Logger } from "@opentelemetry/api";
export function(myLogger: Logger) {

// As above you should refactor any code like this to just use the api.diag global logger
export function MyFunction(myLogger: Logger) {
myLogger.debug("...");
myLogger.info("...");
myLogger.warn("...");
Expand All @@ -347,23 +304,31 @@ export function(myLogger: Logger) {

// Now
import { DiagLogger } from "@opentelemetry/api";
export function(myLogger: DiagLogger) {
export function MyFunction(myLogger: DiagLogger) {
myLogger.debug("...");
myLogger.info("...");
myLogger.warn("...");
myLogger.error("...");

// New levels
myLogger.verbose("..");
myLogger.startup("..");
myLogger.critical("..");
myLogger.terminal("..");
}

// Recommended usage
import { diag } from "@opentelemetry/api";

// Remove or make optional the parameter and don't use it.
export function MyFunction() {
diag.debug("...");
diag.info("...");
diag.warn("...");
diag.error("...");
diag.verbose("..");
}

```

- **api.NoopLogger**

To support minification the NoopLogger class has been removed in favor of a factory method and generally direct usage should no longer be required as components should use the new ```api.diag``` or ```getDiagLoggerFromConfig```.
To support minification the NoopLogger class has been removed in favor of a factory method and generally direct usage should no longer be required as components should use the new ```api.diag```.

```typescript
// Previous direct creation
Expand All @@ -372,7 +337,12 @@ let myLogger = new NoopLogger();

// Now
import { createNoopDiagLogger } from "@opentelemetry/api";
let myLogger = createNoopDiagLogger();
let noopLogger = createNoopDiagLogger();

// Recommended usage -- avoid and just use the global diag instance
import { diag } from "@opentelemetry/api";
diag.debug("...");

```

- **core.LogLevel**
Expand Down Expand Up @@ -401,22 +371,27 @@ DiagLogLevel.INFO !== 2 // true
DiagLogLevel.DEBUG !== 3 // true
```

Usage of the ```getEnv().OTEL_LOG_LEVEL``` now returns a ```api.DiagLogLevel``` and not the removed ```core.LogLevel```, the environment setting continues to support conversion from ```string``` case-insensitive values of the new Enum equivalent and but explicitly does not support initializing via a numeric value (or string version i.e. "0").
Usage of the ```getEnv().OTEL_LOG_LEVEL``` now returns a ```api.DiagLogLevel```, the environment setting continues to support conversion from ```string``` case-insensitive values to the new Enum equivalent, but explicitly does not support initializing via a numeric value (or string version i.e. "30").

- **core.ConsoleLogger**

As with the ```core.LogLevel``` this has been moved to the ```api``` package and renamed to ```DiagConsoleLogger```.

Note: The previous version of the ```ConsoleLogger``` supported a LogLevel "limit", this functionality has been extracted into a helper wrapper sink ```createLogLevelDiagLogger()``` so it can be applied to any DiagLogger implementation.
Note: The previous version of the ```ConsoleLogger``` supported a LogLevel "limit", this functionality has been extracted into a helper wrapper sink ```createLogLevelDiagLogger()``` and you should not need to use it directly, but it can be applied to any DiagLogger implementation.

```javascript
// Previously
import { ConsoleLogger } from "@opentelemetry/core";
let myLogger = new ConsoleLogger(LogLevel.ERROR);

// Now
import { DiagConsoleLogger, createLogLevelDiagLogger } from "@opentelemetry/api";
let myLogger = createLogLevelDiagLogger(LogLevel.ERROR, new DiagConsoleLogger());
import { DiagLogLevel, DiagConsoleLogger, createLogLevelDiagLogger } from "@opentelemetry/api";
let myLogger = createLogLevelDiagLogger(DiagLogLevel.ERROR, new DiagConsoleLogger());

// Recommended approach
import { DiagLogLevel, DiagConsoleLogger, diag } from "@opentelemetry/api";
diag.setLogger(new DiagConsoleLogger());
diag.setLogLevel(DiagLogLevel.ERROR);
```

### 0.16.0 to 0.17.0
Expand Down
7 changes: 4 additions & 3 deletions benchmark/tracer.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@ const benchmark = require('./benchmark');
const opentelemetry = require('../packages/opentelemetry-api');
const { BasicTracerProvider, BatchSpanProcessor, InMemorySpanExporter, SimpleSpanProcessor } = require('../packages/opentelemetry-tracing');

const diagLogger = opentelemetry.createNoopDiagLogger();
// Clear any previous global logger
opentelemetry.diag.setLogger(null);
Copy link
Member

Choose a reason for hiding this comment

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

I guess opentelemetry.diag.setLogger() should also set noop logger ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Calling setLogger() with a "falsey" value will default to a noop implementation


const setups = [
{
Expand All @@ -13,7 +14,7 @@ const setups = [
},
{
name: 'BasicTracerProvider',
provider: new BasicTracerProvider({ diagLogger })
provider: new BasicTracerProvider()
},
{
name: 'BasicTracerProvider with SimpleSpanProcessor',
Expand Down Expand Up @@ -63,7 +64,7 @@ for (const setup of setups) {
suite.run({ async: false });
}
function getProvider(processor) {
const provider = new BasicTracerProvider({ diagLogger });
const provider = new BasicTracerProvider();
provider.addSpanProcessor(processor);
return provider;
}
Expand Down
2 changes: 1 addition & 1 deletion examples/collector-exporter-node/metrics.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,13 @@ const { CollectorMetricExporter } = require('@opentelemetry/exporter-collector')
// const { CollectorMetricExporter } = require('@opentelemetry/exporter-collector-proto');
const { MeterProvider } = require('@opentelemetry/metrics');

// Optional and only needed to see the internal diagnostic logging (during development)
diag.setLogger(new DiagConsoleLogger());
diag.setLogLevel(DiagLogLevel.DEBUG);

const metricExporter = new CollectorMetricExporter({
serviceName: 'basic-metric-service',
// url: 'http://localhost:55681/v1/metrics',
diagLogger: diag,
});

const meter = new MeterProvider({
Expand Down
7 changes: 5 additions & 2 deletions examples/metrics/metrics/observer.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,13 @@
'use strict';

const { MeterProvider } = require('@opentelemetry/metrics');
const { DiagConsoleLogger, DiagLogLevel, diagLogLevelFilter } = require('@opentelemetry/api');
const { DiagConsoleLogger, DiagLogLevel, diag } = require('@opentelemetry/api');
const { PrometheusExporter } = require('@opentelemetry/exporter-prometheus');

// Optional and only needed to see the internal diagnostic logging (during development)
diag.setLogger(new DiagConsoleLogger());
diag.setLogLevel(DiagLogLevel.DEBUG);

const exporter = new PrometheusExporter(
{
startServer: true,
Expand Down Expand Up @@ -61,7 +65,6 @@ meter.createBatchObserver((observerBatchResult) => {
});
}, {
maxTimeoutUpdateMS: 500,
diagLogger: diagLogLevelFilter(DiagLogLevel.DEBUG, new DiagConsoleLogger())
},
);

Expand Down
7 changes: 5 additions & 2 deletions examples/tracer-web/examples/metrics/index.js
Original file line number Diff line number Diff line change
@@ -1,12 +1,15 @@
'use strict';

const { DiagConsoleLogger, DiagLogLevel, diagLogLevelFilter } = require('@opentelemetry/api');
const { DiagConsoleLogger, DiagLogLevel, diag } = require('@opentelemetry/api');
const { CollectorMetricExporter } = require('@opentelemetry/exporter-collector');
const { MeterProvider } = require('@opentelemetry/metrics');

// Optional and only needed to see the internal diagnostic logging (during development)
diag.setLogger(new DiagConsoleLogger());
diag.setLogLevel(DiagLogLevel.DEBUG);

const metricExporter = new CollectorMetricExporter({
serviceName: 'basic-metric-service',
diagLogger: diagLogLevelFilter(DiagLogLevel.DEBUG, new DiagConsoleLogger()),
});

let interval;
Expand Down
5 changes: 2 additions & 3 deletions packages/opentelemetry-api-metrics/src/types/Metric.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,11 @@ import {
BoundCounter,
BoundValueRecorder,
} from './BoundInstrument';
import { DiagLoggerConfig } from '@opentelemetry/api';

/**
* Options needed for metric creation
*/
export interface MetricOptions extends DiagLoggerConfig {
export interface MetricOptions {
/** The name of the component that reports the Metric. */
component?: string;

Expand Down Expand Up @@ -61,7 +60,7 @@ export interface MetricOptions extends DiagLoggerConfig {
boundaries?: number[];
}

export interface BatchObserverOptions extends DiagLoggerConfig {
export interface BatchObserverOptions {
/**
* Indicates how long the batch metric should wait to update before cancel
*/
Expand Down
5 changes: 1 addition & 4 deletions packages/opentelemetry-api/src/api/diag.ts
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ export class DiagAPI implements DiagLogger {

self.setLogger = (logger: DiagLogger): DiagLogger => {
const prevLogger = _logger;
if (prevLogger !== logger && logger !== self) {
if (!logger || logger !== self) {
// Simple special case to avoid any possible infinite recursion on the logging functions
_logger = logger || createNoopDiagLogger();
_filteredLogger = createLogLevelDiagLogger(_logLevel, _logger);
Expand Down Expand Up @@ -146,8 +146,5 @@ export class DiagAPI implements DiagLogger {
public debug!: DiagLogFunction;
public info!: DiagLogFunction;
public warn!: DiagLogFunction;
public startupInfo!: DiagLogFunction;
public error!: DiagLogFunction;
public critical!: DiagLogFunction;
public terminal!: DiagLogFunction;
}
68 changes: 0 additions & 68 deletions packages/opentelemetry-api/src/diag/config.ts

This file was deleted.

Loading