Skip to content

Comments

Add condition for HoststateSummary and ServicestateSummary in VolatileStateResults#1335

Open
moreamazingnick wants to merge 1 commit intoIcinga:mainfrom
RISE-GmbH:fix/tactical-overview-export-refs#1334
Open

Add condition for HoststateSummary and ServicestateSummary in VolatileStateResults#1335
moreamazingnick wants to merge 1 commit intoIcinga:mainfrom
RISE-GmbH:fix/tactical-overview-export-refs#1334

Conversation

@moreamazingnick
Copy link

refs #1334

The $query->getModel() instanceof Host evaluates to true since HoststateSummary extends Host.
This leads to an exception in the second part of the condition in the array_intersect

Error in /usr/share/icingaweb2/modules/icingadb/library/Icingadb/Redis/VolatileStateResults.php:54 with message: Object of class ipl\Sql\Expression could not be converted to string
#0 /usr/share/icingaweb2/modules/icingadb/library/Icingadb/Redis/VolatileStateResults.php(54): array_intersect()
#1 /usr/share/icinga-php/ipl/vendor/ipl/orm/src/Query.php(672): Icinga\Module\Icingadb\Redis\VolatileStateResults::fromQuery()
#2 /usr/share/icingaweb2/modules/icingadb/library/Icingadb/Data/JsonResultSetUtils.php(91): ipl\Orm\Query->execute()
#3 /usr/share/icingaweb2/modules/icingadb/library/Icingadb/Web/Controller.php(393): Icinga\Module\Icingadb\Data\JsonResultSet::stream()
#4 /usr/share/icingaweb2/modules/icingadb/application/controllers/TacticalController.php(54): Icinga\Module\Icingadb\Web\Controller->export()
#5 /usr/share/icingaweb2/modules/icingadb/library/Icingadb/Web/Controller.php(424): Icinga\Module\Icingadb\Controllers\TacticalController->indexAction()
#6 /usr/share/php/Icinga/Web/Controller/Dispatcher.php(76): Icinga\Module\Icingadb\Web\Controller->dispatch()
#7 /usr/share/icinga-php/vendor/vendor/shardj/zf1-future/library/Zend/Controller/Front.php(954): Icinga\Web\Controller\Dispatcher->dispatch()
#8 /usr/share/php/Icinga/Application/Web.php(294): Zend_Controller_Front->dispatch()
#9 /usr/share/php/Icinga/Application/webrouter.php(105): Icinga\Application\Web->dispatch()
#10 /usr/share/icingaweb2/public/index.php(4): require_once(String)
#11 {main}

This is only one quickfix and there might be better fixes for that issue like

if (get_class($query->getModel()) === Host::class) {
    // It is exactly Host, not a subclass
}

Best Regards
Nicolas

@cla-bot cla-bot bot added the cla/signed CLA is signed by all contributors of a PR label Feb 19, 2026
@moreamazingnick
Copy link
Author

but this only solves half of it because here:

// It only makes sense to export a single result to CSV or JSON

$query = $queries[0];

we will only export the first query which renders
yield $this->export($hoststateSummary, $servicestateSummary);
useless

@moreamazingnick
Copy link
Author

but for that I have some approaches:
change function
from

JsonResultSetUtils->stream(Query $query): void

to

JsonResultSetUtils->stream(Query ...$queries): void

and allow multiple query to be merged to one json
this would look like this:

[
   {
      "hosts_acknowledged":"0",
      "hosts_active_checks_enabled":"18",
      "hosts_passive_checks_enabled":"19",
      "hosts_down_handled":"0",
      "hosts_down_unhandled":"0",
      "hosts_event_handler_enabled":"19",
      "hosts_flapping_enabled":"0",
      "hosts_notifications_enabled":"19",
      "hosts_pending":"0",
      "hosts_problems_unacknowledged":"0",
      "hosts_total":"19",
      "hosts_up":"19"
   },
   {
      "services_acknowledged":"1",
      "services_active_checks_enabled":"71",
      "services_passive_checks_enabled":"71",
      "services_critical_handled":"0",
      "services_critical_unhandled":"1",
      "services_event_handler_enabled":"71",
      "services_flapping_enabled":"0",
      "services_notifications_enabled":"71",
      "services_ok":"69",
      "services_pending":"0",
      "services_problems_unacknowledged":"1",
      "services_total":"71",
      "services_unknown_handled":"0",
      "services_unknown_unhandled":"0",
      "services_warning_handled":"1",
      "services_warning_unhandled":"0"
   }
]

