[instrumentation-bunyan] Allow log level to be configured for auto instrumentation and BunyanStream

[hectorhdzg]

Currently there is no way to control which level/severity for logs auto generation, all logs will be emitted

[trentm]

I'm not sure that filtering on log-level/severity is the job of an OTel log-sending appender/stream/transport class.

Currently there is no way to control which level/severity for logs auto generation, all logs will be emitted

The level of logs that are emitted is determined by the Logger class (also in Bunyan's case by the stream info associated with a Bunyan output stream). So, for example, a Logger configured at the INFO level and with an OpenTelemetryBunyanStream will emit at INFO level and above.

I might be misunderstanding your intention, however. Looking at this part of your PR:

https://github.com/open-telemetry/opentelemetry-js-contrib/pull/1919/files#diff-ef1588ccf607d6bccfe2db2d198098611ea4caef1b1630d6466eb668c2cb42b4R160-R166

    logger.addStream({
      type: 'raw',
      stream: new OpenTelemetryBunyanStream({
        logSendingLevel: config.logSendingLevel,
      }),
      level: logger.level(),
    });

Here there are two levels involved with that stream, which I think will be confusing. If logger.level() is INFO, but config.logSendingLevel is DEBUG, the OpenTelemetryBunyanStream will only ever receive log records at INFO-level and above (i.e. it will not receive DEBUG-level records).

Did you perhaps intend some way to configure the auto-instrumentation to set that existing level: logger.level() value? Something like this?

    logger.addStream({
      type: 'raw',
      stream: new OpenTelemetryBunyanStream({
        logSendingLevel: config.logSendingLevel,
      }),
      level: config.logSendingLevel ? bunyanLevelFromSeverityNumber(config.logSendingLevel) : logger.level(),
    });

[hectorhdzg]

@trentm according to docs streams can have specific levels not specific to the actual logger right? https://www.npmjs.com/package/bunyan#stream-type-stream
Scenario we want to support is if customer creates a Bunyan Logger with level "info" for example, we allow telemetry to only be generated for error logs, not the other way around, customer could be using some other stream like a file and store more detailed info, but there will not be any LogRecords being generated.

[trentm]

according to docs streams can have specific levels not specific to the actual logger right?

Bunyan's use of "stream" gets pretty ambiguous sometimes. (I regret using the bare word "stream" for this facility because of the confusion.) Given:

const bunyan = require('bunyan');
const log = bunyan.createLogger({name: 'foo'});
log.addStream({
  name: 'my-stream-name',
  level: 'debug',
  stream: process.stderr
});

Bunyan docs and code loosely refer to that {name: 'my-stream-name', level: 'debug', stream: process.stderr} full object as "a stream" -- though a better name would be something like "stream info", because of course the process.stderr is a Node.js stream.

In Bunyan, it is this "stream info" object that can have a level. When you do that log.addStream({level: 'debug', ...}), Bunyan knows to lower the Logger instance's self._level down to at least 'debug' level so that log.debug(...)s are considered. It is the Logger that handles the filtering on levels, not the stream-like thing (any object with a write() method) passed with the "stream" property (process.stderr in this case). There is nothing that prevents the stream-like thing from encapsulating its own level filtering, but that wasn't Bunyan's intention, and the stream-like thing may be limited in that it will only receive log records at the stream-info's level and above.

Scenario we want to support is if customer creates a Bunyan Logger with level "info" for example, we allow telemetry to only be generated for error logs, not the other way around

I think that would be handled by something like:

    logger.addStream({
      type: 'raw',
      stream: new OpenTelemetryBunyanStream(),
      level: config.logSendingLevel ? bunyanLevelFromSeverityNumber(config.logSendingLevel) : logger.level(),
    });

(I'd meant to write it this way in my preceding comment, but I accidentally left in the logSendingLevel argument to new OpenTelemetryBunyanStream()).

So, in other words, the instrumentation would still take a level/severity-related config option, but that would be passed to addStreams "level" option (logger.addStream({..., level: HERE})).

If that seems reasonable, then...

severity vs level

1. Should the config option to BunyanInstrumentation be in terms of OTel severity number or Bunyan level name/number. Probably the OTel values, because this is an OTel class and it would allow having the same values for a different logger instrumentation (e.g. WinstonInstrumentation).
2. Should the word "severity" be in option name to help disambiguate? logSendingSeverityNumber is a mouthful.
3. bunyanLevelFromSeverityNumber hopefully will be a fairly straightforward reverse implementation of the existing severityNumberFromBunyanLevel.