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

Opentracing Documentation #5703

Merged
merged 9 commits into from
Jul 22, 2019
Merged

Conversation

JorikSchellekens
Copy link
Contributor

@JorikSchellekens JorikSchellekens commented Jul 17, 2019

Adds documentation for how OpenTracing should be configured by users and used by developers.

@codecov
Copy link

codecov bot commented Jul 17, 2019

Codecov Report

Merging #5703 into release-v1.2.0 will decrease coverage by 0.4%.
The diff coverage is 100%.

@@                Coverage Diff                 @@
##           release-v1.2.0    #5703      +/-   ##
==================================================
- Coverage           63.44%   63.03%   -0.41%     
==================================================
  Files                 331      331              
  Lines               36292    36293       +1     
  Branches             5990     5990              
==================================================
- Hits                23025    22877     -148     
- Misses              11628    11754     +126     
- Partials             1639     1662      +23

@JorikSchellekens JorikSchellekens force-pushed the joriks/opentracing_utils branch from 23fa9ed to 3a09218 Compare July 18, 2019 15:08
Copy link
Member

@richvdh richvdh left a comment

Choose a reason for hiding this comment

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

@JorikSchellekens sorry that the below is a bit of a long list - this is a really great start!

I think it would be best to remove the code refactoring from here for now and turn this into a documentation-only PR; we can look a the code changes separately, later on?

docs/opentracing.rst Outdated Show resolved Hide resolved
docs/opentracing.rst Outdated Show resolved Hide resolved
docs/opentracing.rst Outdated Show resolved Hide resolved
docs/opentracing.rst Outdated Show resolved Hide resolved
docs/opentracing.rst Outdated Show resolved Hide resolved
docs/opentracing.rst Outdated Show resolved Hide resolved
docs/opentracing.rst Outdated Show resolved Hide resolved
docs/opentracing.rst Outdated Show resolved Hide resolved
docs/opentracing.rst Outdated Show resolved Hide resolved
docs/opentracing.rst Outdated Show resolved Hide resolved
@JorikSchellekens
Copy link
Contributor Author

@richvdh splitting the refactoring from the documentation does make sense. The documentation refers to a set of functions only introduced in this pr however which would make the documentation inconsistent if it gets into the release.

@richvdh
Copy link
Member

richvdh commented Jul 19, 2019

most of it doesn't seem to?

@JorikSchellekens JorikSchellekens force-pushed the joriks/opentracing_utils branch from 8baeadd to 1345ac8 Compare July 19, 2019 12:53
@JorikSchellekens
Copy link
Contributor Author

The examples use decorators only introduced in this pr. I don't think it would be a great evil if they momentarily get out of sync. Also, I think the code review could be done before the release.

These are all alterations as a result of comments in #5703, it
includes mostly typos and clarifications. The most interesting
changes are:

- Split developer and user docs into two sections
- Add a high level description of OpenTracing
@JorikSchellekens JorikSchellekens changed the title Opentracing usability Opentracing Documentation Jul 19, 2019
@JorikSchellekens JorikSchellekens requested a review from richvdh July 19, 2019 15:27
@richvdh richvdh changed the base branch from develop to release-v1.2.0 July 19, 2019 17:14
Copy link
Member

@richvdh richvdh left a comment

Choose a reason for hiding this comment

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

this is awesome. I've picked a few nits below, but they are all pretty trivial. Suggest you fix them up and merge. Please remember to squash-merge, as the commit history doesn't make much sense.

I've changed the base for the PR to the release branch, so that we can pull this into the release.

Thanks for taking the time to make this great :)

docs/opentracing.rst Outdated Show resolved Hide resolved
docs/opentracing.rst Outdated Show resolved Hide resolved
docs/opentracing.rst Outdated Show resolved Hide resolved
docs/opentracing.rst Outdated Show resolved Hide resolved
docs/opentracing.rst Outdated Show resolved Hide resolved
synapse/config/tracer.py Show resolved Hide resolved
synapse/logging/opentracing.py Outdated Show resolved Hide resolved
synapse/logging/opentracing.py Outdated Show resolved Hide resolved
synapse/logging/opentracing.py Outdated Show resolved Hide resolved
synapse/logging/opentracing.py Outdated Show resolved Hide resolved
Mostly just rewording parts of the docs for clarity.

Co-Authored-By: Richard van der Hoff <1389908+richvdh@users.noreply.github.com>
@JorikSchellekens JorikSchellekens force-pushed the joriks/opentracing_utils branch from 7fa4d1b to 15ff12e Compare July 22, 2019 09:14
@JorikSchellekens
Copy link
Contributor Author

This PR doesn't contain any working code so how can these tests be failing?

@erikjohnston
Copy link
Member

Seems to be this test, which is presumably now flakey :/

If remote user leaves room we no longer receive device updates

@JorikSchellekens JorikSchellekens merged commit 826e6ec into release-v1.2.0 Jul 22, 2019
@richvdh
Copy link
Member

richvdh commented Jul 22, 2019

Seems to be this test, which is presumably now flakey :/

