Skip to content

Add more metrics #1145

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
Closed
josecelano opened this issue Dec 19, 2024 · 2 comments · Fixed by #1176
Closed

Add more metrics #1145

josecelano opened this issue Dec 19, 2024 · 2 comments · Fixed by #1176
Assignees
@josecelano
Labels
Enhancement / Feature Request Something New

Comments

@josecelano
Copy link
Member

josecelano commented Dec 19, 2024 •
edited
Loading

Relates to:

Current metrics (http://0.0.0.0:1212/api/v1/stats?token=MyAccessToken):

{
  "torrents": 0,
  "seeders": 0,
  "completed": 0,
  "leechers": 0,
  "tcp4_connections_handled": 0,
  "tcp4_announces_handled": 0,
  "tcp4_scrapes_handled": 0,
  "tcp6_connections_handled": 0,
  "tcp6_announces_handled": 0,
  "tcp6_scrapes_handled": 0,
  "udp_requests_aborted": 0,
  "udp4_requests": 0,
  "udp4_connections_handled": 0,
  "udp4_announces_handled": 0,
  "udp4_scrapes_handled": 0,
  "udp4_responses": 0,
  "udp4_errors_handled": 0,
  "udp6_requests": 0,
  "udp6_connections_handled": 0,
  "udp6_announces_handled": 0,
  "udp6_scrapes_handled": 0,
  "udp6_responses": 0,
  "udp6_errors_handled": 0
}

We will add these new metrics:

  • upd_average_connect_latency: the average time processing a UDP connect request takes.
  • upd_average_announce_latency: the average time processing a UDP announce request takes.
  • upd_average_scrape_latency: the average time processing a UDP announce request takes.

And also, these:

  • udp_banned_ips_total: the total number of IPs that have been banned for sending wrong connection IDs.
  • udp_requests_banned: the total number of UDP requests that have been banned.

NOTE: Counting the number of requests banned can decrease the effectiveness of the banned service because it implies minimal request processing (to increase the counter). I think it's worth knowing exactly what's happening.

We can add the generic udp_banned_ips_total; in the future, if we have more reasons to ban an IP, we can create sub-counters for each reason.

@da2ce7 Should these metrics also be split into IPv4 and IPV6 metrics? I think it's not relevant in this case.
@josecelano josecelano mentioned this issue Dec 19, 2024
Open
@josecelano josecelano self-assigned this Jan 7, 2025
josecelano added a commit to josecelano/torrust-tracker that referenced this issue Jan 7, 2025
@josecelano
feat: [torrust#1145] add banned reqs counter to stats
6f9b44c
@josecelano josecelano linked a pull request Jan 7, 2025 that will close this issue
Add more metrics #1176
Merged
5 tasks
josecelano added a commit to josecelano/torrust-tracker that referenced this issue Jan 7, 2025
@josecelano
feat: [torrust#1145] add banned ips total for UDP to stats
0bd080b 
```json
{
  "torrents": 0,
  "seeders": 0,
  "completed": 0,
  "leechers": 0,
  "tcp4_connections_handled": 0,
  "tcp4_announces_handled": 0,
  "tcp4_scrapes_handled": 0,
  "tcp6_connections_handled": 0,
  "tcp6_announces_handled": 0,
  "tcp6_scrapes_handled": 0,
  "udp_requests_aborted": 0,
  "udp_requests_banned": 0,
  "udp_banned_ips_total": 0,
  "udp4_requests": 0,
  "udp4_connections_handled": 0,
  "udp4_announces_handled": 0,
  "udp4_scrapes_handled": 0,
  "udp4_responses": 0,
  "udp4_errors_handled": 0,
  "udp6_requests": 0,
  "udp6_connections_handled": 0,
  "udp6_announces_handled": 0,
  "udp6_scrapes_handled": 0,
  "udp6_responses": 0,
  "udp6_errors_handled": 0
}
```

The new metric: `udp_banned_ips_total`. It's the total number of IPs that have been banned for sending wrong connection IDs.
josecelano added a commit to josecelano/torrust-tracker that referenced this issue Jan 7, 2025
@josecelano
feat: [torrust#1145] add banned ips total for UDP to stats
d7f60a1 
```json
{
  "torrents": 0,
  "seeders": 0,
  "completed": 0,
  "leechers": 0,
  "tcp4_connections_handled": 0,
  "tcp4_announces_handled": 0,
  "tcp4_scrapes_handled": 0,
  "tcp6_connections_handled": 0,
  "tcp6_announces_handled": 0,
  "tcp6_scrapes_handled": 0,
  "udp_requests_aborted": 0,
  "udp_requests_banned": 0,
  "udp_banned_ips_total": 0,
  "udp4_requests": 0,
  "udp4_connections_handled": 0,
  "udp4_announces_handled": 0,
  "udp4_scrapes_handled": 0,
  "udp4_responses": 0,
  "udp4_errors_handled": 0,
  "udp6_requests": 0,
  "udp6_connections_handled": 0,
  "udp6_announces_handled": 0,
  "udp6_scrapes_handled": 0,
  "udp6_responses": 0,
  "udp6_errors_handled": 0
}
```

The new metric: `udp_banned_ips_total`. It's the total number of IPs that have been banned for sending wrong connection IDs.
josecelano added a commit to josecelano/torrust-tracker that referenced this issue Jan 7, 2025
@josecelano
feat: [torrust#1145] add banned ips total for UDP to stats
1ce2e33 
```json
{
  "torrents": 0,
  "seeders": 0,
  "completed": 0,
  "leechers": 0,
  "tcp4_connections_handled": 0,
  "tcp4_announces_handled": 0,
  "tcp4_scrapes_handled": 0,
  "tcp6_connections_handled": 0,
  "tcp6_announces_handled": 0,
  "tcp6_scrapes_handled": 0,
  "udp_requests_aborted": 0,
  "udp_requests_banned": 0,
  "udp_banned_ips_total": 0,
  "udp4_requests": 0,
  "udp4_connections_handled": 0,
  "udp4_announces_handled": 0,
  "udp4_scrapes_handled": 0,
  "udp4_responses": 0,
  "udp4_errors_handled": 0,
  "udp6_requests": 0,
  "udp6_connections_handled": 0,
  "udp6_announces_handled": 0,
  "udp6_scrapes_handled": 0,
  "udp6_responses": 0,
  "udp6_errors_handled": 0
}
```

The new metric: `udp_banned_ips_total`. It's the total number of IPs that have been banned for sending wrong connection IDs.
@josecelano
Copy link
Member Author

I'm going to use:

  • udp_average_connect_processing_time
  • udp_average_announce_processing_time
  • udp_average_scrape_processing_time

Because I will not include the time sending the response.

josecelano added a commit to josecelano/torrust-tracker that referenced this issue Jan 7, 2025
@josecelano
refactor: [torrust#1145] add type and processing time to UDP response…
08a862a 
… events

- The `kind`is the type of response: connect, annouince, etc
- The req_processing_time is the time it took to process the requests on
  the backend, without including sending the response back to the client
(network latency).
josecelano added a commit to josecelano/torrust-tracker that referenced this issue Jan 9, 2025
@josecelano
feat: [torrust#1145] add UDP avg processing time to stats
e3327db 
```json
{
  "torrents": 1,
  "seeders": 1,
  "completed": 0,
  "leechers": 0,
  "tcp4_connections_handled": 0,
  "tcp4_announces_handled": 0,
  "tcp4_scrapes_handled": 0,
  "tcp6_connections_handled": 0,
  "tcp6_announces_handled": 0,
  "tcp6_scrapes_handled": 0,
  "udp_requests_aborted": 0,
  "udp_requests_banned": 0,
  "udp_banned_ips_total": 0,
  "udp_avg_connect_processing_time_ns": 37000,
  "udp_avg_announce_processing_time_ns": 42067,
  "udp_avg_scrape_processing_time_ns": 0,
  "udp4_requests": 60,
  "udp4_connections_handled": 30,
  "udp4_announces_handled": 30,
  "udp4_scrapes_handled": 0,
  "udp4_responses": 60,
  "udp4_errors_handled": 0,
  "udp6_requests": 0,
  "udp6_connections_handled": 0,
  "udp6_announces_handled": 0,
  "udp6_scrapes_handled": 0,
  "udp6_responses": 0,
  "udp6_errors_handled": 0
}
```

New metrcis are:

- udp_avg_connect_processing_time_ns
- udp_avg_announce_processing_time_ns
- udp_avg_scrape_processing_time_ns
@josecelano
Copy link
Member Author

I'm going to use:

  • udp_average_connect_processing_time
  • udp_average_announce_processing_time
  • udp_average_scrape_processing_time

Because I will not include the time sending the response.

Renamed to:

  • udp_avg_connect_processing_time_ns
  • udp_avg_announce_processing_time_ns
  • udp_avg_scrape_processing_time_ns

josecelano added a commit to josecelano/torrust-tracker that referenced this issue Jan 9, 2025
@josecelano
feat: [torrust#1145] add UDP avg processing time to stats
903d47f 
```json
{
  "torrents": 1,
  "seeders": 1,
  "completed": 0,
  "leechers": 0,
  "tcp4_connections_handled": 0,
  "tcp4_announces_handled": 0,
  "tcp4_scrapes_handled": 0,
  "tcp6_connections_handled": 0,
  "tcp6_announces_handled": 0,
  "tcp6_scrapes_handled": 0,
  "udp_requests_aborted": 0,
  "udp_requests_banned": 0,
  "udp_banned_ips_total": 0,
  "udp_avg_connect_processing_time_ns": 37000,
  "udp_avg_announce_processing_time_ns": 42067,
  "udp_avg_scrape_processing_time_ns": 0,
  "udp4_requests": 60,
  "udp4_connections_handled": 30,
  "udp4_announces_handled": 30,
  "udp4_scrapes_handled": 0,
  "udp4_responses": 60,
  "udp4_errors_handled": 0,
  "udp6_requests": 0,
  "udp6_connections_handled": 0,
  "udp6_announces_handled": 0,
  "udp6_scrapes_handled": 0,
  "udp6_responses": 0,
  "udp6_errors_handled": 0
}
```

New metrcis are:

- udp_avg_connect_processing_time_ns
- udp_avg_announce_processing_time_ns
- udp_avg_scrape_processing_time_ns
josecelano added a commit that referenced this issue Jan 9, 2025
@josecelano
Merge #1176: Add more metrics
f6aca40 
903d47f feat: [#1145] add UDP avg processing time to stats (Jose Celano)
08a862a refactor: [#1145] add type and processing time to UDP response events (Jose Celano)
1ce2e33 feat: [#1145] add banned ips total for UDP to stats (Jose Celano)
1299f17 feat: make ban service generic for all trackers (Jose Celano)
6f9b44c feat: [#1145] add banned reqs counter to stats (Jose Celano)
2ff476b refactor: rename enum variand Udp4RequestAborted (Jose Celano)

Pull request description:

  Add more metrics useful for detecting tracker errors and load level.

  ### UDP

  - [x] `udp_requests_banned`: the total number of UDP requests that have been banned.
  - [x] `udp_banned_ips_total`: the total number of IPs that have been banned for sending wrong connection IDs.
  - [x] `udp_avg_connect_processing_time_ns`: the average time processing a UDP connect request.
  - [x] `udp_avg_announce_processing_time_ns`: the average time processing a UDP announce request.
  - [x] `udp_avg_scrape_processing_time_ns`: the average time processing a UDP scrape request.

  ### Important refactor

  I needed to pass the Ban Service to the stats handler to get some values. I did not want to add the ban service to the tracker because the tracker is already to "fat". It has many responsibilities. In fact, I want to extract new services out of the tracker like whitelist, authorization, etc. My plan was to extract them and leave the tracker as the application services container. However I think it will be easier if we:

  - We pass new services like `BanService` directly to handlers instead of using the tracker as a facade.
  - Move other services out of the `Tracker` and also pass them directly to handlers.

  At the end, the `Tracker` should have only a couple of methods like `announce` and `scrape`.

ACKs for top commit:
  josecelano:
    ACK 903d47f

Tree-SHA512: 31a8436466ea04608558e603fa8e60fd242dd44c5faae890db1234da31ac245d9eea824aa18d6f5ce3a84b94825909239d965d8f6ac62fddf0de3ac8bfd6b228
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@josecelano josecelano

Labels
Enhancement / Feature Request Something New
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Add more metrics josecelano/torrust-tracker
1 participant
@josecelano