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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for measurements at span level #3219

Merged
merged 9 commits into from
Feb 28, 2024
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@

## Unreleased

### Features

- Add support for measurements at span level ([#3219](https://github.com/getsentry/sentry-java/pull/3219))

### Fixes

- Fix old profiles deletion on SDK init ([#3216](https://github.com/getsentry/sentry-java/pull/3216))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,7 @@ private static SentrySpan timeSpanToSentrySpan(
SpanStatus.OK,
APP_METRICS_ORIGIN,
new HashMap<>(),
new HashMap<>(),
stefanosiano marked this conversation as resolved.
Show resolved Hide resolved
null);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,7 @@ class PerformanceAndroidEventProcessorTest {
SpanStatus.OK,
null,
emptyMap(),
emptyMap(),
null
)
tr.spans.add(appStartSpan)
Expand Down Expand Up @@ -334,6 +335,7 @@ class PerformanceAndroidEventProcessorTest {
SpanStatus.OK,
null,
emptyMap(),
emptyMap(),
null
)
tr.spans.add(appStartSpan)
Expand Down Expand Up @@ -383,6 +385,7 @@ class PerformanceAndroidEventProcessorTest {
SpanStatus.OK,
null,
emptyMap(),
emptyMap(),
null
)
tr.spans.add(appStartSpan)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,10 @@ protected void onResume() {
screenLoadCount++;
final ISpan span = Sentry.getSpan();
if (span != null) {
span.setMeasurement("screen_load_count", screenLoadCount, new MeasurementUnit.Custom("test"));
ISpan measurementSpan = span.startChild("screen_load_measurement", "test measurement");
measurementSpan.setMeasurement(
"screen_load_count", screenLoadCount, new MeasurementUnit.Custom("test"));
measurementSpan.finish();
}
Sentry.reportFullyDisplayed();
}
Expand Down
5 changes: 4 additions & 1 deletion sentry/api/sentry.api
Original file line number Diff line number Diff line change
Expand Up @@ -2565,6 +2565,7 @@ public final class io/sentry/Span : io/sentry/ISpan {
public fun getData (Ljava/lang/String;)Ljava/lang/Object;
public fun getDescription ()Ljava/lang/String;
public fun getFinishDate ()Lio/sentry/SentryDate;
public fun getMeasurements ()Ljava/util/Map;
public fun getOperation ()Ljava/lang/String;
public fun getParentSpanId ()Lio/sentry/SpanId;
public fun getSamplingDecision ()Lio/sentry/TracesSamplingDecision;
Expand Down Expand Up @@ -4185,9 +4186,10 @@ public final class io/sentry/protocol/SentryRuntime$JsonKeys {
public final class io/sentry/protocol/SentrySpan : io/sentry/JsonSerializable, io/sentry/JsonUnknown {
public fun <init> (Lio/sentry/Span;)V
public fun <init> (Lio/sentry/Span;Ljava/util/Map;)V
public fun <init> (Ljava/lang/Double;Ljava/lang/Double;Lio/sentry/protocol/SentryId;Lio/sentry/SpanId;Lio/sentry/SpanId;Ljava/lang/String;Ljava/lang/String;Lio/sentry/SpanStatus;Ljava/lang/String;Ljava/util/Map;Ljava/util/Map;)V
public fun <init> (Ljava/lang/Double;Ljava/lang/Double;Lio/sentry/protocol/SentryId;Lio/sentry/SpanId;Lio/sentry/SpanId;Ljava/lang/String;Ljava/lang/String;Lio/sentry/SpanStatus;Ljava/lang/String;Ljava/util/Map;Ljava/util/Map;Ljava/util/Map;)V
public fun getData ()Ljava/util/Map;
public fun getDescription ()Ljava/lang/String;
public fun getMeasurements ()Ljava/util/Map;
public fun getOp ()Ljava/lang/String;
public fun getOrigin ()Ljava/lang/String;
public fun getParentSpanId ()Lio/sentry/SpanId;
Expand All @@ -4212,6 +4214,7 @@ public final class io/sentry/protocol/SentrySpan$Deserializer : io/sentry/JsonDe
public final class io/sentry/protocol/SentrySpan$JsonKeys {
public static final field DATA Ljava/lang/String;
public static final field DESCRIPTION Ljava/lang/String;
public static final field MEASUREMENTS Ljava/lang/String;
public static final field OP Ljava/lang/String;
public static final field ORIGIN Ljava/lang/String;
public static final field PARENT_SPAN_ID Ljava/lang/String;
Expand Down
24 changes: 3 additions & 21 deletions sentry/src/main/java/io/sentry/SentryTracer.java
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package io.sentry;

import io.sentry.protocol.Contexts;
import io.sentry.protocol.MeasurementValue;
import io.sentry.protocol.SentryId;
import io.sentry.protocol.SentryTransaction;
import io.sentry.protocol.TransactionNameSource;
Expand All @@ -13,7 +12,6 @@
import java.util.Map;
import java.util.Timer;
import java.util.TimerTask;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.CopyOnWriteArrayList;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.atomic.AtomicReference;
Expand Down Expand Up @@ -48,7 +46,6 @@ public final class SentryTracer implements ITransaction {

private final @NotNull Baggage baggage;
private @NotNull TransactionNameSource transactionNameSource;
private final @NotNull Map<String, MeasurementValue> measurements;
private final @NotNull Instrumenter instrumenter;
private final @NotNull Contexts contexts = new Contexts();
private final @Nullable TransactionPerformanceCollector transactionPerformanceCollector;
Expand All @@ -72,7 +69,6 @@ public SentryTracer(
final @Nullable TransactionPerformanceCollector transactionPerformanceCollector) {
Objects.requireNonNull(context, "context is required");
Objects.requireNonNull(hub, "hub is required");
this.measurements = new ConcurrentHashMap<>();

this.root =
new Span(context, this, hub, transactionOptions.getStartTimestamp(), transactionOptions);
Expand Down Expand Up @@ -263,7 +259,7 @@ public void finish(
return;
}

transaction.getMeasurements().putAll(measurements);
transaction.getMeasurements().putAll(root.getMeasurements());
hub.captureTransaction(transaction, traceContext(), hint, profilingTraceData);
}
}
Expand Down Expand Up @@ -712,23 +708,15 @@ public void setData(@NotNull String key, @NotNull Object value) {

@Override
public void setMeasurement(final @NotNull String name, final @NotNull Number value) {
if (root.isFinished()) {
return;
}

this.measurements.put(name, new MeasurementValue(value, null));
root.setMeasurement(name, value);
}

@Override
public void setMeasurement(
final @NotNull String name,
final @NotNull Number value,
final @NotNull MeasurementUnit unit) {
if (root.isFinished()) {
return;
}

this.measurements.put(name, new MeasurementValue(value, unit.apiName()));
root.setMeasurement(name, value, unit);
}

public @Nullable Map<String, Object> getData() {
Expand Down Expand Up @@ -834,12 +822,6 @@ AtomicBoolean isDeadlineTimerRunning() {
return isDeadlineTimerRunning;
}

@TestOnly
@NotNull
Map<String, MeasurementValue> getMeasurements() {
return measurements;
}

@ApiStatus.Internal
@Override
public void setContext(final @NotNull String key, final @NotNull Object context) {
Expand Down
37 changes: 31 additions & 6 deletions sentry/src/main/java/io/sentry/Span.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package io.sentry;

import io.sentry.protocol.MeasurementValue;
import io.sentry.protocol.SentryId;
import io.sentry.util.Objects;
import java.util.ArrayList;
Expand Down Expand Up @@ -41,6 +42,7 @@ public final class Span implements ISpan {
private @Nullable SpanFinishedCallback spanFinishedCallback;

private final @NotNull Map<String, Object> data = new ConcurrentHashMap<>();
private final @NotNull Map<String, MeasurementValue> measurements = new ConcurrentHashMap<>();

Span(
final @NotNull SentryId traceId,
Expand Down Expand Up @@ -323,24 +325,47 @@ public Map<String, String> getTags() {
}

@Override
public void setData(@NotNull String key, @NotNull Object value) {
public void setData(final @NotNull String key, final @NotNull Object value) {
data.put(key, value);
}

@Override
public @Nullable Object getData(@NotNull String key) {
public @Nullable Object getData(final @NotNull String key) {
return data.get(key);
}

@Override
public void setMeasurement(@NotNull String name, @NotNull Number value) {
this.transaction.setMeasurement(name, value);
public void setMeasurement(final @NotNull String name, final @NotNull Number value) {
if (isFinished()) {
stefanosiano marked this conversation as resolved.
Show resolved Hide resolved
return;
}
this.measurements.put(name, new MeasurementValue(value, null));
// We set the measurement in the transaction, too, but we have to check if this is the root span
// of the transaction, to avoid an infinite recursion
if (transaction.getRoot() != this) {
transaction.setMeasurement(name, value);
stefanosiano marked this conversation as resolved.
Show resolved Hide resolved
}
}

@Override
public void setMeasurement(
@NotNull String name, @NotNull Number value, @NotNull MeasurementUnit unit) {
this.transaction.setMeasurement(name, value, unit);
final @NotNull String name,
final @NotNull Number value,
final @NotNull MeasurementUnit unit) {
if (isFinished()) {
return;
}
this.measurements.put(name, new MeasurementValue(value, unit.apiName()));
// We set the measurement in the transaction, too, but we have to check if this is the root span
// of the transaction, to avoid an infinite recursion
if (transaction.getRoot() != this) {
transaction.setMeasurement(name, value, unit);
}
}

@NotNull
public Map<String, MeasurementValue> getMeasurements() {
return measurements;
}

@Override
Expand Down
23 changes: 23 additions & 0 deletions sentry/src/main/java/io/sentry/protocol/SentrySpan.java
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@ public final class SentrySpan implements JsonUnknown, JsonSerializable {
private final @NotNull Map<String, String> tags;
private final @Nullable Map<String, Object> data;

private @NotNull final Map<String, @NotNull MeasurementValue> measurements;

@SuppressWarnings("unused")
private @Nullable Map<String, Object> unknown;

Expand All @@ -59,6 +61,9 @@ public SentrySpan(final @NotNull Span span, final @Nullable Map<String, Object>
this.origin = span.getSpanContext().getOrigin();
final Map<String, String> tagsCopy = CollectionUtils.newConcurrentHashMap(span.getTags());
this.tags = tagsCopy != null ? tagsCopy : new ConcurrentHashMap<>();
final Map<String, MeasurementValue> measurementsCopy =
CollectionUtils.newConcurrentHashMap(span.getMeasurements());
this.measurements = measurementsCopy != null ? measurementsCopy : new HashMap<>();
stefanosiano marked this conversation as resolved.
Show resolved Hide resolved
// we lose precision here, from potential nanosecond precision down to 10 microsecond precision
this.timestamp =
span.getFinishDate() == null
Expand All @@ -82,6 +87,7 @@ public SentrySpan(
@Nullable SpanStatus status,
@Nullable String origin,
@NotNull Map<String, String> tags,
@NotNull Map<String, MeasurementValue> measurements,
@Nullable Map<String, Object> data) {
this.startTimestamp = startTimestamp;
this.timestamp = timestamp;
Expand All @@ -93,6 +99,7 @@ public SentrySpan(
this.status = status;
this.tags = tags;
this.data = data;
this.measurements = measurements;
this.origin = origin;
}

Expand Down Expand Up @@ -144,6 +151,10 @@ public boolean isFinished() {
return origin;
}

public @NotNull Map<String, MeasurementValue> getMeasurements() {
return measurements;
}

// JsonSerializable

public static final class JsonKeys {
Expand All @@ -158,6 +169,7 @@ public static final class JsonKeys {
public static final String ORIGIN = "origin";
public static final String TAGS = "tags";
public static final String DATA = "data";
public static final String MEASUREMENTS = "measurements";
}

@Override
Expand Down Expand Up @@ -189,6 +201,9 @@ public void serialize(final @NotNull ObjectWriter writer, final @NotNull ILogger
if (data != null) {
writer.name(JsonKeys.DATA).value(logger, data);
}
if (!measurements.isEmpty()) {
writer.name(JsonKeys.MEASUREMENTS).value(logger, measurements);
}
if (unknown != null) {
for (String key : unknown.keySet()) {
Object value = unknown.get(key);
Expand Down Expand Up @@ -233,6 +248,7 @@ public static final class Deserializer implements JsonDeserializer<SentrySpan> {
String origin = null;
Map<String, String> tags = null;
Map<String, Object> data = null;
Map<String, MeasurementValue> measurements = null;

Map<String, Object> unknown = null;
while (reader.peek() == JsonToken.NAME) {
Expand Down Expand Up @@ -281,6 +297,9 @@ public static final class Deserializer implements JsonDeserializer<SentrySpan> {
case JsonKeys.DATA:
data = (Map<String, Object>) reader.nextObjectOrNull();
break;
case JsonKeys.MEASUREMENTS:
measurements = reader.nextMapOrNull(logger, new MeasurementValue.Deserializer());
break;
default:
if (unknown == null) {
unknown = new ConcurrentHashMap<>();
Expand All @@ -304,6 +323,9 @@ public static final class Deserializer implements JsonDeserializer<SentrySpan> {
if (tags == null) {
tags = new HashMap<>();
}
if (measurements == null) {
measurements = new HashMap<>();
}
SentrySpan sentrySpan =
new SentrySpan(
startTimestamp,
Expand All @@ -316,6 +338,7 @@ public static final class Deserializer implements JsonDeserializer<SentrySpan> {
status,
origin,
tags,
measurements,
data);
sentrySpan.setUnknown(unknown);
reader.endObject();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,9 @@ public SentryTransaction(
this.timestamp = timestamp;
this.spans.addAll(spans);
this.measurements.putAll(measurements);
for (SentrySpan span : spans) {
stefanosiano marked this conversation as resolved.
Show resolved Hide resolved
this.measurements.putAll(span.getMeasurements());
}
this.transactionInfo = transactionInfo;
}

Expand Down
3 changes: 3 additions & 0 deletions sentry/src/test/java/io/sentry/JsonSerializerTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -1019,6 +1019,8 @@ class JsonSerializerTest {
assertEquals("value1", it)
}
}
assertEquals(1, deserialized?.measurements?.get("test_measurement")?.value)
assertEquals("test", deserialized?.measurements?.get("test_measurement")?.unit)
}

@Test
Expand Down Expand Up @@ -1300,6 +1302,7 @@ class JsonSerializerTest {
}
val tracer = SentryTracer(trace, fixture.hub)
val span = tracer.startChild("child")
span.setMeasurement("test_measurement", 1, MeasurementUnit.Custom("test"))
span.finish(SpanStatus.OK)
tracer.finish()
return span
Expand Down
2 changes: 1 addition & 1 deletion sentry/src/test/java/io/sentry/SentryTracerTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -465,7 +465,7 @@ class SentryTracerTest {
assertEquals("desc", transaction.description)
assertEquals("myValue", transaction.getTag("myTag"))
assertEquals("myValue", transaction.getData("myData"))
assertEquals(1.0f, transaction.measurements["myMetric"]!!.value)
assertEquals(1.0f, transaction.root.measurements["myMetric"]!!.value)
assertEquals("name", transaction.name)
assertEquals(ex, transaction.throwable)
}
Expand Down
Loading