Page MenuHomePhabricator

statsd client does not allow metric names with _seconds suffix
Closed, InvalidPublicBUG REPORT

Description

Found while working on T374048: Migrate GrowthExperiments.Mentorship Module to statslib. It seems the statsd client is checking for all metric names to end with _total (see source). While that is correct for counter metrics, the recommendations docs state that _seconds should be used for timing metrics.

Steps to replicate the issue (include links if applicable):

  • Make a call to mw.track with stats. prefix and _seconds suffix, eg: mw.track( 'stats.mediawiki_growth_experiments_navigation_duration_seconds', 123 )

What happens?:

An error is thrown:

TypeError: Invalid stat name mediawiki_growth_experiments_navigation_duration_seconds
    at formatDogstatsd (statsd.js:115:17)
    at statsd.js:151:15
    at Object.handler (mediawiki.base.js:416:5)
    at fire (jquery.js:3223:31)
    at Object.fireWith (jquery.js:3353:7)
    at Object.fire (jquery.js:3361:10)
    at mw.track (mediawiki.base.js:386:17)
...

What should have happened instead?:
The track call should succeed

Event Timeline

Change #1113112 had a related patch set uploaded (by Sergio Gimeno; author: Sergio Gimeno):

[mediawiki/extensions/WikimediaEvents@master] statsd: allow stats metric names to end with _seconds

https://gerrit.wikimedia.org/r/1113112

Sgs moved this task from Tracking to Current Sprint on the Growth-Team board.
Sgs edited projects, added: Growth-Team (Current Sprint); removed: Growth-Team.
Sgs moved this task from Incoming to Code Review on the Growth-Team (Current Sprint) board.

I think this behavior is intentional. @Krinkle should know more?

Is this essentially a duplicate of T383953: Statsv support for timer metrics? If I read that task correctly, we cannot migrate timings collected in browser JavaScript at all because statsv does not support it (yet). The way I understand our Grafana dashboards, we're interested in measuring percentiles for these timings, not just counting seconds.

Change #1113112 abandoned by Sergio Gimeno:

[mediawiki/extensions/WikimediaEvents@master] statsd: allow stats metric names to end with _seconds

Reason:

Requires implementing timers first

https://gerrit.wikimedia.org/r/1113112