I'm concerned a bit about mixing two terms - "recovery" and "OK".
Users can mix them easily. We have several issues in Jira where this topic being discussed. ZBX-7615, ZBXNEXT-452 and others.

Now we use the "recovery" term only as related to action.
There we can, say, define will it be an OK or Recovery alert.

I'd recommend to avoid usage the "Recovery" term at trigger level and now use only "OK".
Otherwise it will complicate understanding even more.

Another way is to get rid of OK events at all and call everything as Recovery.

In real life to have zabbix working logically (if we take into account escalations and maintenance), we have to use condition "Trigger value = PROBLEM" with "Recovery message" enabled.

Upgrade patch was successfully tested!

(1) Server side ready for testing in development branch svn://svn.zabbix.com/branches/dev/ZBXNEXT-2118.

sasha CLOSED

(2) [S] templates linkage does not support second trigger expression

wiper RESOLVED in r59339

sasha CLOSED

(3) [S] low-level discovery does not support second trigger expression

wiper RESOLVED

sasha CLOSED

(4) [F] Trigger.get contain expandExpression option, which rewrite expression, as discussed with sasha, now for recovery_expression leave as is, and in the future should be another output and flags for expressions.

sasha We decided to use this option for expanding recovery_expression together with expression.

RESOLVED in r59550 and r59552

iivs CLOSED

(5) [S] Take a look at my changes in r59270.

wiper Thanks, CLOSED

(6) [S] comments are not updated for function add_event() and process_trigger()

wiper RESOLVED in r59272

sasha CLOSED with minor formatting in r59280

(7) [S] memory leaks:

• recovery_expression is not cleared by clean_events()
• recovery_expression is not cleared by free_event_info()

wiper RESOLVED r59273

sasha CLOSED

(8) [S] extract_expression_functionids():

• can be rewritten to use const char *expression instead of char *expression. is_uint64_n() should be used here.
• this code will be useless after resolving (10)

  if (NULL == expression)
          return SUCCEED;

wiper good idea, RESOLVED r59275, r59281

sasha CLOSED with changes in r59282.

(9) [S] evaluate_expressions() and zbx_substitute_functions_results():

• if (NULL != tr->expression) checks are useless in these functions
• if (NULL != tr->recovery_expression) checks must be replaced by if (TRIGGER_RECOVERY_MODE_RECOVERY_EXPRESSION == tr->recovery_mode)

wiper RESOLVED in r59277

sasha CLOSED with changes in r59284.

(10) [S] zbx_extract_functionids():

• if (TRIGGER_RECOVERY_MODE_RECOVERY_EXPRESSION == tr->recovery_mode) check must be added to process recovery_expression

wiper RESOLVED in r59276

sasha CLOSED with changes in r59283.

(10) [S] evaluate_expressions():

• if (TRIGGER_RECOVERY_MODE_RECOVERY_EXPRESSION == tr->recovery_mode) check is useless. Can be removed.
• this code can be removed

  memset(&event, 0, sizeof(DB_EVENT));
  event.source = EVENT_SOURCE_TRIGGERS;

wiper RESOLVED in r59278

sasha CLOSED

(11) [S] Take a look at my changes in r59280, r59282, r59283 and r59284

wiper Thanks, already did, CLOSED

(12) [S] lld_triggers_equal() returns SUCCEED if expression OR recovery_expression are equal. Both expressions must be equal.

wiper RESOLVED in r59309

sasha an alternative solution in r59312. take a look!

wiper CLOSED

(13) When linking templates server attempts to link existing triggers to template triggers. The match is done by comparing expressions and recovery expressions, but recovery mode is ignored. This means the recovery mode must be updated when triggers are linked to avoid situation, when host trigger is linked to template trigger with different recovery mode.

wiper RESOLVED in r59403

sasha CLOSED

(14) [AF] defines TRIGGER_REC_MODE_* must be renamed according to the specification

sasha RESOLVED in r59660.

iivs CLOSED

(15) [A] Trigger::validateItems() throws exception for an empty recovery_expression.

sasha RESOLVED in r59430, r59449, r59462, r59466, r59470, r59471, r59475, r59476 and r59479

iivs CLOSED

(16) [A] Trigger::validateItems() must validate all templates from both expressions together.

