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

fix: ForwardingLogger should foward block param #1616

Merged
merged 2 commits into from
Mar 21, 2024

Conversation

BrianHawley
Copy link
Contributor

Also added tests to verify the default forwarding logger level, and to make sure it actually forwards logs.

Fixes #1599

Also added tests to verify the default forwarding logger level, and to
make sure it actually forwards logs.

Fixes open-telemetry#1599
Copy link

linux-foundation-easycla bot commented Mar 16, 2024

CLA Signed

The committers listed above are authorized under a signed CLA.

@arielvalentin
Copy link
Contributor

@BrianHawley please review the EasyCLA.

If you accept it, we will proceed with a review of this PR.

Thank you.

@BrianHawley
Copy link
Contributor Author

@arielvalentin license accepted, thanks.

@dazuma dazuma merged commit b1d6d05 into open-telemetry:main Mar 21, 2024
58 checks passed
@szechyjs
Copy link

This seems to also be busted when using Ougai as the logger.

@adamk9k
Copy link

adamk9k commented Sep 27, 2024

This introduced a bug if the logger used is Semantic Logger

OpenTelemetry error: no block given (yield)

coming from https://github.com/open-telemetry/opentelemetry-ruby/blob/main/sdk/lib/opentelemetry/sdk/forwarding_logger.rb#L41

@kaylareopelle
Copy link
Contributor

Thanks, @adamk9k! I'll open an issue for this to take a closer look. If you know what needs to be done to fix the problem, please feel free to open a PR.

@adamk9k
Copy link

adamk9k commented Oct 2, 2024

Thanks, @adamk9k! I'll open an issue for this to take a closer look. If you know what needs to be done to fix the problem, please feel free to open a PR.

Thank you @kaylareopelle in the meantime I've figured out what the issue was. It was coming from common/lib/opentelemetry/common/utilities.rb version 0.20.0 when it looked like this

def untraced
  Context.with_value(UNTRACED_KEY, true) do |ctx, _|
    yield ctx
  end
end

upgrading to 0.21.0 that includes this #1634 fixed it.

@kaylareopelle
Copy link
Contributor

@adamk9k, that's great news! It sounds like nothing further needs to be done, and #1738 can be closed. Is that correct?

@adamk9k
Copy link

adamk9k commented Oct 3, 2024

@kaylareopelle yes, thank you and sorry for the noise.

@kaylareopelle
Copy link
Contributor

@adamk9k, no problem! Thanks for the update!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Log message passed in progname, not in message parameter
6 participants