Icinga / icingaweb2

A lightweight and extensible web interface to keep an eye on your environment. Analyse problems and act on them.
https://icinga.com/get-started/
GNU General Public License v2.0
805 stars 280 forks source link

new line character is beeing removed in the plugin output #4522

Closed bewue closed 2 years ago

bewue commented 3 years ago

In the current version of IcingaWeb2 a new line character (\n) is beeing removed if there are multiple ones inserted in a row (\n\n...). I have to insert 'foo\n\n\nbar' to get one empty line between foo and bar.

The problem exist at least since version 2.9.3. I dont know the exact version but it has to be 2.8.X where this problem has not exist.

Maybe the fix https://github.com/Icinga/icingaweb2/pull/4317/commits/e01f51ffc79da1df152cfeef772f94f36f17f999 is the reason for this problem?

nilmerg commented 3 years ago

I have to insert 'foo\n\n\nbar' to get one empty line between foo and bar.

There's a test in the referenced change which would have failed if that's the case. I also cannot reproduce this either with comments or plugin output.

How are you testing this?

bewue commented 3 years ago

I have a linux shell script. To format the plugin output i insert new line characters. Worked nice with v2.8.X. I still have the correct output on command line but in IcingaWeb2 some new lines are missing.

Is the test successful if the output has one line break less than the input?

        $this->checkOutput(
            'foo\nbar\n\nraboof',
            "foo\nbar\nraboof"
        );
nilmerg commented 3 years ago

I have to insert 'foo\n\n\nbar' to get one empty line between foo and bar.

Ooof, I've read new line previously. Now I get what you're talking about.

Is the test successful if the output has one line break less than the input?

Yes.

I have a linux shell script.

Is this a custom script available online elsewhere? Do you have a similar problem with other check plugins?

bewue commented 3 years ago

Is the test successful if the output has one line break less than the input?

Yes.

Why you are removing the '\n' ?

That seems to be the reason of the problem i am talking about. Has this changed from 2.8.X to 2.9.X ?

nilmerg commented 3 years ago

Why you are removing the '\n' ?

For historical reasons, it seems. That's why I asked for the plugins you experience this with.

Has this changed from 2.8.X to 2.9.X ?

Yes it has. To fix a bug: #4297

bewue commented 3 years ago

Im using by_ssh to run my check scripts on the hosts.

Here is a simple test Check test script:

#!/bin/bash
echo -e "new-line-check\nfoo\n\nbar"

Output in IcingaWeb2 v2.9.3 (the empty line between foo and bar is missing): grafik

Schmuuu commented 3 years ago

We ran into the same issue. We have many custom plugins written in bash, which provide multiple lines of measurement details, which we tried to make easier to read by multiple blocks of content, separated by empty lines. Since these empty lines are removed, the plugin output not only looks bad, but it is more difficult to grasp.

And to note: it is only the Icinga web GUI having this problem. We also forward the service/ plugin output to an alarm console and the output is nicely displayed with correct empty lines there.

Everything was working fine with 2.8.x. And rewriting that many plugins is not really an option.

Would be nice if you could rework the responsible code, and fix both issues: the original one #4297 with powershell and unnecessary line spacing, but also this issue which removes intentional empty lines from bash plugins. Thanks in advance.

carraroj commented 3 years ago

ref/NC/732387

nilmerg commented 3 years ago

@Schmuuu The thing is, we cannot fix both issues. It's either this or that. The previous (fixed) report included literal \n\n which of course lead to output separated by empty lines. While looking at the code responsible to process plugin output, I noticed that there was indeed a case for pairs of subsequent line breaks which should be normalized to a single one. That wasn't working as intended and I made it working. This of course lead to this report, as intentional empty lines get optimized away.

But we cannot differentiate between intended or not. To us it's just a \n right after another one.

I'm also not sure why the powershell output didn't show these empty lines, but the output clearly contained empty lines. If anyone is able to clear this up (@bunghi ?) we may have an easier time deciding that not optimizing any empty lines away is the better fix.

bewue commented 3 years ago

I dont think you should fix this kind of bug in the frontend at least not if then "correct" input lead to unintentional output.

If the powershell script does funny things, you should fix the powershell script.

Schmuuu commented 3 years ago

@nilmerg Yes, I understand the struggle with the identification if multiple line breaks are intentional or not. I just didn't want to say: 'fix my problem and ignore the problem of other people' ;) And I was just thinking about a check: if a plugin output contains at least one single '\n' anywhere, then double line breaks are probably intentional and should not be removed. If there are always only double line breaks ('\n\n'), then it could an issue like with the powershell plugin.

But I agree with bewue actually, weird plugin behavior should be fixed in the plugin rather.

bunghi commented 3 years ago

