Stop adding a message_id to to-device messages

Currently, we add a `message_id` field to any to-device message that is sent by
a local user to another local user, and then strip it off again before serving
it to the user. This isn't terribly useful, because:

1. it can't be correlated in the sending/receiving clients

2. a single sendToDevice call, or federation EDU, can create multiple to-device
   messages all with the same message id, which again makes them less than
   useful for tracing.

3. It was never wired up for messages received over federation.

So, let's stop doing that, in preparation for something better.

Author: richvdh

synapse/handlers/appservice.py

@@ -578,9 +578,6 @@ async def _get_to_device_messages(
             device_id,
         ), messages in recipient_device_to_messages.items():
             for message_json in messages:
-                # Remove 'message_id' from the to-device message, as it's an internal ID
-                message_json.pop("message_id", None)
-
                 message_payload.append(
                     {
                         "to_user_id": user_id,

synapse/handlers/devicemessage.py

@@ -216,9 +216,6 @@ async def send_device_message(
         """
         sender_user_id = requester.user.to_string()
 
-        message_id = random_string(16)
-        set_tag(SynapseTags.TO_DEVICE_MESSAGE_ID, message_id)
-
         log_kv({"number_of_to_device_messages": len(messages)})
         set_tag("sender", sender_user_id)
         local_messages = {}
@@ -247,7 +244,6 @@ async def send_device_message(
                         "content": message_content,
                         "type": message_type,
                         "sender": sender_user_id,
-                        "message_id": message_id,
                     }
                     for device_id, message_content in by_device.items()
                 }
@@ -267,7 +263,11 @@ async def send_device_message(
 
         remote_edu_contents = {}
         for destination, messages in remote_messages.items():
-            log_kv({"destination": destination})
+            # The EDU contains a "message_id" property which is used for
+            # idempotence. Make up a random one.
+            message_id = random_string(16)
+            log_kv({"destination": destination, "message_id": message_id})
+
             remote_edu_contents[destination] = {
                 "messages": messages,
                 "sender": sender_user_id,

synapse/handlers/sync.py

@@ -1602,13 +1602,6 @@ async def _generate_sync_entry_for_to_device(
                 user_id, device_id, since_stream_id, now_token.to_device_key
             )
 
-            for message in messages:
-                # We pop here as we shouldn't be sending the message ID down
-                # `/sync`
-                message_id = message.pop("message_id", None)
-                if message_id:
-                    set_tag(SynapseTags.TO_DEVICE_MESSAGE_ID, message_id)
-
             logger.debug(
                 "Returning %d to-device messages between %d and %d (current token: %d)",
                 len(messages),