Java Server Transport

The goal is to have a transport with high transaction throughput.

Constraints and requirements:

Not required by Android
Doesn't require offline caching
Flushed data doesn't need to be sent in order
Measure from baseline each change as we experiment
Metrics on dropped items. i.e: Atomic increment for dropped items so we have visibility

Some ideas:

Parallelize envelope serialization
Maintain N concurrent open connections
Producer/Consumer Envelope to Requests
- Requests are gzip compressed blobs ready for the network I/O
To test it out: Benchmarks with a fake HTTP simulating a 1ms latency.

The biggest factor is not serialization but opening HTTP connection and waiting for the result.

Low hanging fruits:

make HttpConnection reuse connections instead opening new ones - this shaves off ~400ms from each envelope and increases throughput 3x
increase the number of threads in AsyncConnection. Single thread with reusable HTTP connections can send ~8 messages per second (depending on location and how quickly Sentry server sends back the response).

Not-so-low hanging fruits - develop a non blocking version of Connection which takes into account the constraints and requirements mentioned above.

At this point microbenchmarking does not bring much value, as the time is spent on HTTP (and likely negotiating TLS) rather than constructs in our Java code.

Switching to non blocking version solves the problem completely BUT as a drawback - pulls dependencies to HTTP clients that users may not want to have on their classpath. I believe the right way to continue is to give users an easy way to provide their own instance of Connection so that they can implement sending data to Sentry using the client of their choice.

In addition to that, we will provide one NIO based implementation - based either on

as an optional dependency.

To provide custom implementation, following two interfaces would have to be implemented:

public interface ConnectionFactory {
  @NotNull Connection create(final @NotNull SentryOptions options);
}

public interface Connection {
  void send(SentryEnvelope event, @Nullable Object hint) throws IOException;

  default void send(SentryEnvelope envelope) throws IOException {
    send(envelope, null);
  }

  void close() throws IOException;
}

SentryOptions object would have a new property connectionFactory, and we would drop transport and transportGate from there.

why would we drop transport and transportGate?

https://develop.sentry.dev/sdk/unified-api/

The transport is an internal construct of the client that abstracts away the event sending. Typically the transport runs in a separate thread and gets events to send via a queue. The transport is responsible for sending, retrying and handling rate limits. The transport might also persist unsent events across restarts if needed.

I believe the connectionFactory and transport should work together and not independently.
Also, transportGate must exist unless we move the whole transport implementation for Android from the sentry package to the sentry android core package.

Ok, then I need to re-think it. Transport returns TransportResult - perhaps if we change it to return Future<TransportResult>. Currently Transport is tightly coupled with the AsyncConnection, to introduce NIO we need to change both, but at least at this stage changing Connection is not possible. Providing an option to set both connection and transport on SentryOptions also does not make sense.

yeah, merging AsyncConnection and HttpTransport makes sense, the abstraction came up from the old Java imp, but it's not needed to remain like this, it's just legacy inheritance.

it'd be fine to return a Future IMO, it's available since API 1 on Android

The signature of transport in .NET for example is just Task (aka: Future). No "TrasnportResult" there.

https://github.com/getsentry/sentry-dotnet/blob/main/src/Sentry/Extensibility/ITransport.cs#L17

Merge branch 'gh-1097-transport-connection-flip' of https://github.co…

Merge remote-tracking branch 'origin/main' into gh-1097-rate-limiter

Merge branch 'gh-1097-rate-limiter' of https://github.com/getsentry/s…

Merge remote-tracking branch 'origin/main' into gh-1097-rate-limiter

Merge branch 'main' into gh-1097-rate-limiter

Navigation Menu

Repository navigation

Java Server Transport #1097

Constraints and requirements:

Some ideas:

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Java Server Transport #1097

Description

Constraints and requirements:

Some ideas:

Activity

maciejwalkowiak commented on Dec 9, 2020

maciejwalkowiak commented on Dec 14, 2020

marandaneto commented on Dec 14, 2020

maciejwalkowiak commented on Dec 14, 2020

marandaneto commented on Dec 14, 2020

bruno-garcia commented on Dec 17, 2020

Metadata

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Issue actions