If remote user leaves room we no longer receive device updates

That one's been flaky for a while...

anoadragon453 added a commit that referenced this pull request Jul 22, 2019
v1.2.0rc1

Features
--------

- Add support for opentracing. ([\#5544](#5544), [\#5712](#5712))
- Add ability to pull all locally stored events out of synapse that a particular user can see. ([\#5589](#5589))
- Add a basic admin command app to allow server operators to run Synapse admin commands separately from the main production instance. ([\#5597](#5597))
- Add `sender` and `origin_server_ts` fields to `m.replace`. ([\#5613](#5613))
- Add default push rule to ignore reactions. ([\#5623](#5623))
- Include the original event when asking for its relations. ([\#5626](#5626))
- Implement `session_lifetime` configuration option, after which access tokens will expire. ([\#5660](#5660))
- Return "This account has been deactivated" when a deactivated user tries to login. ([\#5674](#5674))
- Enable aggregations support by default ([\#5714](#5714))

Bugfixes
--------

- Fix 'utime went backwards' errors on daemonization. ([\#5609](#5609))
- Various minor fixes to the federation request rate limiter. ([\#5621](#5621))
- Forbid viewing relations on an event once it has been redacted. ([\#5629](#5629))
- Fix requests to the `/store_invite` endpoint of identity servers being sent in the wrong format. ([\#5638](#5638))
- Fix newly-registered users not being able to lookup their own profile without joining a room. ([\#5644](#5644))
- Fix bug in #5626 that prevented the original_event field from actually having the contents of the original event in a call to `/relations`. ([\#5654](#5654))
- Fix 3PID bind requests being sent to identity servers as `application/x-form-www-urlencoded` data, which is deprecated. ([\#5658](#5658))
- Fix some problems with authenticating redactions in recent room versions. ([\#5699](#5699), [\#5700](#5700), [\#5707](#5707))
- Ignore redactions of m.room.create events. ([\#5701](#5701))

Updates to the Docker image
---------------------------

- Base Docker image on a newer Alpine Linux version (3.8 -> 3.10). ([\#5619](#5619))
- Add missing space in default logging file format generated by the Docker image. ([\#5620](#5620))

Improved Documentation
----------------------

- Add information about nginx normalisation to reverse_proxy.rst. Contributed by @skalarproduktraum - thanks! ([\#5397](#5397))
- --no-pep517 should be --no-use-pep517 in the documentation to setup the development environment. ([\#5651](#5651))
- Improvements to Postgres setup instructions. Contributed by @Lrizika - thanks! ([\#5661](#5661))
- Minor tweaks to postgres documentation. ([\#5675](#5675))

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

- Remove support for the `invite_3pid_guest` configuration setting. ([\#5625](#5625))

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

- Move logging code out of `synapse.util` and into `synapse.logging`. ([\#5606](#5606), [\#5617](#5617))
- Add a blacklist file to the repo to blacklist certain sytests from failing CI. ([\#5611](#5611))
- Make runtime errors surrounding password reset emails much clearer. ([\#5616](#5616))
- Remove dead code for persiting outgoing federation transactions. ([\#5622](#5622))
- Add `lint.sh` to the scripts-dev folder which will run all linting steps required by CI. ([\#5627](#5627))
- Move RegistrationHandler.get_or_create_user to test code. ([\#5628](#5628))
- Add some more common python virtual-environment paths to the black exclusion list. ([\#5630](#5630))
- Some counter metrics exposed over Prometheus have been renamed, with the old names preserved for backwards compatibility and deprecated. See `docs/metrics-howto.rst` for details. ([\#5636](#5636))
- Unblacklist some user_directory sytests. ([\#5637](#5637))
- Factor out some redundant code in the login implementation. ([\#5639](#5639))
- Update ModuleApi to avoid register(generate_token=True). ([\#5640](#5640))
- Remove access-token support from `RegistrationHandler.register`, and rename it. ([\#5641](#5641))
- Remove access-token support from `RegistrationStore.register`, and rename it. ([\#5642](#5642))
- Improve logging for auto-join when a new user is created. ([\#5643](#5643))
- Remove unused and unnecessary check for FederationDeniedError in _exception_to_failure. ([\#5645](#5645))
- Fix a small typo in a code comment. ([\#5655](#5655))
- Clean up exception handling around client access tokens. ([\#5656](#5656))
- Add a mechanism for per-test homeserver configuration in the unit tests. ([\#5657](#5657))
- Inline issue_access_token. ([\#5659](#5659))
- Update the sytest BuildKite configuration to checkout Synapse in `/src`. ([\#5664](#5664))
- Add a `docker` type to the towncrier configuration. ([\#5673](#5673))
- Convert `synapse.federation.transport.server` to `async`. Might improve some stack traces. ([\#5689](#5689))
- Documentation for opentracing. ([\#5703](#5703))
@JorikSchellekens JorikSchellekens deleted the joriks/opentracing_utils branch July 23, 2019 15:51
anoadragon453 added a commit that referenced this pull request Feb 19, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants