Repository navigation
chore(deps): Bump sentry-arroyo to 2.47.0 - #391
Conversation
Bump both the Python package and the Rust crate. Arroyo 2.44.0 moved the Rust crate to metrics-rs and removed its own recorder API, so drop the arroyo Recorder adapter and instead wrap the DogStatsD recorder to keep arroyo metrics under the streams.pipeline namespace. Kafka producer constructors now return Result; keep the previous panic-on-failure behavior with expect(). Co-Authored-By: Markus Unterwaditzer <markus@sentry.io>
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 53c69f7. Configure here.
| fn prefixed_name(name: &str) -> Option<String> { | ||
| name.starts_with(ARROYO_METRIC_PREFIX) | ||
| .then(|| format!("{STREAMS_METRIC_PREFIX}{name}")) |
There was a problem hiding this comment.
Should arroyo metrics be prefixed with streams.pipeline.? We could add a global application metric label for arroyo metrics using configure_scope: https://github.com/getsentry/relay/blob/46f3659920eefc75b61e03fc5723a603d98b9dc9/relay/src/setup.rs#L110-L112
There was a problem hiding this comment.
Happy to do that, but it's a naming decision, so I'd like a call before changing it. Tradeoffs:
- Keep the prefix (current PR): Rust arroyo metrics keep the names they had before the bump (
streams.pipeline.arroyo.*; the oldMetricsFacadeRecorderadded the same prefix). They also match the Python side, whereDatadogMetricsBackenduses thestreams.pipelinenamespace for arroyo metrics too. No dashboard or monitor changes. - Drop the prefix and use
configure_scope: this is cleaner, and arroyo metrics would match other arroyo users like relay. But the Rust series get renamed toarroyo.*, which breaks existing dashboards and monitors. Rust and Python would also diverge unless we change the Python namespace in the same PR. Pipeline tags already reach arroyo metrics through DogStatsDwith_global_labels, soconfigure_scopewould only be needed for an extra label likeapplication.
Should I switch to the unprefixed names plus configure_scope, or keep this PR name-preserving and do the rename as a follow-up?
There was a problem hiding this comment.
as part of this PR i think we should just retain compat, if you want to unprefix then i would do it in a separate pr
i'm mainly trying to bump arroyo now across the board so that we're unblocked across the various workstreams
Bumps arroyo to 2.47.0 on both sides:
sentry-arroyo>=2.47.0in Python, and thesentry_arroyocrate to2.47.0, which drops the<2.44.0cap.Two arroyo 2.44.0 breaking changes needed code changes in the Rust runtime:
sentry_arroyo::metrics(Recorder,init,MetricValue) is gone, so theMetricsFacadeRecorderadapter insrc/metrics.rsis removed. Arroyo now emitsarroyo.*metrics straight to the global metrics-rs recorder. To keep the dashboard names and the Python side unchanged, the DogStatsD recorder is now wrapped in a small recorder that renamesarroyo.*keys tostreams.pipeline.arroyo.*. Streams' own metrics already start withstreams.pipeline.*, so they pass through unchanged.Result(fix(producer): make Rust Kafka producer constructors fallible arroyo#563).KafkaProducer::newused to panic on failure. The StreamSink and DLQ producers now call.expect(...), so the failure behavior stays the same.The other changes in the range didn't need code changes here: the
ProducerError::Kafkavariant (we don't match on it), the wider rdkafka range (we stay on 0.37), the produce-status tag changes, and the additions on the Python side.This overlaps with #390, which only raises the Python requirement to
>=2.46.0,<2.47.0.via markus.
--
View Junior Session [Sentry]