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

Refactor request logging code #3700

Merged
merged 2 commits into from
Aug 17, 2018
Merged

Refactor request logging code #3700

merged 2 commits into from
Aug 17, 2018

Conversation

richvdh
Copy link
Member

@richvdh richvdh commented Aug 15, 2018

This commit moves a bunch of the logic for deciding when to log the receipt and
completion of HTTP requests into SynapseRequest, rather than in the request
handling wrappers.

Advantages of this are:

  • we get logs for all requests (including OPTIONS and HEADs), rather than
    just those that end up hitting handlers we've remembered to decorate
    correctly.

  • when a request handler wires up a Producer (as the media stuff does
    currently, and as other things will do soon), we log at the point that all
    of the traffic has been sent to the client.

@richvdh
Refactor request logging code
eaaa224 
This commit moves a bunch of the logic for deciding when to log the receipt and
completion of HTTP requests into SynapseRequest, rather than in the request
handling wrappers.

Advantages of this are:
 * we get logs for *all* requests (including OPTIONS and HEADs), rather than
   just those that end up hitting handlers we've remembered to decorate
   correctly.

 * when a request handler wires up a Producer (as the media stuff does
   currently, and as other things will do soon), we log at the point that all
   of the traffic has been sent to the client.
@richvdh
changelog
2de813a
This was referenced Aug 15, 2018
Closed
Merged
@richvdh richvdh requested a review from a team August 15, 2018 14:15
erikjohnston
erikjohnston approved these changes Aug 17, 2018
View reviewed changes
synapse/http/site.py
context = LoggingContext.current_context()
usage = context.get_resource_usage()
except Exception:
usage = ContextResourceUsage()
Copy link
Member

Choose a reason for hiding this comment

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

Why are we removing the error handling?

Copy link
Member Author

Choose a reason for hiding this comment

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

it seemed pretty redundant. We're already assuming that get_resource_usage doesn't throw elsewhere.

@richvdh richvdh merged commit 3f8709f into develop Aug 17, 2018
@richvdh richvdh deleted the rav/wait_for_producers branch August 17, 2018 13:58
richvdh added a commit that referenced this pull request Aug 20, 2018
@richvdh
Fix exceptions when a connection is closed before we read the headers
be65273 
This fixes bugs introduced in #3700, by making sure that we behave sanely
when an incoming connection is closed before the headers are read.
@richvdh richvdh mentioned this pull request Aug 20, 2018
Merged
richvdh added a commit that referenced this pull request Aug 22, 2018
@richvdh
Merge tag 'v0.33.3'
48fec67 
Features
--------

- Add support for the SNI extension to federation TLS connections. Thanks to @vojeroen! ([\#3439](#3439))
- Add /_media/r0/config ([\#3184](#3184))
- speed up /members API and add `at` and `membership` params as per MSC1227 ([\#3568](#3568))
- implement `summary` block in /sync response as per MSC688 ([\#3574](#3574))
- Add lazy-loading support to /messages as per MSC1227 ([\#3589](#3589))
- Add ability to limit number of monthly active users on the server ([\#3633](#3633))
- Support more federation endpoints on workers ([\#3653](#3653))
- Basic support for room versioning ([\#3654](#3654))
- Ability to disable client/server Synapse via conf toggle ([\#3655](#3655))
- Ability to whitelist specific threepids against monthly active user limiting ([\#3662](#3662))
- Add some metrics for the appservice and federation event sending loops ([\#3664](#3664))
- Where server is disabled, block ability for locked out users to read new messages ([\#3670](#3670))
- set admin uri via config, to be used in error messages where the user should contact the administrator ([\#3687](#3687))
- Synapse's presence functionality can now be disabled with the "use_presence" configuration option. ([\#3694](#3694))
- For resource limit blocked users, prevent writing into rooms ([\#3708](#3708))

Bugfixes
--------

- Fix occasional glitches in the synapse_event_persisted_position metric ([\#3658](#3658))
- Fix bug on deleting 3pid when using identity servers that don't support unbind API ([\#3661](#3661))
- Make the tests pass on Twisted < 18.7.0 ([\#3676](#3676))
- Don’t ship recaptcha_ajax.js, use it directly from Google ([\#3677](#3677))
- Fixes test_reap_monthly_active_users so it passes under postgres ([\#3681](#3681))
- Fix mau blocking calulation bug on login ([\#3689](#3689))
- Fix missing yield in synapse.storage.monthly_active_users.initialise_reserved_users ([\#3692](#3692))
- Improve HTTP request logging to include all requests ([\#3700](#3700))
- Avoid timing out requests while we are streaming back the response ([\#3701](#3701))
- Support more federation endpoints on workers ([\#3705](#3705), [\#3713](#3713))
- Fix "Starting db txn 'get_all_updated_receipts' from sentinel context" warning ([\#3710](#3710))
- Fix bug where `state_cache` cache factor ignored environment variables ([\#3719](#3719))
- Fix bug in v0.33.3rc1 which caused infinite loops and OOMs ([\#3723](#3723))
- Fix bug introduced in v0.33.3rc1 which made the ToS give a 500 error ([\#3732](#3732))

Deprecations and Removals
-------------------------

- The Shared-Secret registration method of the legacy v1/register REST endpoint has been removed. For a replacement, please see [the admin/register API documentation](https://github.com/matrix-org/synapse/blob/master/docs/admin_api/register_api.rst). ([\#3703](#3703))

Internal Changes
----------------

- The test suite now can run under PostgreSQL. ([\#3423](#3423))
- Refactor HTTP replication endpoints to reduce code duplication ([\#3632](#3632))
- Tests now correctly execute on Python 3. ([\#3647](#3647))
- Sytests can now be run inside a Docker container. ([\#3660](#3660))
- Port over enough to Python 3 to allow the sytests to start. ([\#3668](#3668))
- Update docker base image from alpine 3.7 to 3.8. ([\#3669](#3669))
- Rename synapse.util.async to synapse.util.async_helpers to mitigate async becoming a keyword on Python 3.7. ([\#3678](#3678))
- Synapse's tests are now formatted with the black autoformatter. ([\#3679](#3679))
- Implemented a new testing base class to reduce test boilerplate. ([\#3684](#3684))
- Rename MAU prometheus metrics ([\#3690](#3690))
- add new error type ResourceLimit ([\#3707](#3707))
- Logcontexts for replication command handlers ([\#3709](#3709))
- Update admin register API documentation to reference a real user ID. ([\#3712](#3712))
@richvdh richvdh restored the rav/wait_for_producers branch September 6, 2018 12:00
@hawkowl hawkowl deleted the rav/wait_for_producers branch September 20, 2018 14:01
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@erikjohnston erikjohnston erikjohnston approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@richvdh @erikjohnston