feat(tasks) Add taskbroker-client metrics for tasks#118052
Conversation
We want to standardize metrics prefixes and tags for taskworkers. Having standardized metrics makes building dashboards and alerting easier for platform teams, and makes documenting metrics for others simpler. I've put usage of the new metrics backend behind an env var so that I can incrementally enable and validate the new metrics. Refs STREAM-1174
| metrics_class = DatadogMetrics( | ||
| application="sentry", | ||
| statsd_host=os.getenv("HOST_IP", "127.0.0.1"), | ||
| statsd_port=os.getenv("SENTRY_STATSD_PORT", 8126), |
There was a problem hiding this comment.
can we try to funnel this through our existing django settings? having the envvar be read in many places is not great
There was a problem hiding this comment.
That's a good question. I'm not thrilled about the extra env var for the port either. Sadly our production metrics are all configured through getsentry and not env vars. I'll rework this to work with the production & self-hosted django settings.
| statsd_port=port, | ||
| sample_rate=settings.SENTRY_METRICS_SAMPLE_RATE, | ||
| enable_prefixed_metrics=True, | ||
| ) |
There was a problem hiding this comment.
Env enabled, metrics silently unchanged
Medium Severity
With USE_TASKWORKER_METRICS=1, if _extract_metrics_config returns a missing host or port (empty options, UDS-only StatsD, dummy backend), the code keeps SentryMetricsBackend and emits no warning. Rollout validation can look enabled while taskworker still uses the old metrics shape.
Reviewed by Cursor Bugbot for commit 6fc270b. Configure here.
There was a problem hiding this comment.
We'll have options defined in all the environments, and if extracting configuration fails we'll get a log message.
| metric_options = settings.SENTRY_METRICS_OPTIONS["primary_backend_args"] | ||
|
|
||
| # Some backends use `host` and others use `statsd_host` | ||
| host = metric_options.get("statsd_host", None) or metric_options.get("host", None) |
There was a problem hiding this comment.
I regret asking for this. but i think architecturally it's probably still better?
There was a problem hiding this comment.
Its a bit gnarly, but better than having more environment variables.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 2 total unresolved issues (including 1 from previous review).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 785bf98. Configure here.
|
|
||
| # Some backends use `host` and others use `statsd_host` | ||
| host = metric_options.get("statsd_host", None) or metric_options.get("host", None) | ||
| raw_port = metric_options.get("statsd_port", None) or metric_options.get("port", None) |
There was a problem hiding this comment.
Host fallback misroutes StatsD
Medium Severity
When USE_TASKWORKER_METRICS is enabled, _extract_metrics_config falls back from statsd_host to host. In DatadogMetricsBackend, host is the metric reporting hostname, not the DogStatsD agent. If statsd_host is omitted but host and statsd_port are set, taskworker DatadogMetrics can send to the wrong address while Sentry’s main metrics backend still uses the default agent.
Reviewed by Cursor Bugbot for commit 785bf98. Configure here.
|
fyi I am seeing failures when trying to run any migration commands locally after this was merged, e.g. from |


We want to standardize metrics prefixes and tags for taskworkers. Having standardized metrics makes building dashboards and alerting easier for platform teams, and makes documenting metrics for others simpler.
I've put usage of the new metrics backend behind an env var so that I can incrementally enable and validate the new metrics.
Refs STREAM-1174