Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Missing unit fields for Android measurements #2204

Merged
merged 9 commits into from
Sep 8, 2022
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 6 additions & 0 deletions CHANGELOG.md
@@ -1,5 +1,11 @@
# Changelog

## Unreleased

### Fixes

- Missing unit fields for Android measurements ([#2204](https://github.com/getsentry/sentry-java/pull/2204))

## 6.3.1

### Fixes
Expand Down
@@ -1,5 +1,7 @@
package io.sentry.android.core;

import static io.sentry.protocol.MeasurementValue.NONE_UNIT;

import android.app.Activity;
import android.util.SparseIntArray;
import androidx.core.app.FrameMetricsAggregator;
Expand Down Expand Up @@ -102,9 +104,9 @@ public synchronized void setMetrics(
return;
}

final MeasurementValue tfValues = new MeasurementValue(totalFrames);
final MeasurementValue sfValues = new MeasurementValue(slowFrames);
final MeasurementValue ffValues = new MeasurementValue(frozenFrames);
final MeasurementValue tfValues = new MeasurementValue(totalFrames, NONE_UNIT);
final MeasurementValue sfValues = new MeasurementValue(slowFrames, NONE_UNIT);
final MeasurementValue ffValues = new MeasurementValue(frozenFrames, NONE_UNIT);
final Map<String, @NotNull MeasurementValue> measurements = new HashMap<>();
measurements.put("frames_total", tfValues);
measurements.put("frames_slow", sfValues);
Expand Down
Expand Up @@ -3,6 +3,7 @@
import static io.sentry.android.core.ActivityLifecycleIntegration.APP_START_COLD;
import static io.sentry.android.core.ActivityLifecycleIntegration.APP_START_WARM;
import static io.sentry.android.core.ActivityLifecycleIntegration.UI_LOAD_OP;
import static io.sentry.protocol.MeasurementValue.MILLISECOND_UNIT;

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

final String appStartKey =
AppStartState.getInstance().isColdStart() ? "app_start_cold" : "app_start_warm";
Expand Down
Expand Up @@ -42,6 +42,7 @@ class ActivityFramesTrackerTest {
val totalFrames = metrics!!["frames_total"]

assertEquals(totalFrames!!.value, 1f)
assertEquals(totalFrames.unit, "none")
}

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

assertEquals(frozenFrames!!.value, 5f)
assertEquals(frozenFrames.unit, "none")
}

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

assertEquals(slowFrames!!.value, 5f)
assertEquals(slowFrames.unit, "none")
}

@Test
Expand Down
Expand Up @@ -10,10 +10,12 @@ import io.sentry.TracesSamplingDecision
import io.sentry.TransactionContext
import io.sentry.android.core.ActivityLifecycleIntegration.UI_LOAD_OP
import io.sentry.protocol.MeasurementValue
import io.sentry.protocol.MeasurementValue.MILLISECOND_UNIT
import io.sentry.protocol.SentryTransaction
import java.util.Date
import kotlin.test.BeforeTest
import kotlin.test.Test
import kotlin.test.assertEquals
import kotlin.test.assertTrue

