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

Commit

Permalink
Apply suggestions from code review
Browse files Browse the repository at this point in the history
Mostly just rewording parts of the docs for clarity.

Co-Authored-By: Richard van der Hoff <1389908+richvdh@users.noreply.github.com>
  • Loading branch information
JorikSchellekens and richvdh committed Jul 22, 2019
1 parent 65d22aa commit 15ff12e
Show file tree
Hide file tree
Showing 4 changed files with 15 additions and 16 deletions.
14 changes: 7 additions & 7 deletions docs/opentracing.rst
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ Our current selected implementation is Jaeger.

OpenTracing is a tool which gives an insight into the causal relationship of
work done in and between servers. The servers each track events and report them
to a centralised server - in our Synapse's case: Jaeger. The basic unit used to
to a centralised server - in Synapse's case: Jaeger. The basic unit used to
represent events is the span. The span roughly represents a single piece of work
that was done and the time at which it occurred. A span can have child spans,
meaning that the work of the child had to be completed for the parent span to
Expand Down Expand Up @@ -59,12 +59,12 @@ Latest documentation is probably at
https://www.jaegertracing.io/docs/1.13/getting-started/


Enable opentracing in Synapse
Enable OpenTracing in Synapse
-----------------------------

Opentracing is not enabled by default. It must be enabled in the homeserver
OpenTracing is not enabled by default. It must be enabled in the homeserver
config by uncommenting the config options under ``opentracing`` as shown in
the [sample config](./sample_config.yaml). For example:
the `sample config <./sample_config.yaml>`_. For example:

.. code-block:: yaml
Expand All @@ -77,7 +77,7 @@ the [sample config](./sample_config.yaml). For example:
Homeserver whitelisting
-----------------------

The homeserver whitelist is configured using regular expression. A list of regular
The homeserver whitelist is configured using regular expressions. A list of regular
expressions can be given and their union will be compared when propagating any
spans contexts to another homeserver.

Expand All @@ -86,9 +86,9 @@ untrusted users since span contexts are usually opaque ids it can lead to
two problems, namely:

- If the span context is marked as sampled by the sending homeserver the receiver will
sample it. Therefore two homeservers with wildly disparaging sampling policies
sample it. Therefore two homeservers with wildly different sampling policies
could incur higher sampling counts than intended.
- Span baggage can be arbitrary data. For safety this has been disabled in Synapse
- Sending servers can attach arbitrary data to spans, known as 'baggage'. For safety this has been disabled in Synapse
but that doesn't prevent another server sending you baggage which will be logged
to OpenTracing's logs.

Expand Down
4 changes: 2 additions & 2 deletions docs/sample_config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -1422,8 +1422,8 @@ opentracing:
#enabled: true

# The list of homeservers we wish to send and receive span contexts and span baggage.
#
# This a list of regexes which are matched against the server_name of the
# See docs/opentracing.rst
# This is a list of regexes which are matched against the server_name of the
# homeserver.
#
# By defult, it is empty, so no servers are matched.
Expand Down
4 changes: 2 additions & 2 deletions synapse/config/tracer.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,8 @@ def generate_config_section(cls, **kwargs):
#enabled: true
# The list of homeservers we wish to send and receive span contexts and span baggage.
#
# This a list of regexes which are matched against the server_name of the
# See docs/opentracing.rst
# This is a list of regexes which are matched against the server_name of the
# homeserver.
#
# By defult, it is empty, so no servers are matched.
Expand Down
9 changes: 4 additions & 5 deletions synapse/logging/opentracing.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,16 +30,15 @@
============================
Python-specific tracing concepts are at https://opentracing.io/guides/python/.
Note that Synapse wraps OpenTracing in a small module in order to make the
Note that Synapse wraps OpenTracing in a small module (this one) in order to make the
OpenTracing dependency optional. That means that the access patterns are
different to those demonstrated in the OpenTracing guides. However, it is
still useful to know, especially if OpenTracing is included as a full dependency
in the future or if you are modifying Synapse's `opentracing` module.
in the future or if you are modifying this module.
Access to the OpenTracing API is mediated through the
``logging/opentracing.py`` module. OpenTracing is encapsulated so that
no span objects from OpenTracing are exposed in Synapses code. This allows
OpenTracing is encapsulated so that
no span objects from OpenTracing are exposed in Synapse's code. This allows
OpenTracing to be easily disabled in Synapse and thereby have OpenTracing as
an optional dependency. This does however limit the number of modifiable spans
at any point in the code to one. From here out references to `opentracing`
Expand Down

0 comments on commit 15ff12e

Please sign in to comment.