sasha RESOLVED in r59430, r59449, r59462, r59466, r59470, r59471, r59475, r59476 and r59479

iivs CLOSED

(17) [A] Trigger::create() without description throws PHP error:

Undefined index: description [in include/classes/api/services/CTrigger.php:533]

$this->checkNoParameters(
    $trigger,
    ['templateid', 'state', 'value'],
    ($update ? _('Cannot update "%1$s" for trigger "%2$s".') : _('Cannot set "%1$s" for trigger "%2$s".')),
    $trigger['description']
);

if (!check_db_fields($triggerDbFields, $trigger)) {
    self::exception(ZBX_API_ERROR_PARAMETERS, _('Incorrect fields for trigger.'));
}

The call of check_db_fields() must be before $this->checkNoParameters().

sasha RESOLVED in r59430, r59449, r59462, r59466, r59470, r59471, r59475, r59476 and r59479

iivs CLOSED

(18) [AF] Macros in recovery_expression must be mass-resolved

sasha RESOLVED in r59430

iivs CLOSED

(19) [F] The "PROBLEM event generation mode" switches from "Single" to "Multiple" on save. When adding and editing it always saves it as "Multiple". It's like a silent silent operation.

oleg.egorov RESOLVED IN r59484

iivs CLOSED

(20) [F] In trigger edit form the recovery expression gets replaced after trying to add another one and the end.

oleg.egorov RESOLVED in r59656

iivs When editing expression via constructor the item is not selected in popup.

REOPENED

oleg.egorov RESOLVED in r59687

iivs CLOSED

(21) [F] Recovery expression constructor breaks some of the normal expression constructor functionallity. Open expression constructor and see the buttons "And", "Or", "Replace" and "Test". As soon as recovery expression constructor is opened, the buttons dissapear and now there is only "Add" and "Test" is grayed out. Both constructors should work independently. And in templated trigger when opening expression constructor, the recovery mode changes and recovery expression field dissapears.

oleg.egorov RESOLVED in r59669

iivs CLOSED

(22) [F] It's said in specification that "Edit" menu for trigger wizard will only be show for triggers having one expression. That includes having no recovery expression. Currently I created a trigger with two expressions:

Problem: (({T1:it1.6.regexp(test4)})<>0)
Recovery: (({T1:it1.6.regexp(test5)})<>0)

and it "Edit" entry is available in menu.

oleg.egorov RESOLVED in r59579

iivs CLOSED

(23) [F] According to specification the label "Expression" should change to "Problem expression" when "Recovery expression" is selected.

oleg.egorov RESOLVED IN r59495

iivs The fix is wrong. It should change only when "Recovery expression" is selected. Otherwise it should stay the same.

REOPENED

oleg.egorov RESOLVED in r59551

iivs Due to having duplicate IDs on page, the JS doesn't work properly.

REOPENED

oleg.egorov RESOLVED in r59580

iivs

• When expression constructor is open, the label for Expression/Problem expression does not change any more.
• When recovery expression constructor is open, chaning mode the block remains visible.
• Trigger prototype expression constructor "Remove" doesn't remove anything (Type is undefined in JS function).

REOPENED

oleg.egorov RESOLVED in r59696 and r59697

iivs CLOSED

(24) [F] Switching from "Recovery expression" to "Expression" or "None" it's possible to get error: "Incorrect value for field "recovery_expression": should be empty." For API validation that's good, but for frontend the message is wrong. Happens when trying to create a trigger with same name, expression.

oleg.egorov RESOLVED in r59496

iivs CLOSED

(25) [F] Having a mode selected "Expression" or "None" for templated triggers, the "Recovery expression" textarea, although read only, it's visible to user.

oleg.egorov RESOLVED in r59517

iivs Related problem: when opening expression constructor, it switches the event generation more. Fix will be under (21).

CLOSED

(26) [F] The button "Add"/"Edit" is active near recovery expression for templated triggers.

oleg.egorov RESOLVED in r59505

iivs CLOSED

(27) [F] Frontend controller uses hardcoded values 0, 1 and 2 instead of constants for "recovery_mode" field.

oleg.egorov RESOLVED in r59501

iivs CLOSED

Performance improvements of trigger.create() and trigger.update() methods.

Import all standard templates into an empty database:

