Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

oohelperd: add support for prometheus metrics #2183

Closed
5 tasks done
bassosimone opened this issue Jul 5, 2022 · 2 comments · Fixed by ooni/probe-cli#897
Closed
5 tasks done

oohelperd: add support for prometheus metrics #2183

bassosimone opened this issue Jul 5, 2022 · 2 comments · Fixed by ooni/probe-cli#897
Assignees
@bassosimone
Labels
priority/medium

Comments

@bassosimone
Copy link
Contributor

bassosimone commented Jul 5, 2022 •
edited
Loading

We discussed which metrics today with @FedericoCeratto and @hellais and we mentioned:
@bassosimone bassosimone self-assigned this Jul 5, 2022
bassosimone added a commit to ooni/probe-cli that referenced this issue Aug 28, 2022
@bassosimone
fix(oohelperd): by default listen on localhost
9e43856 
Part of ooni/probe#2183
@bassosimone bassosimone mentioned this issue Aug 28, 2022
Merged
bassosimone added a commit to ooni/probe-cli that referenced this issue Aug 28, 2022
@bassosimone
fix(oohelperd): by default listen on localhost (#887)
d711c19 
Part of ooni/probe#2183
bassosimone added a commit to ooni/probe-cli that referenced this issue Aug 28, 2022
@bassosimone
feat(oohelperd): log messages at info level
eeb7712 
We're using a request-specific logger where we also print the ID
of the request. This design helps to observe logs produced by
concurrent requests.

Part of ooni/probe#2183

While there, fix ooni/probe#2241
@bassosimone bassosimone mentioned this issue Aug 28, 2022
Merged
bassosimone added a commit to ooni/probe-cli that referenced this issue Aug 28, 2022
@bassosimone
feat(oohelperd): log messages at info level (#896)
4241ee4 
We're using a request-specific logger where we also print the ID
of the request. This design helps to observe logs produced by
concurrent requests.

Part of ooni/probe#2183

While there, fix ooni/probe#2241
bassosimone added a commit to ooni/probe-cli that referenced this issue Aug 28, 2022
@bassosimone
feat(oohelperd): add prometheus metrics
917812b 
Closes ooni/probe#2183

While there, avoid exposing nil values for optional fields of the
THResponse struct (i.e., "ip_info" and "tls_handshake").
@bassosimone bassosimone mentioned this issue Aug 28, 2022
Merged
bassosimone added a commit to ooni/probe-cli that referenced this issue Aug 28, 2022
@bassosimone
feat(oohelperd): add prometheus metrics (#897)
dcdd8fb 
Closes ooni/probe#2183

While there, avoid exposing nil values for optional fields of the
THResponse struct (i.e., "ip_info" and "tls_handshake").

While there, fix `measurexlite`'s `OperationLogger` test
and make it deterministic rather than racy.
@bassosimone
Copy link
Contributor Author

bassosimone commented Aug 29, 2022 •
edited
Loading

We revisited our initial metrics deployment with @FedericoCeratto and wrote together this very short document explaining how to improve the existing metrics:

Terminology

  • the term “request” refers to an HTTP request
  • the term “wctask” refers to the Web Connectivity main measurement task
  • the term “dnstask” refers to the DNS subtask [unused by just to have a taxonomy]
  • the term “tcptlstask” refers to the TCP/TLS subtask [ditto]
  • the term “httptask” refers to the HTTP subtask [ditto]

Rationale: “measurement” is a loaded term and we want to avoid using it also in metrics.

Proposed metrics

  • oohelperd_wctask_duration_seconds

    • prometheus summary
    • quantiles: 0.25, 0.5, 0.75, 0.9, 0.99

  • oohelperd_requests_count

    • counter with two dimensions
      • code is the status code we returned
      • reason is why we returned such a status code
        • bad method
        • body too large
        • cannot parse body
        • main task failed

  • oohelperd_requests_inflight_gauge

    • gauge of number of requests inflight

Concerns

We need to investigate more in depth the impact of using 10m for Summary.

@bassosimone bassosimone reopened this Aug 29, 2022
bassosimone added a commit to ooni/probe-cli that referenced this issue Aug 29, 2022
@bassosimone
fix(oohelperd): improve prometheus metrics
85b6855 
1. add extra label to measurements by status code so that we
can understand why we're returning 400 to clients;

2. add counter for number of measurements started (which is also
part of the summary, but whatever) and failed.

The latter counter is mostly related to cases in which the TH
cannot even start to perform a measurement. That is:

- it's not a failure if a specific host is down;

- it's a failure if we cannot even parse the input URL.

Part of ooni/probe#2183
@bassosimone bassosimone mentioned this issue Aug 29, 2022
Merged
bassosimone added a commit to ooni/probe-cli that referenced this issue Aug 29, 2022
@bassosimone
fix(oohelperd): metrics improvements after design review (#903)
8c855ca 
This diff updates the metrics according to ooni/probe#2183 (comment)
@bassosimone
Copy link
Contributor Author

We can close again: I have just merged the PR implemented the above mentioned changes!

bassosimone added a commit to ooni/probe-cli that referenced this issue Aug 29, 2022
@bassosimone
fix(oohelperd): measurement -> wctask
255d10d 
Make sure we don't say measurement in metrics.

See ooni/probe#2183 (comment)
@bassosimone bassosimone mentioned this issue Aug 29, 2022
Merged
bassosimone added a commit to ooni/probe-cli that referenced this issue Aug 29, 2022
@bassosimone
fix(oohelperd): measurement -> wctask (#906)
705589b 
Make sure we don't say measurement in metrics.

See ooni/probe#2183 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@bassosimone bassosimone

Labels
priority/medium
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

feat(oohelperd): add prometheus metrics ooni/probe-cli
1 participant
@bassosimone