open-telemetry / opentelemetry-specification

Specifications for OpenTelemetry
https://opentelemetry.io
Apache License 2.0
3.74k stars 888 forks source link

[Logs] Reporting logger name, log bridge name and version #4154

Open pellared opened 3 months ago

pellared commented 3 months ago

What are you trying to achieve?

I would like a convention on how the log bridges should report:

Thanks to this backends would get telemetry of the log bridges in use that can help with troubleshooting and provide information what log bridges are popular.

Currently instrumentation scope name is supposed to be used as a "logger name" per: opentelemetry-specification/specification/logs/bridge-api.md

For log sources which define a logger name (e.g. Java Logger Name) the Logger Name should be recorded as the instrumentation scope name.

An initial proposal for the change was here, but it was postponed until now.

Slack thread before the issue was created: https://cloud-native.slack.com/archives/C041APFBYQP/p1721231579496729

Proposal A

Add bridge.name and bridge.version instrumentation scope attributes to report the log bridge package name and version. Use logger's instrumentation scope name parameter via Bridge API to report the logger name.

Benefits:

Drawbacks:

Proposal B

Use logger's instrumentation scope parameters via Bridge API to report the log bridge package name and version. Add logger.name log attribute to report the logger name.

Benefits:

Drawbacks:

pellared commented 3 months ago

In Go, NOT having a logger name it is VERY common:

What should be the logger name obtained by the bridge API in such scenarios? Moreover, it is very clumsy for zap when the logger entry MAY (but usually does not) have a logger name.

For OTel Go it would be more natural to proceed with Proposal B.

pellared commented 3 months ago

@open-telemetry/specs-semconv-approvers PTAL

lmolkova commented 3 months ago

What would be the reason not stop recommending logger name to match the instrumentation scope name? I.e. why not option A? Is it only perf and why would someone acquire a logger on a hot path?

The instrumentation library name used on tracer/meter names is not a requirement and with per-scope config introduced recently we should really provide better granularity than a library name.

WRT bridge.name/version - there were some attempts and interest to define instrumentation scope (or just any) generic attributes for instrumentation library - e.g. https://github.com/open-telemetry/semantic-conventions/pull/1094. I'm happy to support it as long as we can find reasonable and not signal-specific names.

pellared commented 3 months ago

@lmolkova, thanks a lot for your feedback.

The other reason (drawback number 1) is that instrumentation scope name is a required parameter and some (in Go almost all) logging libraries does not have a logger name. What logger name the log bridge should use in such scenario?

The way we currently implemented it in Go log bridges is that we require the user to manually specify the logger name when constructing a log bridge. However, in otelzap we have a special case:

For named loggers, LoggerName is used to access log.Logger from log.LoggerProvider

I find such "conditional logic" for logger name confusing (by default a value coming from the bridge setup, for sub-loggers a value coming from the bridged logging library).

I'm happy to support it as long as we can find reasonable and not signal-specific names.

I think that having a bridge.name is reasonable as if we have something like package.name how would we know e.g. if this is not a package where from where the log has been emitted? I would say it is a similar problem like using "logger.name" as instrumentation scope name. My idea for Proposal A is to define a semantic convention for Log Instrumentation Scope Attributes here.

The instrumentation library name used on tracer/meter names is not a requirement and with per-scope config introduced recently we should really provide better granularity than a library name.

I am not following this. From https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/glossary.md#instrumentation-scope:

The most common approach is to use the instrumentation library as the scope, however other scopes are also common, e.g. a module, a package, or a class can be chosen as the instrumentation scope.

I think that log bridges can be seen as instrumentation libraries for logging libraries.

pellared commented 3 months ago

JavaDoc gives freedom whether to use the bridge name/version or logger name as instrumentation scope when getting the logger. See https://github.com/open-telemetry/opentelemetry-java/blob/468b5289564b45b7a07c5caee3972d2038dbc6ca/api/all/src/main/java/io/opentelemetry/api/logs/LoggerProvider.java#L24-L33:

  /**
   * Gets or creates a named Logger instance.
   *
   * @param instrumentationScopeName A name uniquely identifying the instrumentation scope, such as
   *     the instrumentation library, package, or fully qualified class name. Must not be null.
   * @return a Logger instance.
   */
  default Logger get(String instrumentationScopeName) {
    return loggerBuilder(instrumentationScopeName).build();
  }