@trunk r59419

******************** Script profiler ********************
Total time: 8.459436
Total SQL time: 2.401958
SQL count: 13757 (selects: 9329 | executes: 4428)
Peak memory usage: 419.5M

@development branch r59505

******************** Script profiler ********************
Total time: 4.833837
Total SQL time: 1.586428
SQL count: 8226 (selects: 4478 | executes: 3748)
Peak memory usage: 258.5M

Update all standard templates.

@trunk r59419

******************** Script profiler ********************
Total time: 7.571569
Total SQL time: 2.183881
SQL count: 10606 (selects: 7928 | executes: 2678)
Peak memory usage: 340M

@development branch r59505

******************** Script profiler ********************
Total time: 3.95199
Total SQL time: 1.248854
SQL count: 5995 (selects: 3809 | executes: 2186)
Peak memory usage: 198.25M

(28) [AF] Having a simple expression that belongs to one template, but in recovery expression we can assign a different template, in the end that trigger belongs to both templates. Suppose we have T1 and T2 template. T1 and T2 is linked to a host H1. On template T1, add dependencies from H1 which are actually templated triggers.

 template_trigger_1onT1
     Depends on:
     H1: template_trigger_2onT1 (exp) and template_trigger_1onT2 (rec exp)
     H1: template_trigger_2onT2 (rec exp) and template_trigger_3onT1 (exp)

We can export host and import it back with no problem. Without any changes or deletion, export this template and now it's not possible to import it back. Even exporting both templates T1 and T2 it's not possible. Error: Trigger "template_trigger_1onT1" depends on trigger "template_trigger_2onT1 (exp) and template_trigger_1onT2 (rec exp)", which does not exist.". No changes were made to host and triggers still exist there.

iivs Probably the fault is somewhere in export, since recovery expression in XML file looks like this {13288}=0

sasha Actually has been broken XML export. Thanks for a report!

RESOLVED in r59546

iivs Same error message when trigger prototype depends on a regular trigger. Tigger prototype is on a template and dependency is on a host.

REOPENED

sasha It is reproducible in 3.0 branch. An independent issue must be created.

Error message:

Trigger prototype "trigger prototype" depends on trigger "trigger2", which does not exist. [conf.import.php:173 → CFrontendApiWrapper->import() → CApiWrapper->__call() → CFrontendApiWrapper->callMethod() → CApiWrapper->callMethod() → CFrontendApiWrapper->callClientMethod() → CLocalApiClient->callMethod() → call_user_func_array() → CConfiguration->import() → CConfigurationImport->import() → CConfigurationImport->processDiscoveryRules() → CConfigurationImport->processTriggerPrototypeDependencies() in include/classes/import/CConfigurationImport.php:1132]

iivs Thanks for looking into this. I created an issue for those two cases ZBX-10693

CLOSED

(29) [F] Full-clone of template or host is broken:

Cannot set "value" for trigger prototype "Free disk space is less than 20% on volume {#FSNAME}". [hosts.php:605 → CFrontendApiWrapper->copy() → CApiWrapper->__call() → CFrontendApiWrapper->callMethod() → CApiWrapper->callMethod() → CFrontendApiWrapper->callClientMethod() → CLocalApiClient->callMethod() → call_user_func_array() → CDiscoveryRule->copy() → CDiscoveryRule->copyDiscoveryRule() → CDiscoveryRule->copyTriggerPrototypes() → CTriggerPrototype->create() → CTriggerGeneral->validateCreate() → CApiService->checkNoParameters() → CApiService::exception() in include/classes/api/CApiService.php:790]

oleg.egorov RESOLVED in r59680 and r59683

iivs CLOSED

(30) [F] Recovery expression is not displayed in item popup and event details.

oleg.egorov RESOLVED in r59611

iivs CLOSED

(31) [F] Recovery expression threshold not displayed in graphs.

oleg.egorov As discussed, will be not changed current functionality.
And I can't reproduce other issues in graphs, all seems well.

CLOSED

Server side was successfully tested!

(32) [F] Full-clone of template or host does not populate recovery_mode and recovery_expression in the new triggers.

oleg.egorov RESOLVED IN r59677

iivs Error: "Cannot set "value" for trigger prototype "tp5.1"." Will be fixed under (29).

