Skip to content

Commit

Permalink
Add support for measurements at span level (#3219)
Browse files Browse the repository at this point in the history 
* added measurements to spans
* spans forward measurements to transaction
* added logs when a span or transaction is already finished
* added SentryTracer.setMeasurementFromChild method to avoid overwriting measurements when coming from the children
  • Loading branch information
@stefanosiano
stefanosiano authored Feb 28, 2024
1 parent 39e3ed7 commit 7275aa8
Show file tree
Hide file tree
Showing 19 changed files with 266 additions and 31 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

### Features

- Add support for measurements at span level ([#3219](https://github.com/getsentry/sentry-java/pull/3219))
- Add `enableScopePersistence` option to disable `PersistingScopeObserver` used for ANR reporting which may increase performance overhead. Defaults to `true` ([#3218](https://github.com/getsentry/sentry-java/pull/3218))
- When disabled, the SDK will not enrich ANRv2 events with scope data (e.g. breadcrumbs, user, tags, etc.)

Expand Down
4 changes: 3 additions & 1 deletion ...y-android-core/src/main/java/io/sentry/android/core/PerformanceAndroidEventProcessor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;

Expand Down Expand Up @@ -248,7 +249,8 @@ private static SentrySpan timeSpanToSentrySpan(
span.getDescription(),
SpanStatus.OK,
APP_METRICS_ORIGIN,
new HashMap<>(),
new ConcurrentHashMap<>(),
new ConcurrentHashMap<>(),
defaultSpanData);
}
}
4 changes: 4 additions & 0 deletions ...android-core/src/test/java/io/sentry/android/core/PerformanceAndroidEventProcessorTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,7 @@ class PerformanceAndroidEventProcessorTest {
SpanStatus.OK,
null,
emptyMap(),
emptyMap(),
null
)
tr.spans.add(appStartSpan)
Expand Down Expand Up @@ -337,6 +338,7 @@ class PerformanceAndroidEventProcessorTest {
SpanStatus.OK,
null,
emptyMap(),
emptyMap(),
null
)
tr.spans.add(appStartSpan)
Expand Down Expand Up @@ -386,6 +388,7 @@ class PerformanceAndroidEventProcessorTest {
SpanStatus.OK,
null,
emptyMap(),
emptyMap(),
null
)
tr.spans.add(appStartSpan)
Expand Down Expand Up @@ -431,6 +434,7 @@ class PerformanceAndroidEventProcessorTest {
SpanStatus.OK,
null,
emptyMap(),
emptyMap(),
null
)
tr.spans.add(appStartSpan)
Expand Down
5 changes: 4 additions & 1 deletion ...-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/MainActivity.java
View file
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
7 changes: 6 additions & 1 deletion sentry/api/sentry.api
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2501,6 +2501,8 @@ public final class io/sentry/SentryTracer : io/sentry/ITransaction {
public fun setDescription (Ljava/lang/String;)V
public fun setMeasurement (Ljava/lang/String;Ljava/lang/Number;)V
public fun setMeasurement (Ljava/lang/String;Ljava/lang/Number;Lio/sentry/MeasurementUnit;)V
public fun setMeasurementFromChild (Ljava/lang/String;Ljava/lang/Number;)V
public fun setMeasurementFromChild (Ljava/lang/String;Ljava/lang/Number;Lio/sentry/MeasurementUnit;)V
public fun setName (Ljava/lang/String;)V
public fun setName (Ljava/lang/String;Lio/sentry/protocol/TransactionNameSource;)V
public fun setOperation (Ljava/lang/String;)V
Expand Down Expand Up @@ -2605,6 +2607,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 @@ -4388,9 +4391,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 @@ -4415,6 +4419,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
77 changes: 56 additions & 21 deletions sentry/src/main/java/io/sentry/SentryTracer.java
View file
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 @@ -619,6 +615,12 @@ private boolean hasAllChildrenFinished() {
@Override
public void setOperation(final @NotNull String operation) {
if (root.isFinished()) {
hub.getOptions()
.getLogger()
.log(
SentryLevel.DEBUG,
"The transaction is already finished. Operation %s cannot be set",
operation);
return;
}

Expand All @@ -633,6 +635,12 @@ public void setOperation(final @NotNull String operation) {
@Override
public void setDescription(final @Nullable String description) {
if (root.isFinished()) {
hub.getOptions()
.getLogger()
.log(
SentryLevel.DEBUG,
"The transaction is already finished. Description %s cannot be set",
description);
return;
}

Expand All @@ -647,6 +655,12 @@ public void setDescription(final @Nullable String description) {
@Override
public void setStatus(final @Nullable SpanStatus status) {
if (root.isFinished()) {
hub.getOptions()
.getLogger()
.log(
SentryLevel.DEBUG,
"The transaction is already finished. Status %s cannot be set",
status == null ? "null" : status.name());
return;
}

Expand All @@ -661,6 +675,9 @@ public void setStatus(final @Nullable SpanStatus status) {
@Override
public void setThrowable(final @Nullable Throwable throwable) {
if (root.isFinished()) {
hub.getOptions()
.getLogger()
.log(SentryLevel.DEBUG, "The transaction is already finished. Throwable cannot be set");
return;
}

Expand All @@ -680,6 +697,9 @@ public void setThrowable(final @Nullable Throwable throwable) {
@Override
public void setTag(final @NotNull String key, final @NotNull String value) {
if (root.isFinished()) {
hub.getOptions()
.getLogger()
.log(SentryLevel.DEBUG, "The transaction is already finished. Tag %s cannot be set", key);
return;
}

Expand All @@ -699,6 +719,10 @@ public boolean isFinished() {
@Override
public void setData(@NotNull String key, @NotNull Object value) {
if (root.isFinished()) {
hub.getOptions()
.getLogger()
.log(
SentryLevel.DEBUG, "The transaction is already finished. Data %s cannot be set", key);
return;
}

Expand All @@ -710,25 +734,36 @@ public void setData(@NotNull String key, @NotNull Object value) {
return this.root.getData(key);
}

@Override
public void setMeasurement(final @NotNull String name, final @NotNull Number value) {
if (root.isFinished()) {
return;
@ApiStatus.Internal
public void setMeasurementFromChild(final @NotNull String name, final @NotNull Number value) {
// We don't want to overwrite the root span measurement, if it comes from a child.
if (!root.getMeasurements().containsKey(name)) {
setMeasurement(name, value);
}
}

this.measurements.put(name, new MeasurementValue(value, null));
@ApiStatus.Internal
public void setMeasurementFromChild(
final @NotNull String name,
final @NotNull Number value,
final @NotNull MeasurementUnit unit) {
// We don't want to overwrite the root span measurement, if it comes from a child.
if (!root.getMeasurements().containsKey(name)) {
setMeasurement(name, value, unit);
}
}

@Override
public void setMeasurement(final @NotNull String name, final @NotNull Number value) {
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 @@ -759,6 +794,12 @@ public void setName(@NotNull String name) {
@Override
public void setName(@NotNull String name, @NotNull TransactionNameSource transactionNameSource) {
if (root.isFinished()) {
hub.getOptions()
.getLogger()
.log(
SentryLevel.DEBUG,
"The transaction is already finished. Name %s cannot be set",
name);
return;
}

Expand Down Expand Up @@ -834,12 +875,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
49 changes: 43 additions & 6 deletions sentry/src/main/java/io/sentry/Span.java
View file
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,59 @@ 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()) {
hub.getOptions()
.getLogger()
.log(
SentryLevel.DEBUG,
"The span is already finished. Measurement %s cannot be set",
name);
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.setMeasurementFromChild(name, value);
}
}

@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()) {
hub.getOptions()
.getLogger()
.log(
SentryLevel.DEBUG,
"The span is already finished. Measurement %s cannot be set",
name);
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.setMeasurementFromChild(name, value, unit);
}
}

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

@Override
Expand Down
Loading

0 comments on commit 7275aa8

Please sign in to comment.