Skip to content
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
74 commits
Select commit Hold shift + click to select a range
e418a43
Make serialization and envelope item creation more generic to handle …
maciejwalkowiak Oct 8, 2020
fb2dd7e
Polish.
maciejwalkowiak Oct 9, 2020
d90e6f5
Add ability to send transaction.
maciejwalkowiak Oct 9, 2020
a8f183c
Remove ConvertibleToEnvelopeItem.java.
maciejwalkowiak Oct 9, 2020
f60144e
Polish.
maciejwalkowiak Oct 12, 2020
1da297b
Add more options to TransactionContexts.
maciejwalkowiak Oct 12, 2020
725eca0
WIP
maciejwalkowiak Oct 14, 2020
0ca29c1
Merge remote-tracking branch 'origin/main' into performance
maciejwalkowiak Oct 22, 2020
2dacfb7
Polish domain classes.
maciejwalkowiak Oct 23, 2020
2fe4df3
Test SentryClient captureTransaction.
maciejwalkowiak Oct 23, 2020
41d493f
Make TransactionContexts inherit from Contexts
maciejwalkowiak Oct 23, 2020
7260ae0
Attach Transaction to the scope.
maciejwalkowiak Oct 23, 2020
6f4855b
Polish.
maciejwalkowiak Oct 25, 2020
afd5c04
Return sentry-trace header in Transaction.
maciejwalkowiak Oct 25, 2020
557ac3e
Add span.
maciejwalkowiak Oct 25, 2020
23cd265
Setting transaction on Scope changes transaction name.
maciejwalkowiak Oct 25, 2020
e79a0b0
Add tests.
maciejwalkowiak Oct 26, 2020
a9f2da5
Add changelog.
maciejwalkowiak Oct 26, 2020
d973b8e
Create contexts from the traceparent header.
maciejwalkowiak Oct 26, 2020
0e18588
Rename tx to transaction on Scope.
maciejwalkowiak Oct 26, 2020
86eeb87
Add test.
maciejwalkowiak Oct 26, 2020
9e5180d
Add test.
maciejwalkowiak Oct 26, 2020
3cbb1da
Add missing SpanStatuses.
maciejwalkowiak Oct 26, 2020
d1c3d57
Remove cloning from Transaction and related classes.
maciejwalkowiak Oct 26, 2020
e2e6763
Polish Transaction API.
maciejwalkowiak Oct 26, 2020
e978227
Polish Transaction API.
maciejwalkowiak Oct 26, 2020
f6a6e16
Get current span from the scope.
maciejwalkowiak Oct 27, 2020
bad07df
Rename traceparent to sentryHeader.
maciejwalkowiak Oct 28, 2020
f50f042
Remove platform field from Transaction.
maciejwalkowiak Oct 28, 2020
e0a8f0a
Polish.
maciejwalkowiak Oct 28, 2020
8d62fb1
Throw checked exception when creating contexts from trace header.
maciejwalkowiak Oct 28, 2020
3e2c740
Raname sentryHeader to sentryTrace
maciejwalkowiak Oct 28, 2020
f14de05
Optimise getting span from transaction.
maciejwalkowiak Oct 28, 2020
048a980
Rename Trace to TraceContext.
maciejwalkowiak Oct 28, 2020
e197af5
Add missing methods, move request to SentryBaseEvent, clear transaction.
maciejwalkowiak Oct 29, 2020
4f39a9a
Polish.
maciejwalkowiak Oct 29, 2020
87c070b
Merge remote-tracking branch 'origin/main' into performance
maciejwalkowiak Oct 29, 2020
7dd242a
Rename Transaction to SentryTransaction and reformat code.
maciejwalkowiak Oct 29, 2020
fa0d04c
Merge remote-tracking branch 'origin/main' into performance
maciejwalkowiak Oct 31, 2020
ae84550
Merge remote-tracking branch 'origin/main' into performance
maciejwalkowiak Nov 3, 2020
0d59ee2
Add performance sampling. (#1025)
maciejwalkowiak Nov 4, 2020
0a67f0c
Support deferred sample decision.
maciejwalkowiak Nov 6, 2020
25e221b
Simplify domain model for Performance.
maciejwalkowiak Nov 9, 2020
a8ba82d
Attach events to active spans.
maciejwalkowiak Nov 9, 2020
ec7aa9e
Do not use events sampling strategy for transactions.
maciejwalkowiak Nov 9, 2020
032f6cb
Add beforeTransaction callback.
maciejwalkowiak Nov 9, 2020
5dd3cce
Merge remote-tracking branch 'origin/main' into performance
maciejwalkowiak Nov 9, 2020
12bc6bb
Polish.
maciejwalkowiak Nov 9, 2020
ffa560c
Polish.
maciejwalkowiak Nov 9, 2020
19fc07e
Merge remote-tracking branch 'origin/main' into performance
maciejwalkowiak Nov 10, 2020
1e60688
Drop beforeTransaction
maciejwalkowiak Nov 11, 2020
f1b050a
Add TransactionContext.
maciejwalkowiak Nov 11, 2020
4d1af25
Pass customSamplingContext to startTransaction.
maciejwalkowiak Nov 11, 2020
cd26031
Remove parentSampled from SamplingContext.
maciejwalkowiak Nov 11, 2020
c222864
Fix formatting.
maciejwalkowiak Nov 11, 2020
3c90862
Add ability to attach exception to a span.
maciejwalkowiak Nov 11, 2020
4b7d88f
Polish.
maciejwalkowiak Nov 11, 2020
75ff4bf
Cleanup.
maciejwalkowiak Nov 12, 2020
3b03e3a
Add performance sample.
maciejwalkowiak Nov 13, 2020
52ba943
Fix formatting.
maciejwalkowiak Nov 13, 2020
8a6fe85
Polish.
maciejwalkowiak Nov 16, 2020
ee2bbe4
Reformat.
maciejwalkowiak Nov 16, 2020
b593dc5
Fix thread-safety.
maciejwalkowiak Nov 16, 2020
91ff82b
Move exceptions to `exceptions` package.
maciejwalkowiak Nov 16, 2020
6eba18d
Polish.
maciejwalkowiak Nov 16, 2020
3fd9c6f
Polish.
maciejwalkowiak Nov 16, 2020
aef2463
Code review fixes.
maciejwalkowiak Nov 17, 2020
503c6a4
Fix concurrent usage of getting span from the scope.
maciejwalkowiak Nov 17, 2020
ef16d98
Merge remote-tracking branch 'origin/main' into performance
maciejwalkowiak Nov 17, 2020
4e7e0fb
Merge remote-tracking branch 'origin/main' into performance
maciejwalkowiak Nov 19, 2020
d46dc3c
Add setTag to ISpan.
maciejwalkowiak Nov 20, 2020
b9d65b3
Drop the idea of WeakReference on Span#throwable.
maciejwalkowiak Nov 20, 2020
a568d39
Bring back an old API for SentryEnvelopeItem.
maciejwalkowiak Nov 20, 2020
1bfb3ad
Performance Feature for Spring & Spring Boot (#1024)
maciejwalkowiak Nov 20, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions sentry/src/main/java/io/sentry/DiagnosticLogger.java
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ public DiagnosticLogger(final @NotNull SentryOptions options, final @Nullable IL
* @param level The SentryLevel to test against.
* @return True if a log message would be recorded for the level. Otherwise false.
*/
@Override
public boolean isEnabled(final @Nullable SentryLevel level) {
final SentryLevel diagLevel = options.getDiagnosticLevel();
if (level == null) {
Expand Down
53 changes: 27 additions & 26 deletions sentry/src/main/java/io/sentry/GsonSerializer.java
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,14 @@
import io.sentry.adapters.SentryIdSerializerAdapter;
import io.sentry.adapters.SentryLevelDeserializerAdapter;
import io.sentry.adapters.SentryLevelSerializerAdapter;
import io.sentry.adapters.SpanIdDeserializerAdapter;
import io.sentry.adapters.SpanIdSerializerAdapter;
import io.sentry.adapters.SpanStatusDeserializerAdapter;
import io.sentry.adapters.SpanStatusSerializerAdapter;
import io.sentry.adapters.TimeZoneDeserializerAdapter;
import io.sentry.adapters.TimeZoneSerializerAdapter;
import io.sentry.adapters.TransactionContextsDeserializerAdapter;
import io.sentry.adapters.TransactionContextsSerializerAdapter;
import io.sentry.protocol.Contexts;
import io.sentry.protocol.Device;
import io.sentry.protocol.SentryId;
Expand Down Expand Up @@ -90,6 +96,14 @@ Device.DeviceOrientation.class, new OrientationDeserializerAdapter(logger))
.registerTypeAdapter(SentryEnvelopeHeader.class, new SentryEnvelopeHeaderAdapter())
.registerTypeAdapter(SentryEnvelopeItemHeader.class, new SentryEnvelopeItemHeaderAdapter())
.registerTypeAdapter(Session.class, new SessionAdapter(logger))
.registerTypeAdapter(SpanId.class, new SpanIdDeserializerAdapter(logger))
.registerTypeAdapter(SpanId.class, new SpanIdSerializerAdapter(logger))
.registerTypeAdapter(SpanStatus.class, new SpanStatusDeserializerAdapter(logger))
.registerTypeAdapter(SpanStatus.class, new SpanStatusSerializerAdapter(logger))
.registerTypeAdapter(
TransactionContexts.class, new TransactionContextsSerializerAdapter(logger))
.registerTypeAdapter(
TransactionContexts.class, new TransactionContextsDeserializerAdapter(logger))
.create();
}

Expand Down Expand Up @@ -119,6 +133,13 @@ Device.DeviceOrientation.class, new OrientationDeserializerAdapter(logger))
return gson.fromJson(reader, Session.class);
}

@Override
public Transaction deserializeTransaction(Reader reader) {
Objects.requireNonNull(reader, "The Reader object is required.");

return gson.fromJson(reader, Transaction.class);
}

/**
* Deserialize a SentryEnvelope from a InputStream (Envelope+JSON)
*
Expand All @@ -136,37 +157,17 @@ Device.DeviceOrientation.class, new OrientationDeserializerAdapter(logger))
}
}

/**
* Serialize a SentryEvent to a stream Writer (JSON)
*
* @param event the SentryEvent
* @param writer the Writer
* @throws IOException an IOException
*/
@Override
public void serialize(final @NotNull SentryEvent event, final @NotNull Writer writer)
public <T> void serialize(final @NotNull T entity, final @NotNull Writer writer)
throws IOException {
Objects.requireNonNull(event, "The SentryEvent object is required.");
Objects.requireNonNull(entity, "The entity is required.");
Objects.requireNonNull(writer, "The Writer object is required.");

gson.toJson(event, SentryEvent.class, writer);
writer.flush();
}

/**
* Serialize a Session to a stream Writer (JSON)
*
* @param session the Session
* @param writer the Writer
* @throws IOException an IOException
*/
@Override
public void serialize(final @NotNull Session session, final @NotNull Writer writer)
throws IOException {
Objects.requireNonNull(session, "The Session object is required.");
Objects.requireNonNull(writer, "The Writer object is required.");
if (logger.isEnabled(SentryLevel.DEBUG)) {
logger.log(SentryLevel.DEBUG, "Serializing object: %s", gson.toJson(entity));
}
gson.toJson(entity, entity.getClass(), writer);

gson.toJson(session, Session.class, writer);
writer.flush();
}

Expand Down
61 changes: 57 additions & 4 deletions sentry/src/main/java/io/sentry/Hub.java
Original file line number Diff line number Diff line change
Expand Up @@ -454,9 +454,7 @@ public void pushScope() {
if (!isEnabled()) {
options
.getLogger()
.log(
SentryLevel.WARNING,
"Instance is disabled and this 'pushScope' call is a no-op.");
.log(SentryLevel.WARNING, "Instance is disabled and this 'pushScope' call is a no-op.");
} else {
final StackItem item = stack.peek();
if (item != null) {
Expand Down Expand Up @@ -523,7 +521,9 @@ public void configureScope(final @NotNull ScopeCallback callback) {
if (!isEnabled()) {
options
.getLogger()
.log(SentryLevel.WARNING, "Instance is disabled and this 'configureScope' call is a no-op.");
.log(
SentryLevel.WARNING,
"Instance is disabled and this 'configureScope' call is a no-op.");
} else {
final StackItem item = stack.peek();
if (item != null) {
Expand Down Expand Up @@ -601,4 +601,57 @@ public void flush(long timeoutMillis) {
}
return clone;
}

@Override
public SentryId captureTransaction(Transaction transaction, Object hint) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hint is not used anywhere. Should we keep it for the sake of being future-proof or remove and add another method when it's needed?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it'd be useful if we have a sort of beforeTransaction callback for filtering.
I dont see any hint here though https://develop.sentry.dev/sdk/unified-api/tracing/
cc @HazAT do you know?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree.

There's no callback as of yet, I believe other SDKs (mis)use event processors that go through it.

Folks will get spammed by load balancer requests so we need to consider a way out of transactions before GA (not this PR)

SentryId sentryId = SentryId.EMPTY_ID;
if (!isEnabled()) {
options
.getLogger()
.log(
SentryLevel.WARNING,
"Instance is disabled and this 'captureTransaction' call is a no-op.");
} else if (transaction == null) {
options
.getLogger()
.log(SentryLevel.WARNING, "captureTransaction called with null parameter.");
} else {
try {
final StackItem item = stack.peek();
if (item != null) {
sentryId = item.client.captureTransaction(transaction, item.scope, hint);
} else {
options.getLogger().log(SentryLevel.FATAL, "Stack peek was null when captureTransaction");
}
} catch (Exception e) {
options
.getLogger()
.log(
SentryLevel.ERROR,
"Error while capturing event with id: " + transaction.getEventId(),
e);
}
}
this.lastEventId = sentryId;
return sentryId;
}

@Override
public @Nullable Transaction startTransaction(TransactionContexts transactionContexts) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What should happen when this method is called but another transaction is already started?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

at least for sessions, we stop and capture the current one and start a new one, I believe it should be the same thing.

Transaction transaction = null;
if (!isEnabled()) {
options
.getLogger()
.log(SentryLevel.WARNING, "Instance is disabled and this 'setExtra' call is a no-op.");
} else {
final StackItem item = stack.peek();
if (item != null) {
transaction = new Transaction(transactionContexts);
item.scope.setTx(transaction);
} else {
options.getLogger().log(SentryLevel.FATAL, "Stack peek was null when setExtra");
}
}
return transaction;
}
}
10 changes: 10 additions & 0 deletions sentry/src/main/java/io/sentry/HubAdapter.java
Original file line number Diff line number Diff line change
Expand Up @@ -146,4 +146,14 @@ public void flush(long timeoutMillis) {
public IHub clone() {
return Sentry.getCurrentHub().clone();
}

@Override
public SentryId captureTransaction(Transaction transaction, Object hint) {
return Sentry.captureTransaction(transaction, hint);
}

@Override
public Transaction startTransaction(TransactionContexts transactionContexts) {
return Sentry.startTransaction(transactionContexts);
}
}
17 changes: 17 additions & 0 deletions sentry/src/main/java/io/sentry/IHub.java
Original file line number Diff line number Diff line change
Expand Up @@ -249,4 +249,21 @@ default void addBreadcrumb(@NotNull String message, @NotNull String category) {
* @return the cloned Hub
*/
IHub clone();

/**
* Captures the transaction and enqueues it for sending to Sentry server.
*
* @param transaction the transaction
* @param hint the hint
* @return transaction's id
*/
SentryId captureTransaction(Transaction transaction, Object hint);

/**
* Creates a Transaction bound to the current hub and returns the instance.
*
* @param transactionContexts the transaction contexts
* @return created transaction
*/
Transaction startTransaction(TransactionContexts transactionContexts);
}
12 changes: 12 additions & 0 deletions sentry/src/main/java/io/sentry/ILogger.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package io.sentry;

import org.jetbrains.annotations.Nullable;

/** Sentry SDK internal logging interface. */
public interface ILogger {

Expand Down Expand Up @@ -30,4 +32,14 @@ public interface ILogger {
* @param args the formatting arguments
*/
void log(SentryLevel level, Throwable throwable, String message, Object... args);

/**
* Whether this logger is enabled for the specified SentryLevel.
*
* @param level The SentryLevel to test against.
* @return True if a log message would be recorded for the level. Otherwise false.
*/
default boolean isEnabled(final @Nullable SentryLevel level) {
return true;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm afraid of this though. So we will say log is enabled and incur the cost of whatever the caller was calling this to check for.

Meaning: isEnabled(debug) { do expensive stuff;

What about we default to false? Error on the side of caution?
To be honest both are suboptional

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is just for the sake of raising isEnabled method visibility level and not implementing this method in all loggers. DiagnosticLogger overwrites it with proper implementation.

If we default it to false we should change SystemOutLogger implementation to not log anything or overwrite this method to return always true.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we default it to false we should change SystemOutLogger implementation to not log anything or overwrite this method to return always true.

that sounds good to me

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have left it without default implementation so that it does not silently stop working in case some of our users implemented custom loggers.

}
}
6 changes: 6 additions & 0 deletions sentry/src/main/java/io/sentry/ISentryClient.java
Original file line number Diff line number Diff line change
Expand Up @@ -178,4 +178,10 @@ default void captureSession(Session session) {
default SentryId captureEnvelope(SentryEnvelope envelope) {
return captureEnvelope(envelope, null);
}

SentryId captureTransaction(Transaction transaction, Scope scope, Object hint);

default SentryId captureTransaction(Transaction transaction) {
return captureTransaction(transaction, null, null);
}
}
6 changes: 3 additions & 3 deletions sentry/src/main/java/io/sentry/ISerializer.java
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,11 @@ public interface ISerializer {

Session deserializeSession(Reader reader);

SentryEnvelope deserializeEnvelope(InputStream inputStream);
Transaction deserializeTransaction(Reader reader);

void serialize(SentryEvent event, Writer writer) throws IOException;
SentryEnvelope deserializeEnvelope(InputStream inputStream);

void serialize(Session session, Writer writer) throws IOException;
<T> void serialize(T entity, Writer writer) throws IOException;

void serialize(SentryEnvelope envelope, Writer writer) throws Exception;

Expand Down
10 changes: 10 additions & 0 deletions sentry/src/main/java/io/sentry/NoOpHub.java
Original file line number Diff line number Diff line change
Expand Up @@ -106,4 +106,14 @@ public void flush(long timeoutMillis) {}
public IHub clone() {
return instance;
}

@Override
public SentryId captureTransaction(Transaction transaction, Object hint) {
return SentryId.EMPTY_ID;
}

@Override
public Transaction startTransaction(TransactionContexts transactionContexts) {
return null;
}
}
5 changes: 5 additions & 0 deletions sentry/src/main/java/io/sentry/NoOpSentryClient.java
Original file line number Diff line number Diff line change
Expand Up @@ -36,4 +36,9 @@ public void captureSession(Session session, @Nullable Object hint) {}
public SentryId captureEnvelope(SentryEnvelope envelope, @Nullable Object hint) {
return SentryId.EMPTY_ID;
}

@Override
public SentryId captureTransaction(Transaction transaction, Scope scope, Object hint) {
return SentryId.EMPTY_ID;
}
}
8 changes: 5 additions & 3 deletions sentry/src/main/java/io/sentry/NoOpSerializer.java
Original file line number Diff line number Diff line change
Expand Up @@ -28,15 +28,17 @@ public Session deserializeSession(Reader reader) {
}

@Override
public SentryEnvelope deserializeEnvelope(InputStream inputStream) {
public Transaction deserializeTransaction(Reader reader) {
return null;
}

@Override
public void serialize(SentryEvent event, Writer writer) {}
public SentryEnvelope deserializeEnvelope(InputStream inputStream) {
return null;
}

@Override
public void serialize(Session session, Writer writer) throws IOException {}
public <T> void serialize(T entity, Writer writer) throws IOException {}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not propsing to change this but:
What I dislike about this approach is that now this method compiles with any T and we surely can't serialize anything. It's a leaky abstraction

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Technically we can serialize anything that Gson is able to serialize. If we want to make it explicit what can be serialized perhaps we can introduce a a marker interface?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah a marker interface would be ideal, but we could add this as a task along with moving away from reflection for serializing things, so we do a bigger refactoring and just one breaking change.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍


@Override
public void serialize(SentryEnvelope envelope, Writer outputStream) throws Exception {}
Expand Down
14 changes: 14 additions & 0 deletions sentry/src/main/java/io/sentry/Scope.java
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,9 @@ public final class Scope implements Cloneable {
/** Scope's transaction */
private @Nullable String transaction;

/** Scope's {@link Transaction}. */
private @Nullable Transaction tx;

/** Scope's user */
private @Nullable User user;

Expand Down Expand Up @@ -100,6 +103,14 @@ public void setTransaction(@Nullable String transaction) {
this.transaction = transaction;
}

public Transaction getTx() {
return tx;
}

public void setTx(Transaction tx) {
this.tx = tx;
}

/**
* Returns the Scope's user
*
Expand Down Expand Up @@ -448,6 +459,9 @@ public void removeContexts(final @NotNull String key) {

clone.contexts = contexts.clone();

final Transaction txRef = tx;
clone.tx = txRef != null ? txRef.clone() : null;

return clone;
}

Expand Down
21 changes: 21 additions & 0 deletions sentry/src/main/java/io/sentry/Sentry.java
Original file line number Diff line number Diff line change
Expand Up @@ -484,6 +484,27 @@ public static void endSession() {
getCurrentHub().endSession();
}

/**
* Captures the transaction and enqueues it for sending to Sentry server.
*
* @param transaction the transaction
* @param hint the hint
* @return transaction's id
*/
public static SentryId captureTransaction(Transaction transaction, Object hint) {
return getCurrentHub().captureTransaction(transaction, hint);
}

/**
* Creates a Transaction bound to the current hub and returns the instance.
*
* @param transactionContexts the transaction contexts
* @return created transaction
*/
public static Transaction startTransaction(TransactionContexts transactionContexts) {
return getCurrentHub().startTransaction(transactionContexts);
}

/**
* Configuration options callback
*
Expand Down
Loading