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

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

Open
pellared opened this issue Jul 22, 2024 · 11 comments
Open

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

pellared opened this issue Jul 22, 2024 · 11 comments

Comments

@pellared
Copy link
Member

pellared commented Jul 22, 2024

What are you trying to achieve?

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

  • logger name
  • log bridge [package] name
  • log bridge [package] version

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:

  • Does not require changing the Bridge API specification.

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:

  • Similar pattern to instrumentation libraries where library name and version is reported as instrumentation scope.
  • Since there is no need to obtain different Loggers for each name, performance hit in hot path can be avoided
  • Not all logging libraries have a concept of a logger name. But all bridges should have some name.

Drawbacks:

  • May require changing the Bridge API specification. The change would not be breaking the specification as it does not include normative wording. However, it may be seen a breaking change for the Bridge Log API libraries if such requirement is settled e.g. via documentation. Analysis of existing API docs: here.
@pellared
Copy link
Member Author

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 pellared self-assigned this Jul 22, 2024
@pellared
Copy link
Member Author

pellared commented Jul 23, 2024

@open-telemetry/specs-semconv-approvers PTAL

@lmolkova
Copy link
Contributor

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. #1094. I'm happy to support it as long as we can find reasonable and not signal-specific names.

@pellared
Copy link
Member Author

pellared commented Jul 23, 2024

@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
Copy link
Member Author

pellared commented Jul 23, 2024

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 pellared changed the title [Logs] Reporting log bridge name and version [Logs] Reporting logger name, log bridge name and version Jul 23, 2024
@pellared
Copy link
Member Author

pellared commented Jul 23, 2024

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
Copy link
Member Author

pellared commented Jul 24, 2024

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
Copy link
Member Author

pellared commented Jul 24, 2024

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
Copy link
Member Author

pellared commented Oct 29, 2024

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
Copy link
Member

trask commented Oct 31, 2024

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)

@pellared
Copy link
Member Author

pellared commented Nov 6, 2024

@lmolkova, can you move this issue to https://github.com/open-telemetry/semantic-conventions (I lack permissions)? Are you willing to help and create a PR to address this issue? Naturally, I can/will review the PR. I guess it will be more efficient this way.

@trask trask transferred this issue from open-telemetry/opentelemetry-specification Nov 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Todo
Status: Spec - Accepted
Development

No branches or pull requests

3 participants