C++ API comments does not say anything about the semantics of getting the logger. See https://github.com/open-telemetry/opentelemetry-cpp/blob/deed1e3a0c3d67afd9db522b69daad567cdb0592/api/include/opentelemetry/logs/logger_provider.h#L28-L44:

  /**
   * Gets or creates a named Logger instance.
   *
   * Optionally a version can be passed to create a named and versioned Logger
   * instance.
   *
   * Optionally a configuration file name can be passed to create a configuration for
   * the Logger instance.
   *
   */

  virtual nostd::shared_ptr<Logger> GetLogger(
      nostd::string_view logger_name,
      nostd::string_view library_name            = "",
      nostd::string_view library_version         = "",
      nostd::string_view schema_url              = "",
      const common::KeyValueIterable &attributes = common::NoopKeyValueIterable()) = 0;

PHP LoggerProvider documentation only refers to "main" version of the specification. See https://github.com/open-telemetry/opentelemetry-php/blob/68b1b43cab7b053a3148af56735817b92ddfbbfe/src/API/Logs/LoggerProviderInterface.php#L7-L18:

/**
 * @see https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/logs/bridge-api.md#get-a-logger
 */
interface LoggerProviderInterface
{
    public function getLogger(
        string $name,
        ?string $version = null,
        ?string $schemaUrl = null,
        iterable $attributes = [], //instrumentation scope attributes
    ): LoggerInterface;
}

None other languages have stable Bridge API.

However, it may be seen a breaking change for the Bridge Log API libraries if such requirement is settled e.g. via documentation.

None of the stable Log Bridge APIs' documentations require how "get a logger" must be used.

Moreover, the "get logger" specification is also very ambiguous on how the parameters should be used:

This name uniquely identifies the instrumentation scope, such as the instrumentation library (e.g. io.opentelemetry.contrib.mongodb), package, module or class name. If an application or library has built-in OpenTelemetry instrumentation, both Instrumented library and Instrumentation library may refer to the same library. In that scenario, the name denotes a module name or component name within that library or application. For log sources which define a logger name (e.g. Java Logger Name) the Logger Name should be recorded as the instrumentation scope name.

What is an instrumentation library in the context of log bridges? What about log sources which only sometimes define a Logger Name (as an optional feature)?

Specifies the version of the instrumentation scope if the scope has a version (e.g. a library version).

Version of the bridge? Of the module/package that emits logs?

I do not find any normative and clear recommendation coming form Bridge API docs nor Specification. Given that I do not think that there is a blocker for defining Proposal B in logs semantic conventions. I think it is also a better place to define it in Semantic Conventions as OpenTelemetry Specification cannot force the users to call the Bridge APIs in a certain way. We can also look into improving the "get logger" description to make it less confusing. E.g in

For log sources which define a logger name (e.g. Java Logger Name) the Logger Name should be recorded as the instrumentation scope name.

Change "should" to "can" to make it clear that it is the caller's decision and semantic conversation responsibility to define usage recommendations.

pellared commented 3 months ago

There is also one very important aspect that should be taken into consideration whether it is better to use use [bridged] logger name or bridge [package] name as instrumentation scope name which is "identity" as the instrumentation scope name is the "main" identity parameter when getting the logger.

Loggers are identified by name, version, and schema_url fields.

What would be the desired granularity of LoggerConfigurator?

Should it be more "coupled" to a log bridge (which I see as instrumentation library) or application code (e.g. Java Logger Name.

I am worried about of the cardinality when logger names would be used as instrumentation scope names.

pellared commented 3 months ago

2024-07-23 SIG meeting notes:

@jack-berg, pointed out the main idea of using bridged logger name as instrumentation scope name is that it similar how people [should] name tracers and meters for custom (manual) instrumentation. The bridges are simply supposed to bridge the application logging (which is kind of a custom instrumentation). This also to main reason why we settled on the name "log bridge" and not "log instrumentation library". For the cases where a logger name is not supplied by the bridges logging library, the bridge should use some fallback logger name value (preferably provided by the user). Therefore, Proposal A looks to be more user-friendly.

Side note: This is how current OTel Go log bridges are working. However, we do not emit any instrumentation scope attributes.

pellared commented 3 months ago

My plan is to wait a few days for more feedback, have it "accepted" during TC triaging, and then create a PR in https://github.com/open-telemetry/semantic-conventions with Proposal A to keep the momentum.

In the meantime it would be good to solve:

pellared commented 4 days ago

SIG meeting notes from 2024-10-22 (if I remember correctly, @lmolkova PTAL): Apart for log bridge name and version we may also want to have name and version of the instrumented library (do not confuse with instrumentation library). However, this is a separate issue where instrumentation scope attributes can be handy.

trask commented 3 days ago

SIG meeting notes from 2024-10-22 (if I remember correctly, @lmolkova PTAL)

another part of that discussion was @lmolkova suggesting that we may want to promote more granular scopes in the future for traces and metrics (more granular than instrumentation library name), in which case we would want to introduce a scope attribute to store the instrumentation library name even for traces and metrics (and that scope attribute would fit for storing log bridge name)