tedge alarm does not transform message fragment to text in the tedge-mapper-c8y

[reubenmiller]

Is your feature improvement request related to a problem? Please describe.

The .message fragment in tedge alarms is no longer respected correctly in the tedge-mapper-c8y service. This was caused by merging #1699 (after the official release of 0.9.0, so it does not affect any official releases just yet)

Officially the tedge specification for alarms states that a tedge alarm can have the .message fragment to provide the alarm description. The expectation is the .message fragment is then used by each cloud mapper to map to the cloud providers data model. In the case of Cumulocity IoT, this means using the .message fragment as the .text field when creating a Cumulocity IoT Alarm.

Below is the documented example of the alarm data structure (from the spec)

Topic:   tedge/alarms/<severity-string>/<type-string>
Payload: {
  "message":  <message-string>,
  "status":   <status-string>,  
  "time":     <time-string>
}

The online user documentation seems to be vague on the correct schema, it is only mentioned in an example about publishing data using tedge mqtt pub, here is the example

tedge mqtt pub --retain --qos 1 tedge/alarms/critical/high_temperature '{"message": "Temperature is critical"}'

The tedge-mapper-c8y service is translating the .message fragment to the .text fragment when pushing the alarm to Cumulocity IoT, however since #1699 was merged, which added support for custom fragments in alarms, the translation of the .message field was lost.

Describe the solution you'd like

I am open to how we solve this, e.g. if we remove support for publishing the alarm message on the .message and use the .text field, or we stick with the original spec.

The following solutions are with regarding the handling of an json alarm payload being published on the following topic (for both the main and child device topics):

# main device
tedge/alarms/<severity-string>/<type-string>

# child devices
tedge/alarms/<severity-string>/<type-string>/<child-id>

Option 1: Keep supporting the .message fragment

The tedge-mapper-c8y should transform the .message fragment to the .text fragment, if the user does not already specify the .text field.

The transformation should adhere to the following rules:

If the alarm payload:

• contains only the .message fragment, then the outgoing Cumulocity IoT Alarm json should translate the value from .message to the .text fragment, and the outgoing alarm should not contain the .message fragment.
• contains both the .text and the .message fragment, then the outgoing Cumulocity IoT Alarm json should leave the .text fragment should be left untouched, and any additional fragments should also be left untouched.
• contains only the .text fragment, then it should be used for the outgoing Cumulocity IoT Alarm json .text fragment.
• does not contain neither .text or .message fragments, then a default value for the outgoing Cumulocity IoT Alarm json for the .text fragment should be used following the syntax, Alarm of type '{alarm_type}' raised. Where alarm_type is the .type fragment from the mqtt topic.

Examples of the tranformation (only the .text and .message fragments will be used in the examples, and the .type, .time information will be ignored just for simplicity.

Tedge Alarm (input)	Cumulocity IoT Alarm (output)
{}	{"text": "Alarm of type 'temperature_high' raised"}
{"message": "Temperature high"}	{"text": "Temperature high"}
{"text": "Temperature high"}	{"text": "Temperature high"}
{"message": "Some other text", "text": "Temperature high"}	{"message": "Some other text", "text": "Temperature high"}

Option 2: Remove support for the .message fragment translation
If we remove support for this, then we need to document it properly by doing the following:

• Create a breaking change notice in the documentation to state that the the .message is no longer officially supported
• Remove it from all documentation examples
• Update the spec to reflect the current state
• Create a breaking change text which can be used in the next official release (a short message in the ticket is fine, this message can then be used in the next release notes.

Describe alternatives you've considered

N/A
Additional context

[reubenmiller]

Extended the bug description to include a note that this is only a bug on the current main branch (merged after 0.9.0 release) and does not affect any official releases yet.

[reubenmiller]

The decision has been made to go with Option 1 (re-introduce the .message fragment handling)

Update: Switched to implement Option 2 instead see reasoning

[reubenmiller]

Also the documentation should be updated to better highlight this interaction

• data model
• How to create an alarm

[reubenmiller]

After careful inspection of the previous behaviour (tested in 0.8.1 and 0.7.5), the .message wasn't actually being mapped to the .text fragment when creating the Cumulocity IoT alarm, however from a logical standpoint, the .message (from the tedge alarm scheme) contains the information that Cumulocity IoT expects in the .text fragment (in the outgoing alarm message)

So it definitely makes sense that the outgoing c8y alarm message contains a human readable .text fragment where possible.

[didier-wenzek]

Also the documentation should be updated to better highlight this interaction

We should promote as doc the alarm specs.

And this should be done for all the specs #1547 as proposed here https://github.com/thin-edge/thin-edge.io/discussions/799.

[reubenmiller]

After a further discussion and confusion around alarm handling we will implement "Option 2", where the official tedge alarm spec will change to expect the alarm description being provided in the .text fragment (not .message). The user can still provide additional custom fragments (including .message).

Reason for switching to Option 2

• The .message field actually never really worked, so changing to .text should not break anyone's client implementation
• Simpler logic in the tedge-mapper c8y (less complexity in the mapper)
• The .text fragment name is shorter than .message so there is a slight payload reduction and user convenience (less to type)
• Since Azure and AWS both have very open data structures when it comes to MQTT payload, so technically both could also use the .text fragment for their alarms, so it would align the data structure making any data cloud side on the cloud side slightly more portable to different clouds (as services can expect the data in a similar format).
• Better alignment with the tedge events (implementation), as it also does not support the .message fragment (except as a custom fragment)

[didier-wenzek]

After a further discussion and confusion around alarm handling we will implement "Option 2", where the official tedge alarm spec will change to expect the alarm description being provided in the .text fragment (not .message). The user can still provide additional custom fragments (including .message).

I'm okay with that even if not convinced by the name size argument ;-). I only want to stress that conflicting names and resolution as with option 1 is not something unusual. On the contrary, this is something that will be more an more frequent as thin-edge will integrate more clouds and data sources.

[reubenmiller]

The spec was updated to remove reference to the .message fragment, as the code has been using the .text fragment.

[gligorisaev]

Created test:
#1887