Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
62 commits
Select commit Hold shift + click to select a range
638c36d
Add recordLog emit
hectorhdzg Nov 30, 2023
d461556
Merge branch 'main' into hectorhdzg/winstonlogs
hectorhdzg Nov 30, 2023
4ec2196
Readme lint
hectorhdzg Nov 30, 2023
20f58ab
Merge branch 'hectorhdzg/winstonlogs' of https://github.com/hectorhdz…
hectorhdzg Nov 30, 2023
01286b5
Update
hectorhdzg Dec 2, 2023
adc8f7a
Lint
hectorhdzg Dec 2, 2023
7d24291
Merge branch 'main' into hectorhdzg/winstonlogs
hectorhdzg Dec 2, 2023
fe2bdbb
Update
hectorhdzg Dec 8, 2023
1d4b91a
Merge branch 'main' into hectorhdzg/winstonlogs
hectorhdzg Dec 8, 2023
e76321b
Update
hectorhdzg Dec 8, 2023
d80c76a
Merge branch 'main' into hectorhdzg/winstonlogs
hectorhdzg Dec 8, 2023
683c578
Lint
hectorhdzg Dec 8, 2023
ffa9f9b
Tests
hectorhdzg Dec 8, 2023
441f72a
Added transports
hectorhdzg Dec 29, 2023
6731e97
Add log appenders(Winston transports)
hectorhdzg Jan 3, 2024
1c7ac82
Merge remote-tracking branch 'upstream/main' into hectorhdzg/winstonlogs
hectorhdzg Jan 3, 2024
c662cd0
Merge branch 'main' into hectorhdzg/winstonlogs
hectorhdzg Jan 3, 2024
47db601
npm i
hectorhdzg Jan 3, 2024
9d92764
Lint
hectorhdzg Jan 3, 2024
6595727
Merge branch 'main' into hectorhdzg/winstonlogs
hectorhdzg Jan 3, 2024
4c6e813
Merge branch 'main' into hectorhdzg/winstonlogs
hectorhdzg Jan 12, 2024
4dc6dca
Updating dependencies and readme
hectorhdzg Jan 23, 2024
ff81990
Merge branch 'main' into hectorhdzg/winstonlogs
hectorhdzg Jan 23, 2024
3800b1c
Update
hectorhdzg Jan 31, 2024
54b7156
Merge remote-tracking branch 'upstream/main' into hectorhdzg/winstonlogs
hectorhdzg Jan 31, 2024
8c730a5
npm i
hectorhdzg Jan 31, 2024
13fce4d
Update dependencies
hectorhdzg Jan 31, 2024
175a020
Update
hectorhdzg Feb 1, 2024
ca07f95
Update readme
hectorhdzg Feb 1, 2024
d4a84c4
Readme lint
hectorhdzg Feb 1, 2024
7399017
Merge branch 'main' into hectorhdzg/winstonlogs
hectorhdzg Feb 6, 2024
03b8ce7
Update
hectorhdzg Feb 6, 2024
2d7704c
lint
hectorhdzg Feb 6, 2024
091bddf
Update release manifest
hectorhdzg Feb 6, 2024
c447a6e
Addressing comments
hectorhdzg Feb 8, 2024
b57803c
npm i
hectorhdzg Feb 8, 2024
574ac19
remove inspect break
hectorhdzg Feb 8, 2024
e74650c
Addressing comments
hectorhdzg Feb 9, 2024
6247337
Update
hectorhdzg Feb 9, 2024
3fa81c0
Update packages/winston-transport/README.md
hectorhdzg Feb 10, 2024
3b43b31
Update plugins/node/opentelemetry-instrumentation-winston/README.md
hectorhdzg Feb 10, 2024
5089079
Update packages/winston-transport/test/openTelemetryTransport.test.ts
hectorhdzg Feb 10, 2024
69ae0ea
Update packages/winston-transport/README.md
hectorhdzg Feb 10, 2024
a2bb13e
Addressing comments
hectorhdzg Feb 10, 2024
fc39c78
Update component owners
hectorhdzg Feb 10, 2024
51564b8
Update tav
hectorhdzg Feb 10, 2024
2405492
Addressing comments
hectorhdzg Feb 12, 2024
c2dc4eb
Merge remote-tracking branch 'upstream/main' into hectorhdzg/winstonlogs
hectorhdzg Feb 26, 2024
95d8a6a
Update logger name
hectorhdzg Feb 26, 2024
fb41f93
Merge remote-tracking branch 'upstream/main' into hectorhdzg/winstonlogs
hectorhdzg Feb 28, 2024
ec9da89
Merge branch 'hectorhdzg/winstonlogs' of https://github.com/hectorhdz…
hectorhdzg Feb 28, 2024
1fe760d
Update
hectorhdzg Feb 28, 2024
615c702
Merge branch 'main' into hectorhdzg/winstonlogs
hectorhdzg Feb 28, 2024
8cf9b57
Merge remote-tracking branch 'upstream/main' into hectorhdzg/winstonlogs
hectorhdzg Mar 7, 2024
064cba7
Update logs api dependency
hectorhdzg Mar 7, 2024
9e9e7d1
Merge remote-tracking branch 'upstream/main' into hectorhdzg/winstonlogs
hectorhdzg Mar 11, 2024
fae82c9
Update
hectorhdzg Mar 11, 2024
1750b3e
Merge branch 'main' into hectorhdzg/winstonlogs
hectorhdzg Mar 11, 2024
a36d4e8
Merge branch 'main' into hectorhdzg/winstonlogs
hectorhdzg Mar 13, 2024
a5e1ad6
Addressing comments
hectorhdzg Mar 14, 2024
32fc1a2
Merge branch 'main' into hectorhdzg/winstonlogs
hectorhdzg Mar 14, 2024
5e0291e
Lint
hectorhdzg Mar 14, 2024
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
Added transports
  • Loading branch information