@Schmuuu The thing is, we cannot fix both issues. It's either this or that. The previous (fixed) report included literal \n\n which of course lead to output separated by empty lines. While looking at the code responsible to process plugin output, I noticed that there was indeed a case for pairs of subsequent line breaks which should be normalized to a single one. That wasn't working as intended and I made it working. This of course lead to this report, as intentional empty lines get optimized away.

But we cannot differentiate between intended or not. To us it's just a \n right after another one.

I'm also not sure why the powershell output didn't show these empty lines, but the output clearly contained empty lines. If anyone is able to clear this up (@bunghi ?) we may have an easier time deciding that not optimizing any empty lines away is the better fix.

What I can say is that my problem was fixed.

Before the fix: image

After the fix: image

The powershell script was not modified in the meantime.

Schmuuu commented 3 years ago

@bunghi Can you maybe show a bit of the plugin, how it is written? Is it powershell script directly executed by Icinga or is there a wrapper script around? How is the output printed or produced? Is there a loop reading previous output and printing it line by line or something like that? If so, how?

At all: I'm wondering if this issue can be caused by Windows formatted line endings ('\r\n') in the plugin. Is this something which could cause that behavior for the powershell plugin?

bunghi commented 3 years ago

@Schmuuu this is how we use the powershell scripts in nsclient.ini file: check_service_state = cmd /c echo scripts\Get-SearchServiceState.ps1; exit $LastExitCode | powershell.exe -command -

In the PS script a variable is defined this way: $out = New-Object PSObject It is then modified by the script in a loop, like this: $out | Add-Member Generation $outtxt and at the end printed: Write-Output $out

bewue commented 3 years ago

@bunghi Lets see what chars are in your output.

Replace Write-Output $out with Write-Output $out | Format-Hex

Please post that output.

bewue commented 2 years ago

The powershell script fix can still take a while, I think. Meanwhile i suggest to revert the erroneous bugfix in Icingaweb2. ^^

nilmerg commented 2 years ago

@bunghi Multiple users now reported this and even some of our support customers. I'll revert the change supposed to fix your problem. You'll have to find out what's up with your powershell plugin (or with nsclient) instead I'm afraid.

bunghi commented 2 years ago

It's fine, I will check the powershell script.

greatflyingsteve commented 5 months ago

There seems to have been a reversion. We are seeing this exact behavior again in icingaweb2 v2.11.3:

Output in shell, executed as icinga user using the exact command line from the CheckCommand def:

CRITICAL: 1 processes have unrecoverably crashed and dumped core!  Open a ticket immediately!
   Process 874528, '/usr/bin/sleep', at 2024-03-13 01:15:39 GMT (24min, 11sec ago)
   ## THIS ALERT WILL NOT SELF-RESET ##

===== CRITICAL Remediations =====
This is a serious problem.  ALL coredumps require investigation by Engineering.
    If you see more than one coredump, from this host or from any other, START WAKING PEOPLE UP.
    Multiple coredumps are an EMERGENCY.

    Check the associated service and ensure it has recovered, and that it hasn't logged any obvious
    errors that need to be handled urgently.  If there's no immediate issue, SSH in and grab the
    stacktrace and associated info:
       `coredumpctl info <PID> <EXECUTABLE>`

    ...and use this to open a new Jira ticket in the TMX queue, along with anything weird that stuck
    out in your immediate checks.  When you've opened the Jira ticket, reset this alert using:
        `/path/to/script --reset`
==========

Output as show in icingaweb2 check output for the exact same system:

CRITICAL: 1 processes have unrecoverably crashed and dumped core!  Open a ticket immediately!
Process 874528, '/usr/bin/sleep', at 2024-03-13 01:15:39 GMT (50min, 28sec ago) ## THIS ALERT WILL NOT SELF-RESET ## ===== CRITICAL Remediations ===== This is a serious problem. ALL coredumps require investigation by Engineering. If you see more than one coredump, from this host or from any other, START WAKING PEOPLE UP. Multiple coredumps are an EMERGENCY. Check the associated service and ensure it has recovered, and that it hasn't logged any obvious errors that need to be handled urgently. If there's no immediate issue, SSH in and grab the stacktrace and associated info: `coredumpctl info ` ...and use this to open a new Jira ticket in the TMX queue, along with anything weird that stuck out in your immediate checks. When you've opened the Jira ticket, reset this alert using: `/path/to/script --reset` ==========

There was a reference earlier in this ticket to #4297, and the issue described here was supposed to have been a bug introduced by the bugfix for that issue. There is also now a duplicate of that initial bug report (see icinga2 #9617), also reported with icingaweb2 v2.11.3.

Any idea when this will actually get fixed in any kind of durable way?

nilmerg commented 5 months ago

Your problem is that the output is identified as being HTML. See #4075