CLOSED

(33) [AF] Fixed several coding style issues while making code review. Please see my changes in r59676

oleg.egorov REVIEWED, but please review my minor changes

iivs CLOSED

(34) [F] Translation string changes? (Would be nice to sync them with specification too after the list is complete).

Strings added:

• Cannot set "%1$s" for trigger prototype "%2$s".
• Cannot update "%1$s" for templated trigger "%2$s".
• Cannot update "%1$s" for templated trigger prototype "%2$s".
• Cannot update "%1$s" for trigger prototype "%2$s".
• Cannot update trigger "%1$s": %2$s.
• Cannot update trigger prototype "%1$s": %2$s.
• Expression syntax error.
• Multiple
• OK event generation
• PROBLEM event generation mode
• Problem expression
• Recovery expression
• Recovery expression syntax error.
• Single
• Trigger "%1$s" belongs to templates with different linkages.
• Trigger "%1$s" depends on trigger "%2$s", which does not exist.
• Trigger prototype "%1$s" already exists on "%2$s".
• Trigger prototype "%1$s" belongs to templates with different linkages.
• Trigger recovery expression must contain at least one host:key reference.
• Wrong fields for trigger prototype.
• should be empty
• trigger prototype cannot be moved to another template or host
• trigger with linkages cannot be moved to another template or host

Strings deleted:

• Cannot implode expression "%s".
• Cannot set "templateid" for trigger prototype "%1$s".
• Created: Trigger "%1$s" on "%2$s".
• Created: Trigger prototype "%1$s" on "%2$s".
• Expression Syntax Error.
• Incorrect fields for trigger.
• Incorrect trigger expression. Host "%s" does not exist or you have no access to this host.
• Multiple PROBLEM events generation
• Trigger "%1$s" depends on trigger "%2$s", which does not exist.
• Trigger "%s" belongs to templates with different linkages.
• Trigger just added. No status update so far.
• Updated: Trigger "%1$s" on "%2$s".
• Updated: Trigger prototype "%1$s" on "%2$s".

sasha RESOLVED

Strings deleted in r59786:

• Cannot find item "%1$s" on "%2$s" used in trigger "%3$s".
• Cannot find item "%1$s" on "%2$s" used in trigger prototype "%3$s" of discovery rule "%4$s" on "%5$s".

iivs CLOSED

Available in pre-3.1.0 (trunk) r59709. Import of the XML v1.0-3.0 was fixed in r59786.

(37) [A]

