thin-edge / thin-edge.io

The open edge framework for lightweight IoT devices
https://thin-edge.io
Apache License 2.0
221 stars 54 forks source link

Use c8y operation ID for operation status update #2616

Closed rina23q closed 1 month ago

rina23q commented 9 months ago

Is your feature improvement request related to a problem? Please describe. JSON over MQTT input (#1718) enabled to use c8y's operation ID as thin-edge commend ID. Reporting c8y's operation status with operation ID has benefits over ID-less operation status update via SmartREST. If there are concurrent operations with the same operation type, without operation ID may cause an issue, that is, updating unintended operation.

Describe the solution you'd like However, it's still not clear what is the best solution. We need investigation.

  1. JSON over MQTT As long as I read the c8y's user guide, the endpoint seems not available for updating operations.
  2. REST API We already use REST API to report device's software list. REST API has flexibility, however, some operations require us to know which field name must be used. For example, c8y_Command operation requires text field to keep the command output.
  3. New SmartREST message with ID Since c8y team is planning to introduce the new SmartREST messages to update operation with ID, if we wait for long time (years?), such SmartREST will be available.

Describe alternatives you've considered

Additional context

d-timotijevic commented 9 months ago

We've encountered a similar issue in our projects using the thin-edge.io configuration plugin. The lack of operation ID in the SmartREST template for acknowledgement sometimes leads to incorrect operation being acknowledged, especially when there are concurrent operations of the same type.

This issue aligns with the problem described here, where an operation ID-less update via SmartREST could potentially cause the update of an unintended operation. It's evident that a more precise method of updating operation status is required.

Is there any possibility of accelerating the development of this new SmartREST template with operation ID?

Additionally, we've noticed that the nomenclature used in the Cumulocity documentation might be a bit confusing. There seems to be a template (506) that sets an operation as successful, including what is termed an 'operation ID,' but in reality, this 'ID' refers to the operation type (c8y_Restart). In other places in the documentation ID is typically understood to refer to a unique attribute, for instance a managed object ID, event ID, etc.

image

reubenmiller commented 9 months ago

We've encountered a similar issue in our projects using the thin-edge.io configuration plugin. The lack of operation ID in the SmartREST template for acknowledgement sometimes leads to incorrect operation being acknowledged, especially when there are concurrent operations of the same type.

Yes the ID-less way of interacting with operations is "easy" for simple use cases but becomes limiting when you start talking about concurrent processing of operations of the same type.

Is there any possibility of accelerating the development of this new SmartREST template with operation ID?

We'll be releasing v1 within the next few weeks, but shortly after that we'll be looking to address this ticket...we'll have to assess all the options available, and the pros/cons of each (e.g. REST vs MQTT vs Custom SmartREST Template)

Additionally, we've noticed that the nomenclature used in the Cumulocity documentation might be a bit confusing. There seems to be a template (506) that sets an operation as successful, including what is termed an 'operation ID,' but in reality, this 'ID' refers to the operation type (c8y_Restart). In other places in the documentation ID is typically understood to refer to a unique attribute, for instance a managed object ID, event ID, etc.

This surprises me as the 504 - 506 template ids are more recent inbuilt SmartREST messages which were initially introduced for users whom don't want to use the ID-less operation handling, however we haven't tried it out with thin-edge.io yet as the Template IDs are only available from a specific Cumulocity IoT version, so it does not solve the problem for everyone...But we'll do a more detailed analysis of the options once we start working on the ticket.

rina23q commented 3 months ago

Design proposal

User can switch how to update operation status, with operation ID (using template 504-506) (supported in 10.18 or later) or without operation ID (using template 501-503) by tedge config.

# true (default), false, auto (auto will be supported in the future)
sudo tedge config set c8y.smartrest.use_operation_id true

References for SmartREST static template

(504-506) https://cumulocity.com/docs/smartrest/mqtt-static-templates/#504

rina23q commented 1 month ago

Resolved by #3076. Now tedge-mapper-c8y uses 504, 505, and 506 templates to update operation status by default. When you want to use 501-503 templates (Cumulocity 10.17 or older), run this and restart tedge-mapper-c8y service.

gligorisaev commented 1 month ago

Review done in the PR implementing the feature, discussed with developer and checked for Flake test. Working as described