Refactor EnvironmentLogger

[sarahchen6]

What does this PR do?
Prior to this PR, the EnvironmentLogger module under Core was logging all environment information (i.e. Tracing and Profiling as well as Core). This PR refactors the code such that each product logs their own environment data. The Core namespace still holds the base class, EnvironmentLogging; however, each product now implements their own EnvironmentLogger subclass. For more information, see here.

This PR also updates the environment logger spec files to utilize more unit tests and less integration tests through mocking and stubbing. This will increase protection for the test cases and make them more specific to the each method.

Motivation
Currently, code within the Core namespace depends upon product implementation. This means that when code is imported from Core, it implicitly imports all the other products as well. The overall goal here is to refactor Core such that all product couplings are removed. This PR specifically addresses the coupling in EnvironmentLogger.

Additional Notes
Note that since environment logging has been split by product, the information is no longer outputted on the same line; however, the original prefix DATADOG CONFIGURATION remains the same, while DATADOG DIAGNOSTIC is now DATADOG ERROR for clarity. Furthermore, the information is outputted in the original location.

How to test the change?
These changes can be tested by running the unit tests with bundle exec rspec spec/datadog/PRODUCT_NAME/diagnostics/environment_logger_spec.rb (where PRODUCT_NAME is either core, tracing, or profiling). To run the suite tests, run bundle exec rake spec:main. And finally, the change can be further verified by passing Github tests and CircleCI.

[codecov-commenter]

Codecov Report

Merging #3020 (4632878) into master (5c390e1) will increase coverage by 0.00%.
Report is 60 commits behind head on master.
The diff coverage is 99.30%.

@@           Coverage Diff            @@
##           master    #3020    +/-   ##
========================================
  Coverage   98.12%   98.12%            
========================================
  Files        1324     1328     +4     
  Lines       74710    74902   +192     
  Branches     3404     3406     +2     
========================================
+ Hits        73309    73499   +190     
- Misses       1401     1403     +2     

Files Changed	Coverage Δ
lib/datadog/tracing/workers/trace_writer.rb	96.25% <ø> (ø)
spec/datadog/core/configuration/components_spec.rb	99.03% <ø> (-0.01%)	⬇️
.../datadog/tracing/diagnostics/environment_logger.rb	97.36% <97.36%> (ø)
lib/datadog/core/diagnostics/environment_logger.rb	98.85% <98.73%> (+1.15%)	⬆️
lib/datadog/core/configuration/components.rb	100.00% <100.00%> (ø)
lib/datadog/profiling.rb	100.00% <100.00%> (ø)
lib/datadog/profiling/component.rb	97.77% <100.00%> (+0.02%)	⬆️
...atadog/profiling/diagnostics/environment_logger.rb	100.00% <100.00%> (ø)
lib/datadog/tracing/component.rb	100.00% <100.00%> (ø)
...atadog/core/diagnostics/environment_logger_spec.rb	100.00% <100.00%> (ø)
... and 4 more

... and 2 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[marcotc]

Awesome work, @sarahchen6!
It looks good to me!
I'd like @delner to take a look as well.

[ivoanjo]

I think code-wise, the current change is fine, but is it possible to avoid the logs showing up on different lines?

When testing on the shell, there's almost no difference between stuff showing in the same line or not. BUT often customers run in systems that collect logging data and show it in ways that it may make it hard to find the multiple log messages.

The big advantage of the existing log message is that it is one log message that contains everything. A support person can ask for it, and that's it -- customers only need to supply that one line that includes everything.

By breaking it down into multiple lines I believe that will create confusion; extra confusion even because other libraries (such as Java) only have a single line, so support people will need to change their flows specifically for Ruby, and only on new versions of the library.

If we can have stuff split, but still have the log message show up as before in one line, I think that would be an overall support and UX win. (I'm otherwise OK with the rest of the code changes)

[delner]

Overall I think this looks great. Awesome job, @sarahchen6 !

---

Responding to some of Ivo's points:

...is it possible to avoid the logs showing up on different lines? ... it may make it hard to find the multiple log messages. ...The big advantage of the existing log message is that it is one log message that contains everything.

Although I don't have a particularly strong opinion, I actually do not see this as desirable. The problem with the one message is that it is one massive message, and it's hard to find the interesting information. Moreover, one message means conflicts with keys, which must be negotiated or otherwise subgrouped... neither of which particularly aid human parsing.

Additionally, we ask already users to collect more than one log message when compiling diagnostics. Asking for an additional, relevant line is both trivial and reasonable.

...support people will need to change their flows specifically for Ruby, and only on new versions of the library.

I don't think "having to change" is in itself a strong argument not to make a change, as long as the change is not (1) very costly/difficult, (2) does not meaningfully degrade experience.

If we can have stuff split, but still have the log message show up as before in one line, I think that would be an overall support and UX win. (I'm otherwise OK with the rest of the code changes)

UX aside, my biggest concern is that making everything log on one line implies synchronization between products/threads. And the need synchronization suggests significant code complexity for little to no gain (which I argued above I think is actually a negative.) I don't think it's worthwhile.

My suggestion is we try out this change. Best case here, it's actually an improvement to UX. If I'm wrong and it's somehow significantly painful/intolerable in UX, we can learn from that when it manifests, and we can go the extra iterative step of addressing that pain. But I think we can all agree it's definitely an improvement to the code design of the library, which is the main objective here.

Responded to the points above. Feedback is duly noted, but does not require immediate action; it can be addressed safely (with minimal risk) in a future iteration. Hence this is not blocking.