ref(metrics): Deprecate MetricsUnit in favor of MeasurementUnit - #6086
Closed
runningcode wants to merge 3 commits into
Closed
runningcode wants to merge 3 commits into
runningcode wants to merge 3 commits into
Conversation
MetricsUnit restated, as string constants, every unit MeasurementUnit already defines, because the metrics API only accepted a String unit and the string form of a MeasurementUnit is only reachable through the internal apiName(). Add MeasurementUnit overloads to IMetricsApi as default methods that delegate to the existing String overloads, so apiName() stays internal, and deprecate MetricsUnit. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…rloads Drop the MeasurementUnit overloads from IMetricsApi. Callers reach the string form through apiName(), so the metrics API surface is unchanged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
📲 Install BuildsAndroid
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
📜 Description
io.sentry.metrics.MetricsUnitduplicated, asStringconstants, every unit thatio.sentry.MeasurementUnitalready defines — the same 8 durations, 14 information sizes and 2fractions, with byte-identical values.
This deprecates
MetricsUnitand moves every call site ontoMeasurementUnit:MetricsUnitand its three nested classes are@Deprecated, pointing at theMeasurementUnitequivalents.
MetricControllers passMeasurementUnit.<Kind>.<UNIT>.apiName().IMetricsApiis untouched — it keeps taking aStringunit, sosentry.apiis byte-identical tomainand there is no new public API to review.SentryMetricsEvent.unitlikewise stays aString,since units arriving off the wire are free-form and cannot be mapped back to an enum.
One thing worth a reviewer's opinion:
MeasurementUnit.apiName()is annotated@ApiStatus.Internal, so the samples in this PR — and any user following them — now call a method theSDK marks as internal. The alternatives are to drop that annotation and commit to
apiName()aspublic API, or to add
MeasurementUnitoverloads toIMetricsApithat callapiName()internally.Both were considered and deliberately not taken here; say the word if you'd rather have one.
💡 Motivation and Context
Two parallel sets of the same units is an easy trap for users: pick the wrong one and the compiler is
happy, but the typed measurement API and the metrics API read differently for no reason.
MeasurementUnitis the older, more capable of the two — it carriesapiName()and aCustomescape hatch — so it is the one to keep.
💚 How did you test it?
ScopesTest's existing metrics tests cover the converted call sites.📝 Checklist
sendDefaultPIIis enabled.🔮 Next steps
docs/guides/metrics.mdxinsentry-docsstill importsio.sentry.metrics.MetricsUnitin its Javaand Kotlin snippets and needs a follow-up PR there.
MetricsUnitcan be deleted in v9.