but this will not work with CsvResultSetUtils->stream(Query $query)

Another idea would be to introduce a combined StateSummary that wraps both together and a FakeQuery class that simply unions the two queries.

or the TacticalController gets it's own export function that takes care of the merge

@nilmerg
Copy link
Member

nilmerg commented Feb 20, 2026

Indeed, your current proposal isn't quite right. The fact that VolatileStateResults is even used is the problem. You might solve both issues by combining both summaries in a UnionModel, just like it's been done for the HostgroupSummary.

Adjusting which query is exported isn't an option right now, as there are views that call export with two queries although only one is relevant for json and csv. That's currently by design.

@moreamazingnick
Copy link
Author

moreamazingnick commented Feb 22, 2026

I don't what to make this merge request too messy so this would be my adaption:

TacticalController:

        $stateSummary = Statesummary::on($db);

        $this->filter($hoststateSummary, $filter);
        $this->filter($servicestateSummary, $filter);
        $this->filter($stateSummary, $filter);
        yield $this->export($stateSummary);

StateSummary.php

<?php

/* Icinga DB Web | (c) 2020 Icinga GmbH | GPLv2 */

namespace Icinga\Module\Icingadb\Model;

use Icinga\Module\Icingadb\Common\Auth;
use ipl\Orm\Query;
use ipl\Orm\UnionModel;
use ipl\Sql\Adapter\Pgsql;
use ipl\Sql\Connection;
use ipl\Sql\Expression;
use ipl\Sql\Select;


class Statesummary extends UnionModel
{
    public static function on(Connection $db)
    {
        $q = parent::on($db);

        $q->on(
            Query::ON_SELECT_ASSEMBLED,
            function () use ($q) {
                $auth = new class () {
                    use Auth;
                };

                $auth->assertColumnRestrictions($q->getFilter());
            }
        );

        $q->on($q::ON_SELECT_ASSEMBLED, function (Select $select) use ($q) {
            $model = $q->getModel();

            $groupBy = $q->getResolver()->qualifyColumnsAndAliases((array)$model->getKeyName(), $model, false);

            // For PostgreSQL, ALL non-aggregate SELECT columns must appear in the GROUP BY clause:
            if ($q->getDb()->getAdapter() instanceof Pgsql) {
                /**
                 * Ignore Expressions, i.e. aggregate functions {@see getColumns()},
                 * which do not need to be added to the GROUP BY.
                 */
                $candidates = array_filter($select->getColumns(), 'is_string');
                // Remove already considered columns for the GROUP BY, i.e. the primary key.
                $candidates = array_diff_assoc($candidates, $groupBy);
                $groupBy = array_merge($groupBy, $candidates);
            }

            $select->groupBy($groupBy);
        });

        return $q;
    }

    public function getTableName()
    {
        return "host";
    }

    public function getKeyName()
    {
        return "hosts_acknowledged";
    }

    public function getColumns()
    {
        return $this->getModifiedColumns(['HoststateSummary' => 'SUM', 'ServicestateSummary' => 'SUM']);

    }

    public function getSearchColumns()
    {
    }

    public function getDefaultSort()
    {
    }

    public function getModifiedColumns($config)
    {
        $modifiedColumns = [];

        foreach ($config as $model => $expression) {
            if ($model === "ServicestateSummary") {
                $columns = (new ServicestateSummary())->getSummaryColumns();
            } elseif ($model === "HoststateSummary") {
                $columns = (new HoststateSummary())->getSummaryColumns();
            } else {
                throw new \Exception("Unsupported Model");
            }


            foreach ($columns as $name => $value) {
                if ($expression === "0") {
                    $modifiedColumns[$name] = new Expression('0');
                } elseif ($expression === "SUM") {
                    $modifiedColumns[$name] = new Expression(
                        sprintf('SUM(%s)', $name)
                    );

                } else {
                    $modifiedColumns[$name] = $name;
                }
            }
        }


        return $modifiedColumns;
    }


    public function getUnions()
    {
        $unions = [
            [
                HoststateSummary::class,
                [

                ],
                $this->getModifiedColumns(['HoststateSummary' => 'name', 'ServicestateSummary' => '0'])
            ],
            [
                ServicestateSummary::class,
                [

                ],
                $this->getModifiedColumns(['HoststateSummary' => '0', 'ServicestateSummary' => 'name'])
            ],

        ];

        return $unions;
    }


}

I needed to use a table and a keyname but I think this will not be used in query.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla/signed CLA is signed by all contributors of a PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants