Skip to content

Commit e2ea31c

Browse files
marandanetoadinauergetsentry-bot
authored
Missing unit fields for Android measurements (#2204)
Co-authored-by: Alexander Dinauer <[email protected]> Co-authored-by: Alexander Dinauer <[email protected]> Co-authored-by: Sentry Github Bot <[email protected]>
1 parent de74337 commit e2ea31c

16 files changed

+432
-22
lines changed

‎CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
### Fixes
66

77
- Fixed AbstractMethodError when getting Lifecycle ([#2228](https://github.com/getsentry/sentry-java/pull/2228))
8+
- Missing unit fields for Android measurements ([#2204](https://github.com/getsentry/sentry-java/pull/2204))
89
- Avoid sending empty profiles ([#2232](https://github.com/getsentry/sentry-java/pull/2232))
910

1011
## 6.4.1

‎sentry-android-core/src/main/java/io/sentry/android/core/ActivityFramesTracker.java

+5-3
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
package io.sentry.android.core;
22

3+
import static io.sentry.protocol.MeasurementValue.NONE_UNIT;
4+
35
import android.app.Activity;
46
import android.util.SparseIntArray;
57
import androidx.core.app.FrameMetricsAggregator;
@@ -102,9 +104,9 @@ public synchronized void setMetrics(
102104
return;
103105
}
104106

105-
final MeasurementValue tfValues = new MeasurementValue(totalFrames);
106-
final MeasurementValue sfValues = new MeasurementValue(slowFrames);
107-
final MeasurementValue ffValues = new MeasurementValue(frozenFrames);
107+
final MeasurementValue tfValues = new MeasurementValue(totalFrames, NONE_UNIT);
108+
final MeasurementValue sfValues = new MeasurementValue(slowFrames, NONE_UNIT);
109+
final MeasurementValue ffValues = new MeasurementValue(frozenFrames, NONE_UNIT);
108110
final Map<String, @NotNull MeasurementValue> measurements = new HashMap<>();
109111
measurements.put("frames_total", tfValues);
110112
measurements.put("frames_slow", sfValues);

‎sentry-android-core/src/main/java/io/sentry/android/core/PerformanceAndroidEventProcessor.java

+3-1
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import static io.sentry.android.core.ActivityLifecycleIntegration.APP_START_COLD;
44
import static io.sentry.android.core.ActivityLifecycleIntegration.APP_START_WARM;
55
import static io.sentry.android.core.ActivityLifecycleIntegration.UI_LOAD_OP;
6+
import static io.sentry.protocol.MeasurementValue.MILLISECOND_UNIT;
67

78
import io.sentry.EventProcessor;
89
import io.sentry.Hint;
@@ -65,7 +66,8 @@ public SentryEvent process(@NotNull SentryEvent event, @NotNull Hint hint) {
6566
final Long appStartUpInterval = AppStartState.getInstance().getAppStartInterval();
6667
// if appStartUpInterval is null, metrics are not ready to be sent
6768
if (appStartUpInterval != null) {
68-
final MeasurementValue value = new MeasurementValue((float) appStartUpInterval);
69+
final MeasurementValue value =
70+
new MeasurementValue((float) appStartUpInterval, MILLISECOND_UNIT);
6971

7072
final String appStartKey =
7173
AppStartState.getInstance().isColdStart() ? "app_start_cold" : "app_start_warm";

‎sentry-android-core/src/test/java/io/sentry/android/core/ActivityFramesTrackerTest.kt

+3
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ class ActivityFramesTrackerTest {
4242
val totalFrames = metrics!!["frames_total"]
4343

4444
assertEquals(totalFrames!!.value, 1f)
45+
assertEquals(totalFrames.unit, "none")
4546
}
4647

4748
@Test
@@ -57,6 +58,7 @@ class ActivityFramesTrackerTest {
5758
val frozenFrames = metrics!!["frames_frozen"]
5859

5960
assertEquals(frozenFrames!!.value, 5f)
61+
assertEquals(frozenFrames.unit, "none")
6062
}
6163

6264
@Test
@@ -72,6 +74,7 @@ class ActivityFramesTrackerTest {
7274
val slowFrames = metrics!!["frames_slow"]
7375

7476
assertEquals(slowFrames!!.value, 5f)
77+
assertEquals(slowFrames.unit, "none")
7578
}
7679

7780
@Test

‎sentry-android-core/src/test/java/io/sentry/android/core/PerformanceAndroidEventProcessorTest.kt

+16-1
Original file line numberDiff line numberDiff line change
@@ -10,10 +10,12 @@ import io.sentry.TracesSamplingDecision
1010
import io.sentry.TransactionContext
1111
import io.sentry.android.core.ActivityLifecycleIntegration.UI_LOAD_OP
1212
import io.sentry.protocol.MeasurementValue
13+
import io.sentry.protocol.MeasurementValue.MILLISECOND_UNIT
1314
import io.sentry.protocol.SentryTransaction
1415
import java.util.Date
1516
import kotlin.test.BeforeTest
1617
import kotlin.test.Test
18+
import kotlin.test.assertEquals
1719
import kotlin.test.assertTrue
1820

1921
class PerformanceAndroidEventProcessorTest {
@@ -64,6 +66,19 @@ class PerformanceAndroidEventProcessorTest {
6466
assertTrue(tr.measurements.containsKey("app_start_warm"))
6567
}
6668

69+
@Test
70+
fun `set app cold start unit measurement`() {
71+
val sut = fixture.getSut()
72+
73+
var tr = getTransaction()
74+
setAppStart()
75+
76+
tr = sut.process(tr, Hint())
77+
78+
val measurement = tr.measurements["app_start_cold"]
79+
assertEquals("millisecond", measurement?.unit)
80+
}
81+
6782
@Test
6883
fun `do not add app start metric twice`() {
6984
val sut = fixture.getSut()
@@ -140,7 +155,7 @@ class PerformanceAndroidEventProcessorTest {
140155
val tracer = SentryTracer(context, fixture.hub)
141156
var tr = SentryTransaction(tracer)
142157

143-
val metrics = mapOf("frames_total" to MeasurementValue(1f))
158+
val metrics = mapOf("frames_total" to MeasurementValue(1f, MILLISECOND_UNIT))
144159
whenever(fixture.activityFramesTracker.takeMetrics(any())).thenReturn(metrics)
145160

146161
tr = sut.process(tr, Hint())

‎sentry/api/sentry.api

+10-2
Original file line numberDiff line numberDiff line change
@@ -517,6 +517,7 @@ public final class io/sentry/JsonObjectReader : io/sentry/vendor/gson/stream/Jso
517517
public fun nextIntegerOrNull ()Ljava/lang/Integer;
518518
public fun nextList (Lio/sentry/ILogger;Lio/sentry/JsonDeserializer;)Ljava/util/List;
519519
public fun nextLongOrNull ()Ljava/lang/Long;
520+
public fun nextMapOrNull (Lio/sentry/ILogger;Lio/sentry/JsonDeserializer;)Ljava/util/Map;
520521
public fun nextObjectOrNull ()Ljava/lang/Object;
521522
public fun nextOrNull (Lio/sentry/ILogger;Lio/sentry/JsonDeserializer;)Ljava/lang/Object;
522523
public fun nextStringOrNull ()Ljava/lang/String;
@@ -2366,10 +2367,16 @@ public final class io/sentry/protocol/Gpu$JsonKeys {
23662367
public fun <init> ()V
23672368
}
23682369

2369-
public final class io/sentry/protocol/MeasurementValue : io/sentry/JsonSerializable {
2370-
public fun <init> (F)V
2370+
public final class io/sentry/protocol/MeasurementValue : io/sentry/JsonSerializable, io/sentry/JsonUnknown {
2371+
public static final field MILLISECOND_UNIT Ljava/lang/String;
2372+
public static final field NONE_UNIT Ljava/lang/String;
2373+
public fun <init> (FLjava/lang/String;)V
2374+
public fun <init> (FLjava/lang/String;Ljava/util/Map;)V
2375+
public fun getUnit ()Ljava/lang/String;
2376+
public fun getUnknown ()Ljava/util/Map;
23712377
public fun getValue ()F
23722378
public fun serialize (Lio/sentry/JsonObjectWriter;Lio/sentry/ILogger;)V
2379+
public fun setUnknown (Ljava/util/Map;)V
23732380
}
23742381

23752382
public final class io/sentry/protocol/MeasurementValue$Deserializer : io/sentry/JsonDeserializer {
@@ -2379,6 +2386,7 @@ public final class io/sentry/protocol/MeasurementValue$Deserializer : io/sentry/
23792386
}
23802387

23812388
public final class io/sentry/protocol/MeasurementValue$JsonKeys {
2389+
public static final field UNIT Ljava/lang/String;
23822390
public static final field VALUE Ljava/lang/String;
23832391
public fun <init> ()V
23842392
}

‎sentry/src/main/java/io/sentry/JsonObjectReader.java

+22
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
import java.io.Reader;
77
import java.util.ArrayList;
88
import java.util.Date;
9+
import java.util.HashMap;
910
import java.util.List;
1011
import java.util.Map;
1112
import java.util.TimeZone;
@@ -99,6 +100,27 @@ public void nextUnknown(ILogger logger, Map<String, Object> unknown, String name
99100
return list;
100101
}
101102

103+
public <T> @Nullable Map<String, T> nextMapOrNull(
104+
@NotNull ILogger logger, @NotNull JsonDeserializer<T> deserializer) throws IOException {
105+
if (peek() == JsonToken.NULL) {
106+
nextNull();
107+
return null;
108+
}
109+
beginObject();
110+
Map<String, T> map = new HashMap<>();
111+
do {
112+
try {
113+
String key = nextName();
114+
map.put(key, deserializer.deserialize(this, logger));
115+
} catch (Exception e) {
116+
logger.log(SentryLevel.ERROR, "Failed to deserialize object in map.", e);
117+
}
118+
} while (peek() == JsonToken.BEGIN_OBJECT || peek() == JsonToken.NAME);
119+
120+
endObject();
121+
return map;
122+
}
123+
102124
public <T> @Nullable T nextOrNull(
103125
@NotNull ILogger logger, @NotNull JsonDeserializer<T> deserializer) throws Exception {
104126
if (peek() == JsonToken.NULL) {

‎sentry/src/main/java/io/sentry/protocol/MeasurementValue.java

+81-5
Original file line numberDiff line numberDiff line change
@@ -5,36 +5,88 @@
55
import io.sentry.JsonObjectReader;
66
import io.sentry.JsonObjectWriter;
77
import io.sentry.JsonSerializable;
8+
import io.sentry.JsonUnknown;
9+
import io.sentry.vendor.gson.stream.JsonToken;
810
import java.io.IOException;
11+
import java.util.Map;
12+
import java.util.concurrent.ConcurrentHashMap;
913
import org.jetbrains.annotations.ApiStatus;
1014
import org.jetbrains.annotations.NotNull;
15+
import org.jetbrains.annotations.Nullable;
1116
import org.jetbrains.annotations.TestOnly;
1217

1318
@ApiStatus.Internal
14-
public final class MeasurementValue implements JsonSerializable {
19+
public final class MeasurementValue implements JsonUnknown, JsonSerializable {
20+
21+
public static final @NotNull String NONE_UNIT = "none";
22+
public static final @NotNull String MILLISECOND_UNIT = "millisecond";
23+
1524
@SuppressWarnings("UnusedVariable")
1625
private final float value;
1726

18-
public MeasurementValue(final float value) {
27+
private final @Nullable String unit;
28+
29+
/** the unknown fields of breadcrumbs, internal usage only */
30+
private @Nullable Map<String, Object> unknown;
31+
32+
public MeasurementValue(final float value, final @Nullable String unit) {
1933
this.value = value;
34+
this.unit = unit;
35+
}
36+
37+
@TestOnly
38+
public MeasurementValue(
39+
final float value, final @Nullable String unit, final @Nullable Map<String, Object> unknown) {
40+
this.value = value;
41+
this.unit = unit;
42+
this.unknown = unknown;
2043
}
2144

2245
@TestOnly
2346
public float getValue() {
2447
return value;
2548
}
2649

50+
public @Nullable String getUnit() {
51+
return unit;
52+
}
53+
54+
@Nullable
55+
@Override
56+
public Map<String, Object> getUnknown() {
57+
return unknown;
58+
}
59+
60+
@Override
61+
public void setUnknown(@Nullable Map<String, Object> unknown) {
62+
this.unknown = unknown;
63+
}
64+
2765
// JsonSerializable
2866

2967
public static final class JsonKeys {
3068
public static final String VALUE = "value";
69+
public static final String UNIT = "unit";
3170
}
3271

3372
@Override
3473
public void serialize(@NotNull JsonObjectWriter writer, @NotNull ILogger logger)
3574
throws IOException {
3675
writer.beginObject();
3776
writer.name(JsonKeys.VALUE).value(value);
77+
78+
if (unit != null) {
79+
writer.name(JsonKeys.UNIT).value(unit);
80+
}
81+
82+
if (unknown != null) {
83+
for (final String key : unknown.keySet()) {
84+
final Object value = unknown.get(key);
85+
writer.name(key);
86+
writer.value(logger, value);
87+
}
88+
}
89+
3890
writer.endObject();
3991
}
4092

@@ -43,10 +95,34 @@ public static final class Deserializer implements JsonDeserializer<MeasurementVa
4395
public @NotNull MeasurementValue deserialize(
4496
@NotNull JsonObjectReader reader, @NotNull ILogger logger) throws Exception {
4597
reader.beginObject();
46-
reader.nextName();
47-
MeasurementValue measurementValue = new MeasurementValue(reader.nextFloat());
98+
99+
String unit = null;
100+
float value = 0;
101+
Map<String, Object> unknown = null;
102+
103+
while (reader.peek() == JsonToken.NAME) {
104+
final String nextName = reader.nextName();
105+
switch (nextName) {
106+
case JsonKeys.VALUE:
107+
value = reader.nextFloat();
108+
break;
109+
case JsonKeys.UNIT:
110+
unit = reader.nextStringOrNull();
111+
break;
112+
default:
113+
if (unknown == null) {
114+
unknown = new ConcurrentHashMap<>();
115+
}
116+
reader.nextUnknown(logger, unknown, nextName);
117+
break;
118+
}
119+
}
120+
48121
reader.endObject();
49-
return measurementValue;
122+
final MeasurementValue measurement = new MeasurementValue(value, unit);
123+
measurement.setUnknown(unknown);
124+
125+
return measurement;
50126
}
51127
}
52128
}

‎sentry/src/main/java/io/sentry/protocol/SentryTransaction.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -280,8 +280,8 @@ public static final class Deserializer implements JsonDeserializer<SentryTransac
280280
reader.nextString(); // No need to assign, as it is final.
281281
break;
282282
case JsonKeys.MEASUREMENTS:
283-
Map<String, @NotNull MeasurementValue> deserializedMeasurements =
284-
(Map<String, @NotNull MeasurementValue>) reader.nextObjectOrNull();
283+
Map<String, MeasurementValue> deserializedMeasurements =
284+
reader.nextMapOrNull(logger, new MeasurementValue.Deserializer());
285285
if (deserializedMeasurements != null) {
286286
transaction.measurements.putAll(deserializedMeasurements);
287287
}

‎sentry/src/test/java/io/sentry/JsonObjectReaderTest.kt

+29
Original file line numberDiff line numberDiff line change
@@ -146,6 +146,35 @@ class JsonObjectReaderTest {
146146
assertEquals(expected, actual)
147147
}
148148

149+
// nextMap
150+
151+
@Test
152+
fun `returns null for null map`() {
153+
val jsonString = "{\"key\": null}"
154+
val reader = fixture.getSut(jsonString)
155+
reader.beginObject()
156+
reader.nextName()
157+
158+
assertNull(reader.nextMapOrNull(fixture.logger, Deserializable.Deserializer()))
159+
}
160+
161+
@Test
162+
fun `returns map of deserializables`() {
163+
val deserializableA = "{\"foo\": \"foo\", \"bar\": \"bar\"}"
164+
val deserializableB = "{\"foo\": \"fooo\", \"bar\": \"baar\"}"
165+
val jsonString = "{\"deserializable\": { \"a\":$deserializableA,\"b\":$deserializableB}}"
166+
val reader = fixture.getSut(jsonString)
167+
reader.beginObject()
168+
reader.nextName()
169+
170+
val expected = mapOf(
171+
"a" to Deserializable("foo", "bar"),
172+
"b" to Deserializable("fooo", "baar")
173+
)
174+
val actual = reader.nextMapOrNull(fixture.logger, Deserializable.Deserializer())
175+
assertEquals(expected, actual)
176+
}
177+
149178
// nextDateOrNull
150179

151180
@Test

‎sentry/src/test/java/io/sentry/protocol/MeasurementValueSerializationTest.kt

+1-1
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ class MeasurementValueSerializationTest {
1616
class Fixture {
1717
val logger = mock<ILogger>()
1818
// float cannot represent 0.3 correctly https://docs.oracle.com/cd/E19957-01/806-3568/ncg_goldberg.html
19-
fun getSut() = MeasurementValue(0.30000001192092896f)
19+
fun getSut(value: Float = 0.30000001192092896f, unit: String = "test") = MeasurementValue(value, unit, mapOf<String, Any>("new_type" to "newtype"))
2020
}
2121
private val fixture = Fixture()
2222

‎sentry/src/test/java/io/sentry/protocol/SentryTransactionSerializationTest.kt

+10-1
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,8 @@ class SentryTransactionSerializationTest {
2525
SentrySpanSerializationTest.Fixture().getSut()
2626
),
2727
mapOf(
28-
"386384cb-1162-49e7-aea1-db913d4fca63" to MeasurementValueSerializationTest.Fixture().getSut()
28+
"386384cb-1162-49e7-aea1-db913d4fca63" to MeasurementValueSerializationTest.Fixture().getSut(),
29+
"186384cb-1162-49e7-aea1-db913d4fca63" to MeasurementValueSerializationTest.Fixture().getSut(0.4000000059604645f, "test2")
2930
),
3031
TransactionInfo(TransactionNameSource.CUSTOM.apiName())
3132
).apply {
@@ -49,6 +50,14 @@ class SentryTransactionSerializationTest {
4950
assertEquals(expectedJson, actualJson)
5051
}
5152

53+
@Test
54+
fun `deserialize without measurement unit`() {
55+
val expectedJson = sanitizedFile("json/sentry_transaction_no_measurement_unit.json")
56+
val actual = deserialize(expectedJson)
57+
val actualJson = serialize(actual)
58+
assertEquals(expectedJson, actualJson)
59+
}
60+
5261
@Test
5362
fun `deserialize legacy date format and missing transaction name source`() {
5463
val expectedJson = sanitizedFile("json/sentry_transaction_legacy_date_format.json")

0 commit comments

Comments
 (0)