feat: Add device attributes to ApplicationUp messages

[vlasebian]

Summary

References #5879

Changes

• Add attributes field to ApplicationUplink
• Add attributes field to ApplicationUplinkNormalized
• Add locations, version_ids, network_ids and attributes fields to ApplicationJoinAccept
• Add locations, version_ids, network_ids and attributes fields to ApplicationDownlink
• Add locations, version_ids, network_ids and attributes fields to ApplicationDownlinkFailed
• Add locations, version_ids, network_ids and attributes fields to ApplicationInvalidatedDownlinks
• Add locations, version_ids, network_ids and attributes fields to ApplicationServiceData
• Add attributes registry cache implementation
• Implement attributes metadata registry
• Add attributes registry config
• Enrich uplinks with end device attributes

Testing

Steps

• Launch a local TTS instance
• Connect a gateway to the local instance
• Create an application
• Register an end device in the application
• Go to the settings of the end device and add some attributes
• Go to the integrations of the application and create a web hook (e.g. with webhook.site)
• Check the messages in the live data panel of the console
• Check the webhooks sent to the third party

Results

Join-accept:

Uplink:

Regressions

...

Notes for Reviewers

...

Checklist

• Scope: The referenced issue is addressed, there are no unrelated changes.
• Compatibility: The changes are backwards compatible with existing API, storage, configuration and CLI, according to the compatibility commitments in README.md for the chosen target branch.
• Documentation: Relevant documentation is added or updated.
• Testing: The steps/process to test this feature are clearly explained including testing for regressions.
• Infrastructure: If infrastructural changes (e.g., new RPC, configuration) are needed, a separate issue is created in the infrastructural repositories.
• Changelog: Significant features, behavior changes, deprecations and fixes are added to CHANGELOG.md.
• Commits: Commit messages follow guidelines in CONTRIBUTING.md, there are no fixup commits left.

[vlasebian]

@johanstokking @KrishnaIyer there is a note in the implementation plan that I don't get completely:

[...] a new binary compatible variant of ApplicationDownlink [2]. [..]

[2] ApplicationDownlink is also used during submission, and it does not make much sense to allow users to submit these fields while pushing a downlink.

I understand that this change has to be made as it does not make sense to set these fields when the application sends a downlink to the end device (the fields should be set on end device creation and not modified through uplinks or downlinks).

Does this mean that a whole new message must be defined where these fields are included? Something like:

message EnrichedApplicationDownlink {
  ApplicationDownlink downlinks = 1;
  // End device location metadata, set by the Application Server while handling the message.
  map<string, Location> locations = 3;

  // End device version identifiers, set by the Application Server while handling the message.
  EndDeviceVersionIdentifiers version_ids = 4;

  // Network identifiers, set by the Network Server that handles the message.
  NetworkIdentifiers network_ids = 5;

  // Attributes for devices, set by the Application Server while handling the message.
  map<string, string> attributes = 6 [(validate.rules).map = {
    max_pairs: 10,
    keys: {
      string: {
        pattern: "^[a-z0-9](?:[-]?[a-z0-9]){2,}$",
        max_len: 36
      }
    },
    values: {
      string: {max_len: 200}
    }
  }];

  // next: 7
}

And by binary compatible variant, does that mean serialisable?

[johanstokking]

Does this mean that a whole new message must be defined where these fields are included?

No, these can be part of the existing message. We use comments to indicate which component sets them.

Also, we can use pkg/ttnpb.ProhibitFields() to validate incoming requests, e.g. pass the fields that may not be set when pushing/replacing downlink and return the error back to the client if they're part of the field mask.

[vlasebian]

I see that the downlink replace and the downlink push both use the same request which does not contain a field mask:

message DownlinkQueueRequest {
  EndDeviceIdentifiers end_device_ids = 1 [(validate.rules).message.required = true];
  repeated ApplicationDownlink downlinks = 2 [(validate.rules).repeated.max_items = 100000];
}

However, looking through the code, most of the .Set methods have hardcoded paths, so it should be fine not adding a FieldMask and the corresponding ProhibitField call right?

Also, I noticed that the attributes are saved in the entity registry (that belongs to the IS) which is different than the device registry (that belongs to the AS), is that right? I saw that the locations are also saved in the entity registry. I followed the location registry implementation to fetch the attributes, but only implemented the get call as no set should happened for the attributes in the message handlers.

Let me know if this is okay.

[johanstokking]

However, looking through the code, most of the .Set methods have hardcoded paths, so it should be fine not adding a FieldMask and the corresponding ProhibitField call right?

Yes

Also, I noticed that the attributes are saved in the entity registry (that belongs to the IS) which is different than the device registry (that belongs to the AS), is that right? I saw that the locations are also saved in the entity registry. I followed the location registry implementation to fetch the attributes, but only implemented the get call as no set should happened for the attributes in the message handlers.

Indeed this information is stored in IS, and that has been the main reason we didn't put these fields in the upstream messages: it would require an IS roundtrip. In our global deployments, these are cross-continental roundtrips, delaying uplink messages, increasing memory usage, etc.

So indeed, like location, AS should fetch this from IS in a similar way. These should be combined in one call otherwise there are two sequential roundtrips and that's even worse.

[vlasebian]

Okay, got it. I'll refactor the location and attributes calls into one 👍