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.

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

Extend Metric-Logs: longpolling #928

Closed
peb-adr opened this issue Apr 30, 2024 · 3 comments · Fixed by #929
Closed

Extend Metric-Logs: longpolling #928

peb-adr opened this issue Apr 30, 2024 · 3 comments · Fixed by #929
Assignees

Comments

@peb-adr
Copy link
Member

peb-adr commented Apr 30, 2024

We would like to be able to see how many users are using longpolling as opposed to h2-streams.

So first we need to confirm our assumption that the

  • current_connections,
  • connected_users_current,
  • connected_users_total,
  • connected_users_average_connections and
  • connected_users_anonymous_connections

metrics only measure the active h2-streams and longpolling is currently not affecting these values.

Then we would like to have separate metrics providing similar statistics for longpolling:

  • current_longpolls - "Number of open requests at time of evaluation, polling for a new update",
  • longpolling_users_current - "Number of unique users at time of evaluation, polling for a new update",
  • longpolling_users_total - "Number of unique users who have polled for a new update since start of the service",
  • longpolling_users_average_longpolls - not feasible IMO
  • longpolling_users_anonymous_longpolls - same as current_longpolls but only anonymous requests

Also feel free to add *_local variants where you find it sensible.

@ostcar ostcar linked a pull request May 1, 2024 that will close this issue
@ostcar
Copy link
Member

ostcar commented May 1, 2024

So first we need to confirm our assumption that the

* `current_connections`,

* `connected_users_current`,

* `connected_users_total`,

* `connected_users_average_connections` and

* `connected_users_anonymous_connections`

metrics only measure the active h2-streams and longpolling is currently not affecting these values.

This assumption is wrong. All connections that match this regex are counted: ^\/system\/autoupdate[^\/]. This are nearly all connections. For example, also /system/autoupdate?single=1

So currently, longpolling requests and h2-streams are in the same metic.

I created this #929 that uses two separate metrics. One for longpolling and one for all other requests (mostly h2-streams).

The naming of the metrics is different then requested from you. It was easier to use a prefix then a suffix.

Keep in mind, that the local statistic gets updated immediately when a connection is opened or closed. But the non local statistic is only updated from time to time. The default value for METRIC_SAVE_INTERVAL is 5 minutes.

Longpolling requests get opened and closed with each autoupdate. So there is much more flickering in the number of open longpolling requests. If there is an autoupdate requests, all longpolling connections get closed. If immediately after the metric is saved to redis, it will look like there are very few longpolling requests.

@peb-adr
Copy link
Member Author

peb-adr commented May 1, 2024

Longpolling requests get opened and closed with each autoupdate. So there is much more flickering in the number of open longpolling requests. If there is an autoupdate requests, all longpolling connections get closed. If immediately after the metric is saved to redis, it will look like there are very few longpolling requests.

I had that thought as well.
Do you think calculating an average for the last METRIC_SAVE_INTERVAL is more useful?
If so and it's not too much effort you could try that.

@ostcar
Copy link
Member

ostcar commented May 2, 2024

I don't want to do this. At least not now.

It would add more complexity for something, that is probably not needed when the metric are plotted by grafana. In grafana, you can probably see the spikes and get a feeling, what the real numbers are

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants