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

Opentracing end to end encryption #5687

Closed
wants to merge 47 commits into from

Conversation

JorikSchellekens
Copy link
Contributor

@JorikSchellekens JorikSchellekens commented Jul 15, 2019

This traces most of the code paths related to e2e stuff. It adds context information into stored edus and tries to link spans across transactions. It traces device_list updates, key queries and key claiming.

This pr is waiting on #5703. I've set the base to joriks/opentracing_utils so that they can be easily compared. I'll switch it over to develop once #5703 lands.

* limitations under the License.
*/

ALTER TABLE device_lists_outbound_pokes ADD context TEXT;
Copy link
Member

Choose a reason for hiding this comment

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

typically we include a comment explaining what the new column means.

This comment was marked as resolved.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

How's that?

Copy link
Member

Choose a reason for hiding this comment

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

it's a bit wordy, to be honest :/. I mean, it's great, but I think it's more detail than we need here.

Opentracing context data for inclusion in the device_list_update EDUs, as a json-encoded dictionary

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fair enough

@JorikSchellekens JorikSchellekens force-pushed the joriks/opentracing_e2e branch 2 times, most recently from 54d50ca to 7f282d1 Compare July 17, 2019 13:37
@JorikSchellekens JorikSchellekens changed the title Increase opentracing usability Opentracing end to end encryption Jul 17, 2019
@JorikSchellekens JorikSchellekens changed the base branch from develop to joriks/opentracing_utils July 17, 2019 13:53
@codecov
Copy link

codecov bot commented Jul 17, 2019

Codecov Report

Merging #5687 into joriks/opentracing_utils will decrease coverage by 38.31%.
The diff coverage is 24%.

@@                     Coverage Diff                      @@
##           joriks/opentracing_utils   #5687       +/-   ##
============================================================
- Coverage                     63.22%   24.9%   -38.32%     
============================================================
  Files                           331     171      -160     
  Lines                         36216   16088    -20128     
  Branches                       5964    2343     -3621     
============================================================
- Hits                          22897    4007    -18890     
- Misses                        11688   11996      +308     
+ Partials                       1631      85     -1546

@codecov
Copy link

codecov bot commented Jul 17, 2019

Codecov Report

Merging #5687 into develop will increase coverage by 0.12%.
The diff coverage is 62.99%.

@@             Coverage Diff             @@
##           develop    #5687      +/-   ##
===========================================
+ Coverage    63.33%   63.46%   +0.12%     
===========================================
  Files          331      331              
  Lines        36584    36563      -21     
  Branches      6046     6009      -37     
===========================================
+ Hits         23172    23205      +33     
+ Misses       11760    11712      -48     
+ Partials      1652     1646       -6

@JorikSchellekens JorikSchellekens force-pushed the joriks/opentracing_utils branch 2 times, most recently from 8baeadd to 1345ac8 Compare July 19, 2019 12:53
@JorikSchellekens JorikSchellekens requested a review from a team July 23, 2019 14:16
@JorikSchellekens JorikSchellekens changed the base branch from joriks/opentracing_utils to develop July 23, 2019 14:17
@JorikSchellekens JorikSchellekens removed the request for review from a team July 23, 2019 14:17
changelog.d/5687.misc Outdated Show resolved Hide resolved
synapse/api/auth.py Show resolved Hide resolved
synapse/api/auth.py Outdated Show resolved Hide resolved
synapse/api/auth.py Show resolved Hide resolved
synapse/api/auth.py Show resolved Hide resolved
synapse/storage/devices.py Outdated Show resolved Hide resolved
synapse/storage/devices.py Outdated Show resolved Hide resolved
synapse/storage/devices.py Outdated Show resolved Hide resolved
* limitations under the License.
*/

ALTER TABLE device_lists_outbound_pokes ADD context TEXT;

This comment was marked as resolved.

synapse/storage/devices.py Outdated Show resolved Hide resolved
JorikSchellekens and others added 9 commits August 5, 2019 11:30
Co-Authored-By: Richard van der Hoff <1389908+richvdh@users.noreply.github.com>
Co-Authored-By: Richard van der Hoff <1389908+richvdh@users.noreply.github.com>
Co-Authored-By: Richard van der Hoff <1389908+richvdh@users.noreply.github.com>
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.

as discussed, can you flatten out one of the layers of dictionaries, and rename the EDU key?

synapse/handlers/device.py Show resolved Hide resolved
synapse/logging/opentracing.py Show resolved Hide resolved
synapse/logging/opentracing.py Show resolved Hide resolved
synapse/logging/opentracing.py Show resolved Hide resolved
synapse/storage/devices.py Show resolved Hide resolved
synapse/storage/devices.py Show resolved Hide resolved
synapse/storage/devices.py Show resolved Hide resolved
synapse/storage/devices.py Show resolved Hide resolved
synapse/storage/devices.py Show resolved Hide resolved
@JorikSchellekens
Copy link
Contributor Author

This pr has been superseded by the following smaller prs:
#5856
#5855
#5853
#5852

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.

2 participants