hectorhdzg committed Dec 29, 2023
commit 441f72aacbaf0ab7027bf53fe80b75b22e80d47b
136 changes: 37 additions & 99 deletions package-lock.json

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -59,14 +59,14 @@
"sinon": "15.2.0",
"test-all-versions": "5.0.1",
"ts-mocha": "10.0.0",
"typescript": "4.4.4",
"winston": "3.3.3",
"winston2": "npm:[email protected]"
"typescript": "4.4.4"
},
"dependencies": {
"@opentelemetry/api-logs": "^0.45.1",
"@opentelemetry/core": "1.18.1",
"@opentelemetry/instrumentation": "^0.45.1"
"@opentelemetry/instrumentation": "^0.45.1",
"winston": "3.3.3",
Copy link
Contributor

Choose a reason for hiding this comment

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

Most (all?) the winston@3 transports I see have a dependency just on winston-transport. Could this be changed to only depend on it, and not take a runtime dependency on winston itself?

Copy link
Member

Choose a reason for hiding this comment

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

The guild here explains why instrumentations should not depend on the instrumented library.

I think the instrumentation has to find the winston-transport instances (there might be several of different versions) via the hooking mechanism used for patching. If this isn't possible some other mechanism should be used to avoid depending on the instrumented library.

Copy link
Contributor

Choose a reason for hiding this comment

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

@Flarna If this PR changes to dep on winston-transport would that satisfy the guidelines? This is instrumenting winston, not winston-transport.

One possible use-case of this package -- as described at https://github.com/open-telemetry/opentelemetry-js-contrib/blob/4c6e81347bb1dbc0c87a0de484a48354cef46cb2/plugins/node/opentelemetry-instrumentation-winston/README.md#using-opentelemetrywinstontransport-without-instrumentation -- will be for a user to directly use the OpenTelemetryTransportv3 class to configure their Winston logger. In that case there isn't necessary an active Winston instrumentation. In other words, I don't think it is possible to rely on being able to get winston-transport via the hooking mechanism.

Copy link
Member

@Flarna Flarna Jan 23, 2024

Choose a reason for hiding this comment

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

As long as @opentelemetry/instrumentation-winston is part of @opentelemetry/auto-instrumentations-node I would avoid such dependencies because

  • it adds winston (incl deps) even in projects where winston is not used
  • it has the risk of causing double installs because version ranges do not match
  • might cause issuse with types (see fix: avoid leaking winston types #932)

Maybe we should remove winston instrumentation from @opentelemetry/auto-instrumentations-node. OptIn will be likely done by winston users only.
Or split packages into the "classic" instrumentation (part of auto package) and a V2 and a V3 transport extension (opt-in).

"winston2": "npm:[email protected]"
},
"homepage": "https://github.com/open-telemetry/opentelemetry-js-contrib/tree/main/plugins/node/opentelemetry-instrumentation-winston#readme"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
/*
* Copyright The OpenTelemetry Authors
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

import { Logger, logs } from '@opentelemetry/api-logs';
import * as winston from 'winston2';
import { VERSION } from './version';
import { emitLogRecord } from './utils';

export class OpenTelemetryTransportv2 extends winston.Transport {
private _logger: Logger;

constructor(options: winston.TransportOptions) {
super(options);
this._logger = logs.getLogger(
'@opentelemetry/instrumentation-winston',
VERSION
);
}

log(level: string, msg: string, meta: any, callback: Function) {
try {
const logRecord: Record<string, any> = {
level: level,
msg: msg,
meta: meta,
};
emitLogRecord(logRecord, this._logger);
this.emit('logged');
} catch (error) {
this.emit('warn', error);
}
if (callback) {
callback(null, true);
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
/*
* Copyright The OpenTelemetry Authors
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

import { Logger, logs } from '@opentelemetry/api-logs';
import * as Transport from 'winston-transport';
import { VERSION } from './version';
import { emitLogRecord } from './utils';

export class OpenTelemetryTransportv3 extends Transport {
private _logger: Logger;

constructor(options: Transport.TransportStreamOptions) {
super(options);
this._logger = logs.getLogger(
'@opentelemetry/instrumentation-winston',
VERSION
);
}

public override log(info: any, next: () => void) {
try {
emitLogRecord(info, this._logger);
this.emit('logged', info);
} catch (error) {
this.emit('warn', error);
}
if (next) {
setImmediate(next);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,6 @@ import {
Span,
SpanContext,
} from '@opentelemetry/api';
import * as util from 'node:util';
import {
LogRecord,
Logger,
SeverityNumber,
logs,
} from '@opentelemetry/api-logs';
import {
InstrumentationBase,
InstrumentationNodeModuleDefinition,
Expand All @@ -37,25 +30,23 @@ import {
} from '@opentelemetry/instrumentation';
import type { WinstonInstrumentationConfig } from './types';
import type {
Winston2ConfigureMethod,
Winston2LogMethod,
Winston2LoggerModule,
Winston3ConfigureMethod,
Winston3LogMethod,
Winston3Logger,
} from './internal-types';
import { VERSION } from './version';
import { OpenTelemetryTransportv3 } from './OpenTelemetryTransportv3';
import { OpenTelemetryTransportv2 } from './OpenTelemetryTransportv2';

const winston3Versions = ['>=3 <4'];
const winstonPre3Versions = ['>=1 <3'];

export class WinstonInstrumentation extends InstrumentationBase {
private _logger: Logger;

constructor(config: WinstonInstrumentationConfig = {}) {
super('@opentelemetry/instrumentation-winston', VERSION, config);
this._logger = logs.getLogger(
'@opentelemetry/instrumentation-winston',
VERSION
);
}

protected init() {
Expand All @@ -76,6 +67,12 @@ export class WinstonInstrumentation extends InstrumentationBase {
}
this._wrap(logger.prototype, 'write', this._getPatchedWrite());

// Wrap configure
if (isWrapped(logger.prototype['configure'])) {
this._unwrap(logger.prototype, 'configure');
}
this._wrap(logger.prototype, 'configure', this._getPatchedV3Configure());

return logger;
},
(logger, moduleVersion) => {
Expand Down Expand Up @@ -104,6 +101,12 @@ export class WinstonInstrumentation extends InstrumentationBase {
}
this._wrap(proto, 'log', this._getPatchedLog());

// Wrap configure
if (isWrapped(proto.configure)) {
this._unwrap(proto, 'configure');
}
this._wrap(proto, 'configure', this._getPatchedV2Configure());

return fileExports;
},
(fileExports, moduleVersion) => {
Expand Down Expand Up @@ -163,11 +166,6 @@ export class WinstonInstrumentation extends InstrumentationBase {
}
}
}

if (!config.disableLogSending) {
instrumentation._emitLogRecord(record);
}

return original.apply(this, args);
};
};
Expand Down Expand Up @@ -210,59 +208,63 @@ export class WinstonInstrumentation extends InstrumentationBase {
}
}

if (!config.disableLogSending) {
const logRecord: Record<string, any> = {};
if (args.length >= 1) {
// Level is always first argument
logRecord['level'] = args[0];

// Get meta if present
let metaIndex = 0;
for (let i = args.length - 1; i >= 0; i--) {
if (typeof args[i] === 'object') {
metaIndex = i;
logRecord['meta'] = args[metaIndex];
break;
}
}
const callback =
typeof args[args.length - 1] === 'function' ? 1 : 0;
// Arguments between level and meta or callbkack if present
const msgArguments = args.length - metaIndex - callback - 1;
return original.apply(this, args);
};
};
}

if (msgArguments > 0) {
if (msgArguments === 1) {
logRecord['msg'] = args[1];
} else {
// Handle string interpolation
const values = args.slice(2, msgArguments + 1);
logRecord['msg'] = util.format(args[1], ...values);
}
private _getPatchedV3Configure() {
return (original: Winston3ConfigureMethod) => {
const instrumentation = this;
return function patchedConfigure(
this: never,
...args: Parameters<typeof original>
) {
const config = instrumentation.getConfig();
if (!config.disableLogSending) {
if (args && args.length > 0) {
let originalTransports = args[0].transports;
let newTransports = Array.isArray(originalTransports)
? originalTransports
: [];
const openTelemetryTransport = new OpenTelemetryTransportv3({});
if (originalTransports && !Array.isArray(originalTransports)) {
newTransports = [originalTransports];
}
newTransports.push(openTelemetryTransport);
originalTransports = newTransports;
}
instrumentation._emitLogRecord(logRecord);
}

return original.apply(this, args);
};
};
}

private _emitLogRecord(record: Record<string, any>): void {
const { message, msg, level, meta, ...splat } = record;
const attributes = Object.assign(meta ?? {}, {});
for (const key in splat) {
if (Object.prototype.hasOwnProperty.call(splat, key)) {
attributes[key] = splat[key];
}
}
const logRecord: LogRecord = {
severityNumber: getSeverityNumber(level),
severityText: level,
body: message ?? msg,
attributes: attributes,
private _getPatchedV2Configure() {
return (original: Winston2ConfigureMethod) => {
const instrumentation = this;
return function patchedConfigure(
this: never,
...args: Parameters<typeof original>
) {
const config = instrumentation.getConfig();
if (!config.disableLogSending) {
if (args && args.length > 0) {
let originalTransports = args[0].transports;
let newTransports = Array.isArray(originalTransports)
? originalTransports
: [];
const openTelemetryTransport = new OpenTelemetryTransportv2({});
if (originalTransports && !Array.isArray(originalTransports)) {
newTransports = [originalTransports];
}
newTransports.push(openTelemetryTransport);
originalTransports = newTransports;
}
}
return original.apply(this, args);
};
};
this._logger.emit(logRecord);
}
}

Expand All @@ -282,41 +284,3 @@ function injectRecord(

return Object.assign(record, fields);
}

const npmLevels: Record<string, number> = {
error: SeverityNumber.ERROR,
warn: SeverityNumber.WARN,
info: SeverityNumber.INFO,
http: SeverityNumber.DEBUG3,
verbose: SeverityNumber.DEBUG2,
debug: SeverityNumber.DEBUG,
silly: SeverityNumber.TRACE,
};

const sysLoglevels: Record<string, number> = {
emerg: SeverityNumber.FATAL3,
alert: SeverityNumber.FATAL2,
crit: SeverityNumber.FATAL,
error: SeverityNumber.ERROR,
warning: SeverityNumber.WARN,
notice: SeverityNumber.INFO2,
info: SeverityNumber.INFO,
debug: SeverityNumber.DEBUG,
};

const cliLevels: Record<string, number> = {
error: SeverityNumber.ERROR,
warn: SeverityNumber.WARN,
help: SeverityNumber.INFO3,
data: SeverityNumber.INFO2,
info: SeverityNumber.INFO,
debug: SeverityNumber.DEBUG,
prompt: SeverityNumber.TRACE4,
verbose: SeverityNumber.TRACE3,
input: SeverityNumber.TRACE2,
silly: SeverityNumber.TRACE,
};

function getSeverityNumber(level: string): SeverityNumber | undefined {
return npmLevels[level] ?? sysLoglevels[level] ?? cliLevels[level];
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,16 +14,23 @@
* limitations under the License.
*/

import type { Logger as Winston3Logger } from 'winston';
import type {
Logger as Winston3Logger,
LoggerOptions as Winstons3LoggerOptions,
} from 'winston';
import type {
LoggerInstance as Winston2Logger,
LogMethod as Winston2LogMethod,
} from 'winston2';
export type Winston3LogMethod = Winston3Logger['write'];
export type { Winston3Logger };
export type Winston3ConfigureMethod = Winston3Logger['configure'];
export type { Winston3Logger, Winstons3LoggerOptions };

export type { Winston2LogMethod };
export type Winston2ConfigureMethod = Winston2Logger['configure'];
export type Winston2LoggerModule = {
Logger: Winston2Logger & { prototype: { log: Winston2LogMethod } };
Logger: Winston2Logger & {
prototype: { log: Winston2LogMethod; configure: Winston2ConfigureMethod };
};
};
export type { Winston2Logger };
Loading