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

Add metrics for tracking 3PID /requestToken requests. #8712

Merged
merged 4 commits into from
Nov 13, 2020
Merged

Add metrics for tracking 3PID /requestToken requests. #8712

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
69830fd
Add metrics for tracking 3PID /requestToken requests.
erikjohnston Nov 3, 2020
8193350
Newsfile
erikjohnston Nov 3, 2020
5570f1d
Apply suggestions from code review
erikjohnston Nov 13, 2020
e511765
Update synapse/metrics/__init__.py
erikjohnston Nov 13, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/8712.misc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Add metrics the allow the local sysadmin to track 3PID `/requestToken` requests.
10 changes: 10 additions & 0 deletions synapse/metrics/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -502,6 +502,16 @@ def collect(self):

last_ticked = time.time()

# 3PID send info
threepid_send_requests = Histogram(
"synapse_threepid_send_requests_with_tries",
documentation="Number of requests for a 3pid token by try count. Note if"
" there is a request with try count of 4, then there would have been one"
" each for 1, 2 and 3",
buckets=(1, 2, 3, 4, 5, 10),
labelnames=("type", "reason"),
)


class ReactorLastSeenMetric:
def collect(self):
Expand Down
13 changes: 13 additions & 0 deletions synapse/rest/client/v2_alpha/account.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
parse_json_object_from_request,
parse_string,
)
from synapse.metrics import threepid_send_requests
from synapse.push.mailer import Mailer
from synapse.util.msisdn import phone_number_to_msisdn
from synapse.util.stringutils import assert_valid_client_secret, random_string
Expand Down Expand Up @@ -143,6 +144,10 @@ async def on_POST(self, request):
# Wrap the session id in a JSON object
ret = {"sid": sid}

threepid_send_requests.labels(type="email", reason="password_reset").observe(
send_attempt
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 worth noting that the spec allows client to put any integer for send_attempt, which may generate incorrect metric results. I'm not sure if known good clients converge on the same behaviour as a result.

Additionally, it looks as though we don't even check if this value is an integer...

Copy link
Member Author

Choose a reason for hiding this comment

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

TBH I think I'm fine with that. I believe the code would have exploded before if the client didn't give it an int since the DB uses an int field.

Copy link
Member

Choose a reason for hiding this comment

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

Ah, yes good point, this would be on the other side of the DB.

On the "any integer" bit, we concluded in #synapse-dev that most clients start from 1, which will yield uniform metrics, and some outliers are no big deal.

)

return 200, ret


Expand Down Expand Up @@ -411,6 +416,10 @@ async def on_POST(self, request):
# Wrap the session id in a JSON object
ret = {"sid": sid}

threepid_send_requests.labels(type="email", reason="add_threepid").observe(
send_attempt
)

return 200, ret


Expand Down Expand Up @@ -481,6 +490,10 @@ async def on_POST(self, request):
next_link,
)

threepid_send_requests.labels(type="msisdn", reason="add_threepid").observe(
send_attempt
)

return 200, ret


Expand Down
9 changes: 9 additions & 0 deletions synapse/rest/client/v2_alpha/register.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@
parse_json_object_from_request,
parse_string,
)
from synapse.metrics import threepid_send_requests
from synapse.push.mailer import Mailer
from synapse.util.msisdn import phone_number_to_msisdn
from synapse.util.ratelimitutils import FederationRateLimiter
Expand Down Expand Up @@ -163,6 +164,10 @@ async def on_POST(self, request):
# Wrap the session id in a JSON object
ret = {"sid": sid}

threepid_send_requests.labels(type="email", reason="register").observe(
send_attempt
)

return 200, ret


Expand Down Expand Up @@ -234,6 +239,10 @@ async def on_POST(self, request):
next_link,
)

threepid_send_requests.labels(type="msisdn", reason="register").observe(
send_attempt
)

return 200, ret


Expand Down