dd-trace-rb
dd-trace-rb copied to clipboard
Avoid diagnostics on rails console
Currently I get a huge DATADOG CONFIGURATION payload output when starting a rails console.
The Rails::Console
constant is only loaded when the rails console
command is run:
https://github.com/rails/rails/blob/main/railties/lib/rails/commands/console/console_command.rb
It's a semi-conventional way to detect if we're running in a rails console. This PR uses its presence as a signal that we're in a repl, and so avoiding printing the diagnostic information by default.
👋 @sj26, this is a good call!
We actually introduced specific code recently to detect just this kind of scenario, as we needed this detection logic elsewhere in the code: https://github.com/DataDog/dd-trace-rb/blob/65c019dbad0969618b06fc370eb154d27a18c82d/lib/datadog/core/environment/execution.rb#L9-L12
This just wasn't propagated to the EnvironmentLogger yet, given it was undergoing a major refactoring when the above changes were introduced.
Oh, neat!
I don't think the code you've linked would catch my particular case — a rails console in production. But I could make the change to that core class, if you prefer, and maybe point this environment logger at that core class?
Hm, looking more closely I don't think Datadog::Core::Environment.development?
encapsulates what this EnvironmentLogger
class wants to know. It's probably closer to interactive?
Which makes me think a more reliable test for what this is trying to do might be "does this process have a tty on stdin", i.e. $stdin.isatty
i.e. I wonder if this would be good:
https://github.com/DataDog/dd-trace-rb/compare/master...sj26:dd-trace-rb:interactive-environment
@sj26, your proposed changes are very clean :) https://github.com/DataDog/dd-trace-rb/compare/master...sj26:dd-trace-rb:interactive-environment
Hm, looking more closely I don't think Datadog::Core::Environment.development? encapsulates what this EnvironmentLogger class wants to know. It's probably closer to interactive?
We wan the environment logger to output information when: It won't bother developers.
So it applies to local REPLs, as well as local test runs or rake tests.
In Rails console in a production environment is probably a place where we don't want it because it's verbose. But it could be helpful if you are having issues with Datadog and is trying to debug something related to it in a Rails console in production. That being said, Rails production consoles are used much more frequently to do things other than debugging Datadog observability, so I disabling seems reasonable.
Do you agree with my description above, @sj26? I want to make sure this would make sense for your use case.
Yes, that sounds right to me :+1:
The $stdin.tty?
thing works for most things you've listed, like rake tasks etc too.
Feel free to use that branch if you like? Or I can pull that in here? Not sure how to do the testing piece though.