From 1d65292e94077390af0ad9c5ee8cd8b0db9b357c Mon Sep 17 00:00:00 2001 From: Jorik Schellekens Date: Thu, 5 Sep 2019 14:41:04 +0100 Subject: [PATCH 1/5] Link the send loop with the edus contexts The contexts were being filtered too early so the send loop wasn't being linked to them unless the destination was whitelisted. --- synapse/federation/sender/transaction_manager.py | 11 ++++++++--- synapse/federation/units.py | 3 +++ synapse/handlers/devicemessage.py | 5 +---- 3 files changed, 12 insertions(+), 7 deletions(-) diff --git a/synapse/federation/sender/transaction_manager.py b/synapse/federation/sender/transaction_manager.py index 62ca6a3e8..42f46394b 100644 --- a/synapse/federation/sender/transaction_manager.py +++ b/synapse/federation/sender/transaction_manager.py @@ -26,6 +26,7 @@ from synapse.logging.opentracing import ( set_tag, start_active_span_follows_from, tags, + whitelisted_homeserver, ) from synapse.util.metrics import measure_func @@ -59,9 +60,13 @@ class TransactionManager(object): # The span_contexts is a generator so that it won't be evaluated if # opentracing is disabled. (Yay speed!) - span_contexts = ( - extract_text_map(json.loads(edu.get_context())) for edu in pending_edus - ) + span_contexts = [] + keep_destination = whitelisted_homeserver(destination) + + for edu in pending_edus: + span_contexts.append(extract_text_map(json.loads(edu.get_context()))) + if keep_destination: + edu.strip_context() with start_active_span_follows_from("send_transaction", span_contexts): diff --git a/synapse/federation/units.py b/synapse/federation/units.py index aa8462120..b4d743cde 100644 --- a/synapse/federation/units.py +++ b/synapse/federation/units.py @@ -41,6 +41,9 @@ class Edu(JsonEncodedObject): def get_context(self): return getattr(self, "content", {}).get("org.matrix.opentracing_context", "{}") + def strip_context(self): + getattr(self, "content", {})["org.matrix.opentracing_context"] = "{}" + class Transaction(JsonEncodedObject): """ A transaction is a list of Pdus and Edus to be sent to a remote home diff --git a/synapse/handlers/devicemessage.py b/synapse/handlers/devicemessage.py index 01731cb2d..0043cbea1 100644 --- a/synapse/handlers/devicemessage.py +++ b/synapse/handlers/devicemessage.py @@ -25,7 +25,6 @@ from synapse.logging.opentracing import ( log_kv, set_tag, start_active_span, - whitelisted_homeserver, ) from synapse.types import UserID, get_domain_from_id from synapse.util.stringutils import random_string @@ -121,9 +120,7 @@ class DeviceMessageHandler(object): "sender": sender_user_id, "type": message_type, "message_id": message_id, - "org.matrix.opentracing_context": json.dumps(context) - if whitelisted_homeserver(destination) - else None, + "org.matrix.opentracing_context": json.dumps(context), } log_kv({"local_messages": local_messages}) From 93bc9d73bfc3fafa1862ba0cc65fd31bfbb1add9 Mon Sep 17 00:00:00 2001 From: Jorik Schellekens Date: Thu, 5 Sep 2019 14:45:07 +0100 Subject: [PATCH 2/5] newsfile --- changelog.d/5984.bugfix | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/5984.bugfix diff --git a/changelog.d/5984.bugfix b/changelog.d/5984.bugfix new file mode 100644 index 000000000..98dcfb188 --- /dev/null +++ b/changelog.d/5984.bugfix @@ -0,0 +1 @@ +Link send loop opentracing to edu contexts regardles of destination. From 5ade977d0836a9d7615b4e8cc578c48a26198ee8 Mon Sep 17 00:00:00 2001 From: Jorik Schellekens Date: Thu, 5 Sep 2019 15:06:13 +0100 Subject: [PATCH 3/5] Opentracing context cannot be none --- synapse/storage/devices.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/storage/devices.py b/synapse/storage/devices.py index 41f62828b..79a58df59 100644 --- a/synapse/storage/devices.py +++ b/synapse/storage/devices.py @@ -856,7 +856,7 @@ class DeviceStore(DeviceWorkerStore, BackgroundUpdateStore): "ts": now, "opentracing_context": json.dumps(context) if whitelisted_homeserver(destination) - else None, + else "{}", } for destination in hosts for device_id in device_ids From 7093790fbc805162a5ebb36973aa52313aa1153b Mon Sep 17 00:00:00 2001 From: Jorik Schellekens Date: Thu, 5 Sep 2019 15:07:00 +0100 Subject: [PATCH 4/5] Bugfix phrasing Co-Authored-By: Erik Johnston --- changelog.d/5984.bugfix | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelog.d/5984.bugfix b/changelog.d/5984.bugfix index 98dcfb188..3387bf82b 100644 --- a/changelog.d/5984.bugfix +++ b/changelog.d/5984.bugfix @@ -1 +1 @@ -Link send loop opentracing to edu contexts regardles of destination. +Fix sending of EDUs when opentracing is enabled with an empty whitelist. From ef20aa52ebb03b322e72a2fc4fefd21a373593a6 Mon Sep 17 00:00:00 2001 From: Jorik Schellekens Date: Thu, 5 Sep 2019 15:07:17 +0100 Subject: [PATCH 5/5] use access methods (duh..) Co-Authored-By: Erik Johnston --- synapse/federation/sender/transaction_manager.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/synapse/federation/sender/transaction_manager.py b/synapse/federation/sender/transaction_manager.py index 42f46394b..5b6c79c51 100644 --- a/synapse/federation/sender/transaction_manager.py +++ b/synapse/federation/sender/transaction_manager.py @@ -64,7 +64,9 @@ class TransactionManager(object): keep_destination = whitelisted_homeserver(destination) for edu in pending_edus: - span_contexts.append(extract_text_map(json.loads(edu.get_context()))) + context = edu.get_context() + if context: + span_contexts.append(extract_text_map(json.loads(context))) if keep_destination: edu.strip_context()