fix(yabeda): Normalize plural Yabeda units to Sentry's singular form

[sentry-junior]

Description

Normalize plural Yabeda unit symbols (e.g. :seconds, :milliseconds, :bytes) to Sentry's canonical singular form (e.g. "second", "millisecond", "byte") in Sentry::Yabeda::Adapter#unit_for.

Yabeda registers units as plural Ruby symbols, but the Sentry units spec expects singular strings. Previously unit_for just called .to_s, sending unrecognized plural units. This change uses .chomp("s") to strip the trailing "s".

Changes

• sentry-yabeda/lib/sentry/yabeda/adapter.rb: Replace .to_s with .to_s.chomp("s") and remove the TODO comment
• sentry-yabeda/spec/sentry/yabeda/adapter_spec.rb: Update existing unit expectations to singular form; add dedicated "unit normalization" test group covering plural→singular, milliseconds, nil, and already-singular cases
• sentry-yabeda/spec/sentry/yabeda/integration_spec.rb: Update integration spec expectation to "millisecond"

• resolves: fix(yabeda): Normalize plural Yabeda units to Sentry's singular form #2952

[sdk-maintainer-bot]

This PR has been automatically closed. The referenced issue does not show a discussion between you and a maintainer.

To avoid wasted effort on both sides, please discuss your proposed approach in the issue first and wait for a maintainer to respond before opening a PR.

Please review our contributing guidelines for more details.

[sentry]

Bug: The use of chomp("s") to handle pluralization will incorrectly truncate any custom unit that naturally ends in "s", like :process.
_{Severity: MEDIUM}

Suggested Fix

Instead of unconditionally calling chomp("s"), use a more robust method for singularization, such as a dedicated inflection library. Alternatively, if only handling standard units, use a targeted replacement based on a known list of plural forms rather than a blanket string manipulation.

Prompt for AI Agent

Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.

Location: sentry-yabeda/lib/sentry/yabeda/adapter.rb#L72

Potential issue: The `unit_for` method unconditionally calls `.chomp("s")` on the string
representation of a metric's unit. This is problematic for custom units whose names
legitimately end in "s", such as `:process` or `:status`. In these cases, the unit will
be incorrectly truncated to `"proces"` or `"statu"` before being sent to Sentry. This
results in a mangled, unrecognized unit in Sentry without raising any error or warning,
leading to silent data corruption for those metrics. Standard Sentry units are
unaffected as they do not end in 's' in their singular form.

_{Did we get this right? 👍 / 👎 to inform future reviews.}

[dingsdax]

that's right but ok in our case, as we do have a fixed set of units for now imho

[cursor]

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 8694506. Configure here.}

[cursor]

Naive chomp("s") corrupts abbreviated unit names

Low Severity

Using chomp("s") to normalize units will mangle any unit whose singular/canonical form ends in "s". Abbreviated units like :ms, :ops, or :fps would become "m", "op", or "fp". The test "leaves already-singular units unchanged" uses :second (ending in "d"), so it doesn't actually exercise this risky scenario and gives false confidence in the approach.

Additional Locations (1)

• sentry-yabeda/spec/sentry/yabeda/adapter_spec.rb#L161-L171

 

^{Reviewed by Cursor Bugbot for commit 8694506. Configure here.}