class PerformanceAndroidEventProcessorTest {
Expand Down Expand Up @@ -64,6 +66,19 @@ class PerformanceAndroidEventProcessorTest {
assertTrue(tr.measurements.containsKey("app_start_warm"))
}

@Test
fun `set app cold start unit measurement`() {
val sut = fixture.getSut()

var tr = getTransaction()
setAppStart()

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

val measurement = tr.measurements["app_start_cold"]
assertEquals("millisecond", measurement?.unit)
}

@Test
fun `do not add app start metric twice`() {
val sut = fixture.getSut()
Expand Down Expand Up @@ -140,7 +155,7 @@ class PerformanceAndroidEventProcessorTest {
val tracer = SentryTracer(context, fixture.hub)
var tr = SentryTransaction(tracer)

val metrics = mapOf("frames_total" to MeasurementValue(1f))
val metrics = mapOf("frames_total" to MeasurementValue(1f, MILLISECOND_UNIT))
whenever(fixture.activityFramesTracker.takeMetrics(any())).thenReturn(metrics)

tr = sut.process(tr, Hint())
Expand Down
86 changes: 81 additions & 5 deletions sentry/src/main/java/io/sentry/protocol/MeasurementValue.java
Expand Up @@ -5,36 +5,88 @@
import io.sentry.JsonObjectReader;
import io.sentry.JsonObjectWriter;
import io.sentry.JsonSerializable;
import io.sentry.JsonUnknown;
import io.sentry.vendor.gson.stream.JsonToken;
import java.io.IOException;
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
import org.jetbrains.annotations.ApiStatus;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
import org.jetbrains.annotations.TestOnly;

@ApiStatus.Internal
public final class MeasurementValue implements JsonSerializable {
public final class MeasurementValue implements JsonUnknown, JsonSerializable {

public static final @NotNull String NONE_UNIT = "none";
public static final @NotNull String MILLISECOND_UNIT = "millisecond";

@SuppressWarnings("UnusedVariable")
private final float value;

public MeasurementValue(final float value) {
private final @Nullable String unit;

/** the unknown fields of breadcrumbs, internal usage only */
private @Nullable Map<String, Object> unknown;

public MeasurementValue(final float value, final @Nullable String unit) {
this.value = value;
this.unit = unit;
}

@TestOnly
public MeasurementValue(
final float value, final @Nullable String unit, final @Nullable Map<String, Object> unknown) {
this.value = value;
this.unit = unit;
this.unknown = unknown;
}

@TestOnly
public float getValue() {
return value;
}

public @Nullable String getUnit() {
return unit;
}

@Nullable
@Override
public Map<String, Object> getUnknown() {
return unknown;
}

@Override
public void setUnknown(@Nullable Map<String, Object> unknown) {
this.unknown = unknown;
}

// JsonSerializable

public static final class JsonKeys {
public static final String VALUE = "value";
public static final String UNIT = "unit";
}

@Override
public void serialize(@NotNull JsonObjectWriter writer, @NotNull ILogger logger)
throws IOException {
writer.beginObject();
writer.name(JsonKeys.VALUE).value(value);

if (unit != null) {
writer.name(JsonKeys.UNIT).value(unit);
}

if (unknown != null) {
for (final String key : unknown.keySet()) {
final Object value = unknown.get(key);
writer.name(key);
writer.value(logger, value);
}
}

writer.endObject();
}

Expand All @@ -43,10 +95,34 @@ public static final class Deserializer implements JsonDeserializer<MeasurementVa
public @NotNull MeasurementValue deserialize(
@NotNull JsonObjectReader reader, @NotNull ILogger logger) throws Exception {
reader.beginObject();
reader.nextName();
MeasurementValue measurementValue = new MeasurementValue(reader.nextFloat());

String unit = null;
float value = 0;
Map<String, Object> unknown = null;

while (reader.peek() == JsonToken.NAME) {
final String nextName = reader.nextName();
switch (nextName) {
case JsonKeys.VALUE:
value = reader.nextFloat();
break;
case JsonKeys.UNIT:
unit = reader.nextStringOrNull();
break;
default:
if (unknown == null) {
unknown = new ConcurrentHashMap<>();
}
reader.nextUnknown(logger, unknown, nextName);
break;
}
}

reader.endObject();
return measurementValue;
final MeasurementValue measurement = new MeasurementValue(value, unit);
measurement.setUnknown(unknown);

return measurement;
}
}
}
Expand Up @@ -16,7 +16,7 @@ class MeasurementValueSerializationTest {
class Fixture {
val logger = mock<ILogger>()
// float cannot represent 0.3 correctly https://docs.oracle.com/cd/E19957-01/806-3568/ncg_goldberg.html
fun getSut() = MeasurementValue(0.30000001192092896f)
fun getSut() = MeasurementValue(0.30000001192092896f, "test", mapOf<String, Any>("new_type" to "newtype"))
}
private val fixture = Fixture()

Expand Down
4 changes: 3 additions & 1 deletion sentry/src/test/resources/json/measurement_value.json
@@ -1,3 +1,5 @@
{
"value": 0.30000001192092896
"value": 0.30000001192092896,
"unit": "test",
"new_type": "newtype"
}