feat: 🚧 WIP: Logger instrumentation #983
Conversation
…-ruby-contrib into logger-instrumentation
…elle/opentelemetry-ruby-contrib into logger-instrumentation
Appraisal can't install gems from a git source. Since the appraisal is only necessary for active_support_logger, disable those tests while working on other features.
chore: Allow logger patch tests to run
…-ruby-contrib into logger-instrumentation
…elle/opentelemetry-ruby-contrib into logger-instrumentation
feat: map logger level to OTel level
It's used in the instrumentation when on_emit is called
| require 'opentelemetry-instrumentation-http_client' | ||
| require 'opentelemetry-instrumentation-logger' |
There was a problem hiding this comment.
Should the Logger bridge be part of instrumentation/all? I didn't realize the difference between a "bridge" and an "instrumentation" until after this had been worked out. We could create a separate directory at a peer level with instrumentation named "bridge(s?)" and put the code there.
If someone knows of an "instrumentation" definition within OTel documentation, that might help.
I'm sure there's other pros/cons, but this is what I have off the top of my head:
Pros to having it in instrumentation/all:
- Easier for people to adopt if they're already installing their instrumentation via all
- One less CI workflow to create, along with all the other structure required for another grouping (ex.
processor,propagator)
Cons:
- The specs reference a "Bridge API", so we could be breaking from that concept by grouping this with instrumentation
There was a problem hiding this comment.
I think it's ok to include it in all since it has the standard instrumentation structure. Although, instrumentation present do ... end that check if the instrumentation is little bit loose in my opinion. Maybe also check if ::OpenTelemetry::SDK::Logs present?
| # # TOOD: Re-enable before release, along with active_support_logger tests | ||
| # appraise 'rails-6.0' do | ||
| # gem 'rails', '~> 6.0.0' | ||
| # end | ||
|
|
||
| # appraise 'rails-6.1' do | ||
| # gem 'rails', '~> 6.1.0' | ||
| # end | ||
|
|
||
| # appraise 'rails-7.0' do | ||
| # gem 'rails', '~> 7.0.0' | ||
| # end |
There was a problem hiding this comment.
I haven't been able to get Appraisal to work with gems installed from git source. This might just be due to my limited knowledge of Appraisal. We may need to wait until the opentelemetry-logs-related gems are released before we can re-instate these.
There was a problem hiding this comment.
Have you tried something like:
gem 'opentelemetry-logs-api', github: 'open-telemetry/opentelemetry-ruby', branch: 'main', glob: 'logs_api/opentelemetry-logs-api.gemspec'
gem 'opentelemetry-logs-sdk', github: 'open-telemetry/opentelemetry-ruby', branch: 'main', glob: 'logs_sdk/opentelemetry-logs-sdk.gemspec'
| gem 'opentelemetry-instrumentation-base', path: '../base' | ||
| gem 'opentelemetry-api', git: 'https://github.com/kaylareopelle/opentelemetry-ruby', branch: 'log-record-processor3', glob: 'api/*.gemspec' | ||
| gem 'opentelemetry-logs-api', git: 'https://github.com/kaylareopelle/opentelemetry-ruby', branch: 'log-record-processor3', glob: 'logs_api/*.gemspec' | ||
| gem 'opentelemetry-logs-sdk', git: 'https://github.com/kaylareopelle/opentelemetry-ruby', branch: 'log-record-processor3', glob: 'logs_sdk/*.gemspec' | ||
| gem 'opentelemetry-sdk', git: 'https://github.com/kaylareopelle/opentelemetry-ruby', branch: 'log-record-processor3', glob: 'sdk/*.gemspec' |
There was a problem hiding this comment.
Current workaround to test against the WIP SDK/etc. code.
| # The OpenTelemetry module provides global accessors for telemetry objects. | ||
| # See the documentation for the `opentelemetry-api` gem for details. | ||
| module OpenTelemetry | ||
| # Instrumentation should be able to handle the case when the library is not installed on a user's system. |
There was a problem hiding this comment.
| # Instrumentation should be able to handle the case when the library is not installed on a user's system. |
| module Instrumentation | ||
| module Logger | ||
| module Patches | ||
| # Patches for the ActiveSupport::Logger class included in Rails |
There was a problem hiding this comment.
What's this all about? Well, when this instrumentation is used with Rails, we get double the logs emitted because of how Rails broadcasts the logs. This workaround is similar to what we did in New Relic's Ruby agent.
I'm open to feedback and better solutions.
An additional workaround still needs to be developed for the Rails 7.1 broadcast logger.
| severity_text: severity, | ||
| severity_number: severity_number(severity), | ||
| timestamp: datetime, | ||
| body: msg # New Relic uses formatted_message here. This also helps us with not recording progname, because it is included in the formatted message by default. Which seems more appropriate? |
There was a problem hiding this comment.
Formatted message is what you're used to seeing in stdout or your logs. By default, it includes severity, timestamp, process.pid, and progname in addition to the message.
I don't see a clear answer from the Logs Data Model entry on body.
| end | ||
|
|
||
| present do | ||
| defined?(::Logger) |
There was a problem hiding this comment.
(Repeat) Maybe also check if ::OpenTelemetry::SDK::Logs present?
If there is no otel sdk/api logs installed, then I assume OpenTelemetry.logger_provider will give error?
There was a problem hiding this comment.
@xuan-cao-swi accommodated this feedback
feat: methods to set custom logger name and version
chore: Add tests for name and version config
feat: add check for logs sdk
Description
This is a work-in-progress implementation of an OpenTelemetry logs bridge for Ruby's built-in Logger library.
It relies on a WIP Logs SDK and OTLP exporter implementation. If you'd like to test out this experimental code, follow the instructions on this gist, or clone this demo repo.
The SDK and OTLP exporter code is being reviewed in small chunks on the open-telemetry/opentelemetry-ruby repository. Follow the progress on the Logs project board.
Once the SDK/exporter code is merged into the main repo, this PR can be taken out of draft mode.
@khushijain21 is a co-author of this PR and contributed functionality as part of her LFX mentorship with OpenTelemetry in 2024.
TODOs:
Closes #668