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

feat(server): reduce metrics resources #123

Merged
merged 7 commits into from
Oct 20, 2022
Merged

feat(server): reduce metrics resources #123

merged 7 commits into from
Oct 20, 2022

Conversation

fortuna
Copy link

@fortuna fortuna commented Oct 18, 2022

This PR:

  • Removes location from the shadowsocks_tcp_probes time series, which is the largest one.
  • Removes 0s from shadowsocks_data_bytes. We were creating 4 entries (each proxy leg) for every country that attempted to connect, even if they were unsuccessful connections. The metric was dominated by zeros.
  • Reduces the cardinality of shadowsocks_data_bytes by removing the status and location. We don't need that for data limits. To enable country usage tracking, I introduce shadowsocks_data_bytes_per_location. To enable status tracking for UDP, I introduce shadowsocks_udp_packets_from_client_per_location. TCP already had shadowsocks_connections_closed for status tracking.

@fortuna fortuna requested a review from a team as a code owner October 18, 2022 21:41
@fortuna fortuna changed the title Fortuna metrics Reduce metrics resources Oct 18, 2022
@fortuna fortuna requested a review from a team as a code owner October 18, 2022 22:40
@fortuna fortuna removed request for a team October 18, 2022 22:42
@bemasc
Copy link

bemasc commented Oct 19, 2022

What do you think about keeping the TCP probes histogram but removing the location dimension? It seems like that should reduce the usage by a large factor without losing the visibility.

Copy link

@jyyi1 jyyi1 left a comment

Choose a reason for hiding this comment

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

LGTM, thanks.

@fortuna
Copy link
Author

fortuna commented Oct 20, 2022

@bemasc I restored the tcp probe metric, without location. Another look?

@fortuna fortuna merged commit fbc1b4f into master Oct 20, 2022
@fortuna fortuna deleted the fortuna-metrics branch October 20, 2022 20:09
bemasc pushed a commit that referenced this pull request Oct 20, 2022
This text was deleted in #123.

This is the same as the previous text, except that the mention of the location dimension has been removed, as this was deleted in #123.
fortuna pushed a commit that referenced this pull request Oct 21, 2022
This text was deleted in #123.

This is the same as the previous text, except that the mention of the location dimension has been removed, as this was deleted in #123.
@fortuna fortuna changed the title Reduce metrics resources feat: reduce metrics resources Nov 30, 2022
@fortuna fortuna changed the title feat: reduce metrics resources feat(server): reduce metrics resources Nov 30, 2022
dataBytes *prometheus.CounterVec
timeToCipherMs *prometheus.HistogramVec
buildInfo *prometheus.GaugeVec
accessKeys prometheus.Gauge

This comment was marked as spam.

@@ -208,20 +208,20 @@ func isFound(accessKey string) string {
}

// addIfNonZero helps avoid the creation of series that are always zero.

This comment was marked as spam.

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 this pull request may close these issues.

5 participants