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

create_kafka_client leads to: undefined method `formatter=' for #<Logging::Logger:0x000055aa7dcb8ae0> (NoMethodError) #111

Open
olleolleolle opened this issue Apr 11, 2019 · 14 comments
Assignees
Labels

Comments

@olleolleolle
Copy link

olleolleolle commented Apr 11, 2019

This Issue is about compatibility re: what is passed into ruby-kafka:

      with a delivery interval set
#<Thread:0x000055aa7e6877d8@/usr/local/bundle/gems/logging-2.2.2/lib/logging/diagnostic_context.rb:471 run> terminated with exception (report_on_exception is true):
Traceback (most recent call last):
	7: from /usr/local/bundle/gems/logging-2.2.2/lib/logging/diagnostic_context.rb:474:in `block in create_with_logging_context'
	6: from /opt/phobos/spec/lib/phobos/producer_spec.rb:142:in `block (6 levels) in <top (required)>'
	5: from /opt/phobos/lib/phobos/producer.rb:85:in `create_async_producer'
	4: from /opt/phobos/lib/phobos.rb:53:in `create_kafka_client'
	3: from /usr/local/bundle/gems/ruby-kafka-0.7.6/lib/kafka.rb:366:in `new'
	2: from /usr/local/bundle/gems/ruby-kafka-0.7.6/lib/kafka.rb:366:in `new'
	1: from /usr/local/bundle/gems/ruby-kafka-0.7.6/lib/kafka/client.rb:75:in `initialize'
/usr/local/bundle/gems/ruby-kafka-0.7.6/lib/kafka/tagged_logger.rb:57:in `new': undefined method `formatter=' for #<Logging::Logger:0x000055aa7dcb8ae0> (NoMethodError)
Did you mean?  formatter

Originally posted by @olleolleolle in #110 (comment)

@olleolleolle
Copy link
Author

olleolleolle commented Apr 11, 2019

Snooping around notes

Hm, #formatter= is not a part of the generic API: (Narrator: "It was.")

https://ruby-doc.org/stdlib-2.6.2/libdoc/logger/rdoc/Logger.html#top

#formatter is added by this conditional include:
https://github.com/TwP/logging/blob/master/lib/logging/rails_compat.rb

@mensfeld
Copy link

It is: You can use formatter= for escaping all data. - from the 2.6.2 docs.

@olleolleolle
Copy link
Author

olleolleolle commented Apr 11, 2019

@mensfeld Ah, I ought to make a tiny docs amendment to the stdlib. (Update: 👓 Now I see why I didn't find it immediately: Attributes are not listed among the methods, in the handy sidebar.)

@dorner
Copy link
Contributor

dorner commented Apr 11, 2019

This is actually more of an issue with ruby-kafka - this is my fault since a PR I wrote which was merged seems to be causing some of these crashes. We'll likely be pulling that out and trying it from a different perspective.

@klippx
Copy link
Member

klippx commented Apr 11, 2019

We could add ruby-kafka <= $someVersion to the gemspec until this issue is sorted.

@peco8
Copy link

peco8 commented Jun 17, 2019

Any updates? I just work around this issue by disabling ruby-kafka logs.

In config/phobos.yml.

  # Comment the block to disable ruby-kafka logs
  #ruby_kafka:
  #  level: error

@dorner
Copy link
Contributor

dorner commented Jun 17, 2019

There are a couple of workarounds. Another one is to monkey-patch the logging logger so it actually conforms to the Logger interface:

module Logging
  class Logger
    def formatter=(*args); end
  end
end

You can also manually configure Phobos with a custom logger:

Phobos.configure do |c| 
  c.custom_logger = Logger.new(STDOUT)
  c.custom_kafka_logger = Logger.new(STDOUT)
end

The monkey patch is included in this PR: #112 and a full fix is merged into ruby-kafka but not released yet.

@klippx
Copy link
Member

klippx commented Jun 18, 2019

Can we create a new issue to track removing the monkey patch?

We can make the monkey patch an optional thing Phobos users can opt in to.

@dorner
Copy link
Contributor

dorner commented Jun 18, 2019

Sure, I can make an issue to remove it once RubyKafka does the next release. Honestly the monkey patch is incredibly benign (it just defines a couple of no-op methods).

@klippx
Copy link
Member

klippx commented Jun 19, 2019

@dorner have you reproduced this issue? And verified #112 solves this issue?

Would it be possible for @peco8 and/or @olleolleolle to verify that the branch cache-sync-producer solves this issue?

@olleolleolle
Copy link
Author

@mriska Perhaps you can verify the solving of this?

(Thanks for asking, but I no longer can check.)

@dorner
Copy link
Contributor

dorner commented Jun 19, 2019

@klippx it's easy to confirm the fix - if you run bundle upgrade ruby-kafka and run rspecs on Phobos, they'll all crash. With the monkey patch in place they'll succeed.

@capripot
Copy link

I don't think the patch #112 resolves zendesk/ruby-kafka#732

Should we make Logging::Logger inherit from ::Logger?

@dorner
Copy link
Contributor

dorner commented Jul 26, 2019

We can't do that unfortunately. I've added one more patch to RubyKafka - here's hoping it will fix it for good and all. zendesk/ruby-kafka#762

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

No branches or pull requests

6 participants