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.

Sign up for GitHub

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

Jump to bottom

Document how to enable/disable Debug Output on the fly #9981

Merged
merged 1 commit into from
Oct 30, 2024
Merged

Document how to enable/disable Debug Output on the fly #9981

merged 1 commit into from
Oct 30, 2024

Conversation

Al2Klimov
Copy link
Member

This is a good alternative to icinga2 feature enable debuglog:

  • Object creation/deletion via API happens immediately and requires no restart
  • Hence, the debug log is enabled exactly as long as desired

@Al2Klimov Al2Klimov added enhancement New feature or request area/configuration DSL, parser, compiler, error handling area/documentation End-user or developer help area/api REST API area/log Logging related consider backporting Should be considered for inclusion in a bugfix release labels Jan 26, 2024
@cla-bot cla-bot bot added the cla/signed label Jan 26, 2024
@Al2Klimov Al2Klimov mentioned this pull request Jan 26, 2024
Merged
yhabteab
yhabteab reviewed May 7, 2024
View reviewed changes
Copy link
Member

@yhabteab yhabteab left a comment

Choose a reason for hiding this comment

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

Pls. rebase!

doc/15-troubleshooting.md Outdated Show resolved Hide resolved
doc/15-troubleshooting.md Outdated Show resolved Hide resolved
Al2Klimov
Al2Klimov commented May 7, 2024
View reviewed changes
doc/15-troubleshooting.md Outdated
* Hence, the debug log is enabled exactly as long as desired

However, in case of [a HA zone](06-distributed-monitoring.md#distributed-monitoring-scenarios-ha-master-agents)
the feature gets enabled/disabled on both nodes once they're connected.
Copy link
Member Author

Choose a reason for hiding this comment

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

⚠️ Thing to consider

Copy link
Member

Choose a reason for hiding this comment

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

What did happen to this consideration? I mean it's clearly a bug, but I didn't get why posted such a prominent warning here and doing nothing about it.

Copy link
Member Author

Choose a reason for hiding this comment

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

why posted such a prominent warning here

This PR (necessarily) "sells" the user a shiny HA feature which has a bug which may negatively affect what the user's doing inspired by this PR. Hence, you as reviewers may reconsider the PR as whole...

and doing nothing about it

... because, should we really make a disclaimer in the docs like, HA may be not reliable?

Copy link
Member

Choose a reason for hiding this comment

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

... because, should we really make a disclaimer in the docs like, HA may be not reliable?

No, I wouldn't do it either, as the bug literally impacts every Icinga 2 object. I just didn't get the purpose of this comment with a prominent and eye catching warning sign.

@Al2Klimov Al2Klimov requested a review from yhabteab May 8, 2024 10:07
@Al2Klimov Al2Klimov self-assigned this Aug 20, 2024
oxzi
oxzi reviewed Sep 10, 2024
View reviewed changes
Copy link
Member

@oxzi oxzi left a comment

Choose a reason for hiding this comment

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

I like this feature and think it might help users if we document it. Thus, unless there are technical limitations, I would like to see it in the manual.

doc/15-troubleshooting.md Outdated
Comment on lines 184 to 187
at runtime. This is a good alternative to `icinga2 feature enable debuglog`:

* Object creation/deletion via API happens immediately and requires no restart
* Hence, the debug log is enabled exactly as long as desired
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
at runtime. This is a good alternative to `icinga2 feature enable debuglog`:
* Object creation/deletion via API happens immediately and requires no restart
* Hence, the debug log is enabled exactly as long as desired
at runtime.
This is a good alternative to `icinga2 feature enable debuglog` as object
creation/deletion via API happens immediately and requires no restart.

I would just continue with text, as the list makes it harder to read. Furthermore, the second bullet point adds no real value, imo.

Copy link
Member Author

Choose a reason for hiding this comment

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

the second bullet point adds no real value, imo.

Well, if your reload takes 1/2h, your debug log is enabled for 1/2h longer than necessary, spamming the disk. And if your setup is that large, it will spam a lot.

Copy link
Member

Choose a reason for hiding this comment

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

This totally makes sense. Could you then please add this information to the documentation, because for me this was not the obvious intent of the second sentence.

Btw, I am in favor of this PR and come back to it occasionally, for example, just this morning.

Copy link
Member

Choose a reason for hiding this comment

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

As stated above, the bullet list is hard to read. The second point is obsolete with the just introduced paragraph below. Please remove it.

@Al2Klimov Al2Klimov requested a review from oxzi October 14, 2024 11:11
oxzi
oxzi reviewed Oct 15, 2024
View reviewed changes
doc/15-troubleshooting.md Outdated Show resolved Hide resolved
doc/15-troubleshooting.md Outdated
Comment on lines 184 to 187
at runtime. This is a good alternative to `icinga2 feature enable debuglog`:

* Object creation/deletion via API happens immediately and requires no restart
* Hence, the debug log is enabled exactly as long as desired
Copy link
Member

Choose a reason for hiding this comment

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

As stated above, the bullet list is hard to read. The second point is obsolete with the just introduced paragraph below. Please remove it.

@Al2Klimov Al2Klimov removed the request for review from yhabteab October 23, 2024 09:03
@Al2Klimov Al2Klimov marked this pull request as draft October 23, 2024 09:03
@Al2Klimov Al2Klimov marked this pull request as ready for review October 23, 2024 12:51
oxzi
oxzi reviewed Oct 24, 2024
View reviewed changes
Copy link
Member

@oxzi oxzi left a comment

Choose a reason for hiding this comment

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

Just this small nitpick of a double space, otherwise I am happy with it.

doc/15-troubleshooting.md Outdated Show resolved Hide resolved
oxzi
oxzi reviewed Oct 24, 2024
View reviewed changes
doc/15-troubleshooting.md Outdated Show resolved Hide resolved
@Al2Klimov Al2Klimov mentioned this pull request Oct 24, 2024
Merged
@Al2Klimov @oxzi
Document how to enable/disable Debug Output on the fly
e889528 
This is a good alternative to `icinga2 feature enable debuglog`:

* Object creation/deletion via API happens immediately and requires no restart
* Hence, the debug log is enabled exactly as long as desired

Co-authored-by: alvar <[email protected]>
@Al2Klimov Al2Klimov merged commit 65a642d into master Oct 30, 2024
27 checks passed
@Al2Klimov Al2Klimov deleted the Al2Klimov-patch-3 branch October 30, 2024 15:18
@Al2Klimov Al2Klimov mentioned this pull request Oct 30, 2024
Open
2 tasks
@Al2Klimov Al2Klimov added this to the 2.15.0 milestone Nov 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yhabteab yhabteab yhabteab approved these changes

@oxzi oxzi oxzi approved these changes

Assignees

@Al2Klimov Al2Klimov

Labels
area/api REST API area/configuration DSL, parser, compiler, error handling area/documentation End-user or developer help area/log Logging related cla/signed consider backporting Should be considered for inclusion in a bugfix release enhancement New feature or request
Projects
None yet
Milestone
2.15.0
Development

Successfully merging this pull request may close these issues.
3 participants
@Al2Klimov @oxzi @yhabteab