Undefined index: recovery_expression [conf.import.php:173 → CFrontendApiWrapper->import() → CApiWrapper->__call() → CFrontendApiWrapper->callMethod() → CApiWrapper->callMethod() → CFrontendApiWrapper->callClientMethod() → CLocalApiClient->callMethod() → call_user_func_array() → CConfiguration->import() → CConfigurationImport->import() → CConfigurationImport->gatherReferences() in include\classes\import\CConfigurationImport.php:249]
Undefined index: recovery_expression [conf.import.php:173 → CFrontendApiWrapper->import() → CApiWrapper->__call() → CFrontendApiWrapper->callMethod() → CApiWrapper->callMethod() → CFrontendApiWrapper->callClientMethod() → CLocalApiClient->callMethod() → call_user_func_array() → CConfiguration->import() → CConfigurationImport->import() → CConfigurationImport->gatherReferences() in include\classes\import\CConfigurationImport.php:332]
Undefined index: recovery_expression [conf.import.php:173 → CFrontendApiWrapper->import() → CApiWrapper->__call() → CFrontendApiWrapper->callMethod() → CApiWrapper->callMethod() → CFrontendApiWrapper->callClientMethod() → CLocalApiClient->callMethod() → call_user_func_array() → CConfiguration->import() → CConfigurationImport->import() → CConfigurationImport->processTriggers() in include\classes\import\CConfigurationImport.php:1314]
Undefined index: recovery_expression [conf.import.php:173 → CFrontendApiWrapper->import() → CApiWrapper->__call() → CFrontendApiWrapper->callMethod() → CApiWrapper->callMethod() → CFrontendApiWrapper->callClientMethod() → CLocalApiClient->callMethod() → call_user_func_array() → CConfiguration->import() → CConfigurationImport->import() → CConfigurationImport->processTriggers() in include\classes\import\CConfigurationImport.php:1342]
Undefined index: recovery_expression [conf.import.php:173 → CFrontendApiWrapper->import() → CApiWrapper->__call() → CFrontendApiWrapper->callMethod() → CApiWrapper->callMethod() → CFrontendApiWrapper->callClientMethod() → CLocalApiClient->callMethod() → call_user_func_array() → CConfiguration->import() → CConfigurationImport->import() → CConfigurationImport->processTriggers() → CConfigurationImport->processTriggerDependencies() in include\classes\import\CConfigurationImport.php:1374]
Undefined index: recovery_expression [conf.import.php:173 → CFrontendApiWrapper->import() → CApiWrapper->__call() → CFrontendApiWrapper->callMethod() → CApiWrapper->callMethod() → CFrontendApiWrapper->callClientMethod() → CLocalApiClient->callMethod() → call_user_func_array() → CConfiguration->import() → CConfigurationImport->import() → CConfigurationImport->processTriggers() → CConfigurationImport->processTriggerDependencies() in include\classes\import\CConfigurationImport.php:1379]
Undefined index: recovery_expression [conf.import.php:173 → CFrontendApiWrapper->import() → CApiWrapper->__call() → CFrontendApiWrapper->callMethod() → CApiWrapper->callMethod() → CFrontendApiWrapper->callClientMethod() → CLocalApiClient->callMethod() → call_user_func_array() → CConfiguration->import() → CConfigurationImport->import() → CConfigurationImport->processDiscoveryRules() in include\classes\import\CConfigurationImport.php:1029]
Undefined index: recovery_expression [conf.import.php:173 → CFrontendApiWrapper->import() → CApiWrapper->__call() → CFrontendApiWrapper->callMethod() → CApiWrapper->callMethod() → CFrontendApiWrapper->callClientMethod() → CLocalApiClient->callMethod() → call_user_func_array() → CConfiguration->import() → CConfigurationImport->import() → CConfigurationImport->processDiscoveryRules() in include\classes\import\CConfigurationImport.php:1123]
Undefined index: recovery_expression [conf.import.php:173 → CFrontendApiWrapper->import() → CApiWrapper->__call() → CFrontendApiWrapper->callMethod() → CApiWrapper->callMethod() → CFrontendApiWrapper->callClientMethod() → CLocalApiClient->callMethod() → call_user_func_array() → CConfiguration->import() → CConfigurationImport->import() → CConfigurationImport->processDiscoveryRules() → CConfigurationImport->processTriggerPrototypeDependencies() in include\classes\import\CConfigurationImport.php:1158]
Undefined index: recovery_expression [conf.import.php:173 → CFrontendApiWrapper->import() → CApiWrapper->__call() → CFrontendApiWrapper->callMethod() → CApiWrapper->callMethod() → CFrontendApiWrapper->callClientMethod() → CLocalApiClient->callMethod() → call_user_func_array() → CConfiguration->import() → CConfigurationImport->import() → CConfigurationImport->processDiscoveryRules() → CConfigurationImport->processTriggerPrototypeDependencies() in include\classes\import\CConfigurationImport.php:1163]

The import was done from 3.0 to 3.2.

sasha

Strings deleted:

• Cannot find item "%1$s" on "%2$s" used in trigger "%3$s".
• Cannot find item "%1$s" on "%2$s" used in trigger prototype "%3$s" of discovery rule "%4$s" on "%5$s".

RESOLVED in r59768

iivs Looks good. Please see my minor changes in r59784

sasha Great! CLOSED

Fixed in pre-3.1.0 (trunk) r59786.

(38) Documentation updates:

• https://www.zabbix.com/documentation/3.2/manual/introduction/whatsnew320#easier_trigger_hysteresis
• https://www.zabbix.com/documentation/3.2/manual/config/triggers/trigger
• https://www.zabbix.com/documentation/3.2/manual/config/triggers/expression#hysteresis
• https://www.zabbix.com/documentation/3.2/manual/appendix/macros/supported_by_location (TRIGGER.EXPRESSION.RECOVERY added)

Noting that, with more changes expected to trigger configuration, the screenshots will have to be updated again later.

RESOLVED

sasha Great work! CLOSED