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

feat: Ruby Logger instrumentation #983

Open
wants to merge 70 commits into
base: main
Choose a base branch
from

Conversation

kaylareopelle
Copy link
Contributor

@kaylareopelle kaylareopelle commented May 14, 2024

Description

This an OpenTelemetry logs bridge for Ruby's standard Logger library.

It also includes patches to ActiveSupport::Logger.broadcast and the ActiveSupport::BroadcastLogger to emit only one log record for a broadcast.

@khushijain21 is a co-author of this PR and contributed functionality as part of her LFX mentorship with OpenTelemetry in 2024.

Closes #668

kaylareopelle and others added 30 commits September 19, 2023 08:37
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
@kaylareopelle kaylareopelle changed the title feat: 🚧 WIP: Logger instrumentation feat: Ruby Logger instrumentation Dec 5, 2024
* Add references to released logs gems
* Test Rails 7.0 - 8.0
* Rubocop
* Set gem version
…ntelemetry-ruby-contrib into logger-instrumentation
severity_text: severity,
severity_number: severity_number(severity),
timestamp: datetime,
body: msg,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

New Relic uses formatted_message here. This includes both the progname and the message in the body, whereas msg does not include the progname. Which seems more appropriate?

@kaylareopelle
Copy link
Contributor Author

I'm still trying to figure out why one of the tests is failing on the CI, but passes locally. However, everything except the ActiveSupportLogger code should be ready for review. Please take a look and I'll push the update as soon as I can!

@tomash
Copy link

tomash commented Mar 10, 2025

I am very interested in getting this to work with Opentelemetry. Any way I could help with this work?

kaylareopelle and others added 3 commits March 10, 2025 16:00
…ntelemetry-ruby-contrib into logger-instrumentation
* move dev dependencies to Gemfile
* require 'logger' in test helper
* set min version to Ruby 3.1
@kaylareopelle
Copy link
Contributor Author

kaylareopelle commented Mar 10, 2025

Hi @tomash! Thanks for reaching out. It's great to hear there's interest in this PR. I'd love your help!

The features for the bridge should be complete. I just updated the branch to align with some of the changes in main. Before this is merged, we need to agree on the approach and get more feedback overall.

Specifically, I could use help with:

  1. Reviewing the code: What are your thoughts on how OpenTelemetry logs are bridged from the Ruby Logger? Do you have suggestions on how we could improve the approach? An alternative approach has been presented in Rails <> OpenTelemetry Logs opentelemetry-ruby#1789, would you rather see something like this?

  2. Testing the code in an app: Install the gem from this branch and provide feedback on how it works in your application. This is a Gemfile from a sample application I wrote to add the OpenTelemetry bridge for the Ruby logger to a Rails application. Give it a try and let us know how it goes.

@tomash
Copy link

tomash commented Mar 11, 2025

@kaylareopelle

For now I have this monstrosity in Gemfile:

gem "opentelemetry-instrumentation-logger",
  git: "https://github.com/kaylareopelle/opentelemetry-ruby-contrib.git",
  branch: "logger-instrumentation",
  glob: "instrumentation/logger/opentelemetry-instrumentation-logger.gemspec"

And I can confirm it works well for our app (the logs appear in Honeycomb just as events). The only problem is that Rails logs are noisy by default and it makes "top messages" unusable (top message being "rendered profiles/_carousel.html.erb"), so I've also dropped in Lograge to produce fewer logs.

I haven't seen open-telemetry/opentelemetry-ruby#1789 , will evaluate and compare! I do like how lightweight and easy to read it is.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
keep Ensures stale-bot keeps this issue/PR open
Projects
Status: Pending PR
Development

Successfully merging this pull request may close these issues.

Add instrumentation for Ruby Logger leveraging Log Records
4 participants