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

Remove MirroredLogger and settings #15397

Merged
merged 1 commit into from
Jun 19, 2017
Merged

Conversation

Fryguy
Copy link
Member

@Fryguy Fryguy commented Jun 19, 2017

In reviewing our logging in light of the common logging rearchitecture, we found that MirroredLogger is no longer necessary.

  • Since it mirrors lines, when we single stream the logs with common logging we end up with duplicate entries.
  • The automate log was hardcoded to always mirror at info level, so in speaking with @gmcculloug there should be no problem for them to just look at the automate.log directly.
  • I also spoke to @jdeubel directly, and he agreed that the mirroring was no longer needed and in fact, at times, it gets in the way. cc @jcarter12
  • I doubled checked the ui-classic code, and there are no UI screens or code related to mirroring. The only way it could have been set in the UI was via the AdvancedSettings, which is yaml driven.

Note that I have PRs for 4 other provider repos to remove the entry from settings.yml, but they are not in the critical path for this PR.

@bdunne Please review

Copy link
Member

@bdunne bdunne left a comment

Choose a reason for hiding this comment

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

👍 LGTM

@miq-bot
Copy link
Member

miq-bot commented Jun 19, 2017

Checked commit Fryguy@fb72573 with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
4 files checked, 1 offense detected

lib/vmdb/loggers.rb

  • ⚠️ - Line 72, Col 5 - Lint/IneffectiveAccessModifier - private (on line 38) does not make singleton methods private. Use private_class_method or private inside a class << self block instead.

@bdunne bdunne merged commit 2323c7f into ManageIQ:master Jun 19, 2017
@bdunne bdunne added the fine/no label Jun 19, 2017
@bdunne bdunne added this to the Sprint 63 Ending Jun 19, 2017 milestone Jun 19, 2017
@Fryguy Fryguy deleted the kill_mirrored_logger branch June 19, 2017 18:29
miha-plesko added a commit to miha-plesko/manageiq that referenced this pull request Jan 5, 2018
There is a [PR](ManageIQ#16641)
already merged in master that introduces $vcloud_log, but since
logging has changed in this [PR](ManageIQ#15397),
there are merge conflicts for fine branch. Hence this commit.

Signed-off-by: Miha Pleško <[email protected]>
d-m-u pushed a commit to d-m-u/manageiq that referenced this pull request Jun 6, 2018
There is a [PR](ManageIQ#16641)
already merged in master that introduces $vcloud_log, but since
logging has changed in this [PR](ManageIQ#15397),
there are merge conflicts for fine branch. Hence this commit.

Signed-off-by: Miha Pleško <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants