Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Log when events are (unexpectedly) filtered out of responses in tests #14213

1 change: 1 addition & 0 deletions changelog.d/14213.misc
@@ -0,0 +1 @@
Log when events are (maybe unexpectedly) filtered out of responses in tests.
3 changes: 2 additions & 1 deletion docker/README.md
Expand Up @@ -73,7 +73,8 @@ The following environment variables are supported in `generate` mode:
will log sensitive information such as access tokens.
This should not be needed unless you are a developer attempting to debug something
particularly tricky.

* `SYNAPSE_LOG_TESTING`: if set, Synapse will log additional information useful
for testing.

## Postgres

Expand Down
30 changes: 24 additions & 6 deletions docker/conf/log.config
Expand Up @@ -49,17 +49,35 @@ handlers:
class: logging.StreamHandler
formatter: precise

{% if not SYNAPSE_LOG_SENSITIVE %}
{#
If SYNAPSE_LOG_SENSITIVE is unset, then override synapse.storage.SQL to INFO
so that DEBUG entries (containing sensitive information) are not emitted.
#}
loggers:
# This is just here so we can leave `loggers` in the config regardless of whether
# we configure other loggers below (avoid empty yaml dict error).
_placeholder:
level: "INFO"

{% if not SYNAPSE_LOG_SENSITIVE %}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we remove SYNAPSE_LOG_SENSITIVE in favor of SYNAPSE_LOG_TESTING here?

The docs around SYNAPSE_LOG_SENSITIVE already mention it's for "a developer attempting to debug something particularly tricky."

Are we worried about a deprecation period in this case? If it's only a developer thing, I don't think we need to worry about it at all.

{#
If SYNAPSE_LOG_SENSITIVE is unset, then override synapse.storage.SQL to INFO
so that DEBUG entries (containing sensitive information) are not emitted.
#}
synapse.storage.SQL:
# beware: increasing this to DEBUG will make synapse log sensitive
# information such as access tokens.
level: INFO
{% endif %}
{% endif %}

{% if SYNAPSE_LOG_TESTING %}
{#
If Synapse is under test, log a few more useful things for a developer
attempting to debug something particularly tricky.

With `synapse.visibility.filtered_event_debug`, it logs when events are (maybe
unexpectedly) filtered out of responses in tests. It's just nice to be able to
look at the CI log and figure out why an event isn't being returned.
#}
synapse.visibility.filtered_event_debug:
level: DEBUG
{% endif %}

root:
level: {{ SYNAPSE_LOG_LEVEL or "INFO" }}
Expand Down
4 changes: 4 additions & 0 deletions scripts-dev/complement.sh
Expand Up @@ -267,6 +267,10 @@ if [[ -n "$SYNAPSE_TEST_LOG_LEVEL" ]]; then
# Since this is just a test suite, this is fine and won't reveal anyone's
# personal information
export PASS_SYNAPSE_LOG_SENSITIVE=1

# Log a few more useful things for a developer attempting to debug something
# particularly tricky.
export PASS_SYNAPSE_TEST_LOG_LEVEL=1
fi

# Run the tests!
Expand Down
14 changes: 7 additions & 7 deletions synapse/visibility.py
Expand Up @@ -41,7 +41,7 @@
from synapse.util import Clock

logger = logging.getLogger(__name__)

filtered_event_logger = logging.getLogger("synapse.visibility.filtered_event_debug")

VISIBILITY_PRIORITY = (
HistoryVisibility.WORLD_READABLE,
Expand Down Expand Up @@ -97,8 +97,8 @@ async def filter_events_for_client(
events_before_filtering = events
events = [e for e in events if not e.internal_metadata.is_soft_failed()]
if len(events_before_filtering) != len(events):
if logger.isEnabledFor(logging.DEBUG):
logger.debug(
if filtered_event_logger.isEnabledFor(logging.DEBUG):
filtered_event_logger.debug(
"filter_events_for_client: Filtered out soft-failed events: Before=%s, After=%s",
[event.event_id for event in events_before_filtering],
[event.event_id for event in events],
Expand Down Expand Up @@ -319,7 +319,7 @@ def _check_client_allowed_to_see_event(
_check_filter_send_to_client(event, clock, retention_policy, sender_ignored)
== _CheckFilter.DENIED
):
logger.debug(
filtered_event_logger.debug(
"_check_client_allowed_to_see_event(event=%s): Filtered out event because `_check_filter_send_to_client` returned `_CheckFilter.DENIED`",
event.event_id,
)
Expand All @@ -341,7 +341,7 @@ def _check_client_allowed_to_see_event(
)
return event

logger.debug(
filtered_event_logger.debug(
"_check_client_allowed_to_see_event(event=%s): Filtered out event because it's an outlier",
event.event_id,
)
Expand All @@ -367,7 +367,7 @@ def _check_client_allowed_to_see_event(

membership_result = _check_membership(user_id, event, visibility, state, is_peeking)
if not membership_result.allowed:
logger.debug(
filtered_event_logger.debug(
"_check_client_allowed_to_see_event(event=%s): Filtered out event because the user can't see the event because of their membership, membership_result.allowed=%s membership_result.joined=%s",
event.event_id,
membership_result.allowed,
Expand All @@ -378,7 +378,7 @@ def _check_client_allowed_to_see_event(
# If the sender has been erased and the user was not joined at the time, we
# must only return the redacted form.
if sender_erased and not membership_result.joined:
logger.debug(
filtered_event_logger.debug(
"_check_client_allowed_to_see_event(event=%s): Returning pruned event because `sender_erased` and the user was not joined at the time",
event.event_id,
)
Expand Down
6 changes: 6 additions & 0 deletions tests/test_utils/logging_setup.py
Expand Up @@ -54,4 +54,10 @@ def setup_logging() -> None:
log_level = os.environ.get("SYNAPSE_TEST_LOG_LEVEL", "ERROR")
root_logger.setLevel(log_level)

# Log when events are (maybe unexpectedly) filtered out of responses in tests. It's
# just nice to be able to look at the CI log and figure out why an event isn't being
# returned.
logging.getLogger("synapse.visibility.filtered_event_debug").setLevel(logging.DEBUG)
MadLittleMods marked this conversation as resolved.
Show resolved Hide resolved

# Blow away the pyo3-log cache so that it reloads the configuration.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment is just copied from

# Blow away the pyo3-log cache so that it reloads the configuration.
reset_logging_config()

reset_logging_config()