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

Improve serialization perf and fix memory leak in SentryEvent #1693

Merged
merged 4 commits into from
Jun 7, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,9 @@

- Improve timestamp precision of transactions and spans ([#1680](https://github.com/getsentry/sentry-dotnet/pull/1680))
- Flatten AggregateException ([#1672](https://github.com/getsentry/sentry-dotnet/pull/1672))
- NOTE: This can affect grouping. You can keep the original behavior by setting the option `KeepAggregateException` to `true`.
- NOTE: This can affect grouping. You can keep the original behavior by setting the option `KeepAggregateException` to `true`.
- Serialize stack frame addresses as strings. ([#1692](https://github.com/getsentry/sentry-dotnet/pull/1692))
- Improve serialization perf and fix memory leak in `SentryEvent` ([#1693](https://github.com/getsentry/sentry-dotnet/pull/1693))

### Features

Expand Down
2 changes: 1 addition & 1 deletion src/Sentry/Envelopes/Envelope.cs
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,7 @@ public static Envelope FromSession(SessionUpdate sessionUpdate)
}

return
Json.Parse(buffer.ToArray()).GetDictionaryOrNull()
Json.Parse(buffer.ToArray(), JsonExtensions.GetDictionaryOrNull)
?? throw new InvalidOperationException("Envelope header is malformed.");
}

Expand Down
22 changes: 11 additions & 11 deletions src/Sentry/Envelopes/EnvelopeItem.cs
Original file line number Diff line number Diff line change
Expand Up @@ -317,7 +317,7 @@ internal static EnvelopeItem FromClientReport(ClientReport report)
}

return
Json.Parse(buffer.ToArray()).GetDictionaryOrNull()
Json.Parse(buffer.ToArray(), JsonExtensions.GetDictionaryOrNull)
?? throw new InvalidOperationException("Envelope item header is malformed.");
}

Expand All @@ -339,49 +339,49 @@ private static async Task<ISerializable> DeserializePayloadAsync(
{
var bufferLength = (int)(payloadLength ?? stream.Length);
var buffer = await stream.ReadByteChunkAsync(bufferLength, cancellationToken).ConfigureAwait(false);
var json = Json.Parse(buffer);
var sentryEvent = Json.Parse(buffer, SentryEvent.FromJson);

return new JsonSerializable(SentryEvent.FromJson(json));
return new JsonSerializable(sentryEvent);
}

// User report
if (string.Equals(payloadType, TypeValueUserReport, StringComparison.OrdinalIgnoreCase))
{
var bufferLength = (int)(payloadLength ?? stream.Length);
var buffer = await stream.ReadByteChunkAsync(bufferLength, cancellationToken).ConfigureAwait(false);
var json = Json.Parse(buffer);
var userFeedback = Json.Parse(buffer, UserFeedback.FromJson);

return new JsonSerializable(UserFeedback.FromJson(json));
return new JsonSerializable(userFeedback);
}

// Transaction
if (string.Equals(payloadType, TypeValueTransaction, StringComparison.OrdinalIgnoreCase))
{
var bufferLength = (int)(payloadLength ?? stream.Length);
var buffer = await stream.ReadByteChunkAsync(bufferLength, cancellationToken).ConfigureAwait(false);
var json = Json.Parse(buffer);
var transaction = Json.Parse(buffer, Transaction.FromJson);

return new JsonSerializable(Transaction.FromJson(json));
return new JsonSerializable(transaction);
}

// Session
if (string.Equals(payloadType, TypeValueSession, StringComparison.OrdinalIgnoreCase))
{
var bufferLength = (int)(payloadLength ?? stream.Length);
var buffer = await stream.ReadByteChunkAsync(bufferLength, cancellationToken).ConfigureAwait(false);
var json = Json.Parse(buffer);
var sessionUpdate = Json.Parse(buffer, SessionUpdate.FromJson);

return new JsonSerializable(SessionUpdate.FromJson(json));
return new JsonSerializable(sessionUpdate);
}

// Client Report
if (string.Equals(payloadType, TypeValueClientReport, StringComparison.OrdinalIgnoreCase))
{
var bufferLength = (int)(payloadLength ?? stream.Length);
var buffer = await stream.ReadByteChunkAsync(bufferLength, cancellationToken).ConfigureAwait(false);
var json = Json.Parse(buffer);
var clientReport = Json.Parse(buffer, ClientReport.FromJson);

return new JsonSerializable(ClientReport.FromJson(json));
return new JsonSerializable(clientReport);
}

// Arbitrary payload
Expand Down
2 changes: 1 addition & 1 deletion src/Sentry/GlobalSessionManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ public GlobalSessionManager(
_options = options;
_clock = clock ?? SystemClock.Clock;
_persistedSessionProvider = persistedSessionProvider
?? (filePath => PersistedSessionUpdate.FromJson(Json.Load(filePath)));
?? (filePath => Json.Load(filePath, PersistedSessionUpdate.FromJson));

// TODO: session file should really be process-isolated, but we
// don't have a proper mechanism for that right now.
Expand Down
13 changes: 7 additions & 6 deletions src/Sentry/Internal/Json.cs
Original file line number Diff line number Diff line change
@@ -1,27 +1,28 @@
using System;
using System.IO;
using System.Text.Json;

namespace Sentry.Internal
{
internal static class Json
{
public static JsonElement Parse(byte[] json)
public static T Parse<T>(byte[] json, Func<JsonElement, T> factory)
{
using var jsonDocument = JsonDocument.Parse(json);
return jsonDocument.RootElement.Clone();
return factory.Invoke(jsonDocument.RootElement);
}

public static JsonElement Parse(string json)
public static T Parse<T>(string json, Func<JsonElement, T> factory)
{
using var jsonDocument = JsonDocument.Parse(json);
return jsonDocument.RootElement.Clone();
return factory.Invoke(jsonDocument.RootElement);
}

public static JsonElement Load(string filePath)
public static T Load<T>(string filePath, Func<JsonElement, T> factory)
{
using var file = File.OpenRead(filePath);
using var jsonDocument = JsonDocument.Parse(file);
return jsonDocument.RootElement.Clone();
return factory.Invoke(jsonDocument.RootElement);
}
}
}
4 changes: 2 additions & 2 deletions src/Sentry/SentryEvent.cs
Original file line number Diff line number Diff line change
Expand Up @@ -273,8 +273,8 @@ public static SentryEvent FromJson(JsonElement json)
var logger = json.GetPropertyOrNull("logger")?.GetString();
var serverName = json.GetPropertyOrNull("server_name")?.GetString();
var release = json.GetPropertyOrNull("release")?.GetString();
var exceptionValues = json.GetPropertyOrNull("exception")?.GetPropertyOrNull("values")?.EnumerateArray().Select(SentryException.FromJson).Pipe(v => new SentryValues<SentryException>(v));
var threadValues = json.GetPropertyOrNull("threads")?.GetPropertyOrNull("values")?.EnumerateArray().Select(SentryThread.FromJson).Pipe(v => new SentryValues<SentryThread>(v));
var exceptionValues = json.GetPropertyOrNull("exception")?.GetPropertyOrNull("values")?.EnumerateArray().Select(SentryException.FromJson).ToList().Pipe(v => new SentryValues<SentryException>(v));
var threadValues = json.GetPropertyOrNull("threads")?.GetPropertyOrNull("values")?.EnumerateArray().Select(SentryThread.FromJson).ToList().Pipe(v => new SentryValues<SentryThread>(v));
var level = json.GetPropertyOrNull("level")?.GetString()?.ParseEnum<SentryLevel>();
var transaction = json.GetPropertyOrNull("transaction")?.GetString();
var request = json.GetPropertyOrNull("request")?.Pipe(Request.FromJson);
Expand Down
2 changes: 1 addition & 1 deletion test/Sentry.Tests/Protocol/BreadcrumbTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ public void SerializeObject_ParameterlessConstructor_IncludesTimestamp()
var sut = new Breadcrumb("test", "unit");

var actualJson = sut.ToJsonString();
var actual = Breadcrumb.FromJson(Json.Parse(actualJson));
var actual = Json.Parse(actualJson, Breadcrumb.FromJson);

Assert.NotEqual(default, actual.Timestamp);
}
Expand Down
2 changes: 1 addition & 1 deletion test/Sentry.Tests/Protocol/Context/AppTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ public void SerializeObject_AllPropertiesSetToNonDefault_SerializesValidObject()

var actualString = sut.ToJsonString();

var actual = App.FromJson(Json.Parse(actualString));
var actual = Json.Parse(actualString, App.FromJson);
actual.Should().BeEquivalentTo(sut);
}

Expand Down
2 changes: 1 addition & 1 deletion test/Sentry.Tests/Protocol/Context/BrowserTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ public void SerializeObject_AllPropertiesSetToNonDefault_SerializesValidObject()

var actualString = sut.ToJsonString();

var actual = Browser.FromJson(Json.Parse(actualString));
var actual = Json.Parse(actualString, Browser.FromJson);
actual.Should().BeEquivalentTo(sut);
}

Expand Down
18 changes: 9 additions & 9 deletions test/Sentry.Tests/Protocol/Context/ContextsTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ public void SerializeObject_NoPropertyFilled_SerializesEmptyObject()

var actualString = sut.ToJsonString();

var actual = Contexts.FromJson(Json.Parse(actualString));
var actual = Json.Parse(actualString, Contexts.FromJson);
actual.Should().BeEquivalentTo(sut);

Assert.Equal("{}", actualString);
Expand All @@ -28,7 +28,7 @@ public void SerializeObject_SingleUserDefinedKeyPropertySet_SerializeSinglePrope

var actualString = sut.ToJsonString();

var actual = Contexts.FromJson(Json.Parse(actualString));
var actual = Json.Parse(actualString, Contexts.FromJson);
actual.Should().BeEquivalentTo(sut);

Assert.Equal("{\"server\":{\"type\":\"os\",\"name\":\"Linux\"}}", actualString);
Expand All @@ -47,7 +47,7 @@ public void SerializeObject_SingleDevicePropertySet_SerializeSingleProperty()

var actualString = sut.ToJsonString();

var actual = Contexts.FromJson(Json.Parse(actualString));
var actual = Json.Parse(actualString, Contexts.FromJson);
actual.Should().BeEquivalentTo(sut);

Assert.Equal("{\"device\":{\"type\":\"device\",\"arch\":\"x86\"}}", actualString);
Expand All @@ -66,7 +66,7 @@ public void SerializeObject_SingleAppPropertySet_SerializeSingleProperty()

var actualString = sut.ToJsonString();

var actual = Contexts.FromJson(Json.Parse(actualString));
var actual = Json.Parse(actualString, Contexts.FromJson);
actual.Should().BeEquivalentTo(sut);

Assert.Equal("{\"app\":{\"type\":\"app\",\"app_name\":\"My.App\"}}", actualString);
Expand All @@ -85,7 +85,7 @@ public void SerializeObject_SingleGpuPropertySet_SerializeSingleProperty()

var actualString = sut.ToJsonString();

var actual = Contexts.FromJson(Json.Parse(actualString));
var actual = Json.Parse(actualString, Contexts.FromJson);
actual.Should().BeEquivalentTo(sut);

Assert.Equal("{\"gpu\":{\"type\":\"gpu\",\"name\":\"My.Gpu\"}}", actualString);
Expand All @@ -104,7 +104,7 @@ public void SerializeObject_SingleRuntimePropertySet_SerializeSingleProperty()

var actualString = sut.ToJsonString();

var actual = Contexts.FromJson(Json.Parse(actualString));
var actual = Json.Parse(actualString, Contexts.FromJson);
actual.Should().BeEquivalentTo(sut);

Assert.Equal("{\"runtime\":{\"type\":\"runtime\",\"version\":\"2.1.1.100\"}}", actualString);
Expand All @@ -121,7 +121,7 @@ public void SerializeObject_AnonymousObject_SerializedCorrectly()

// Act
var json = contexts.ToJsonString();
var roundtrip = Contexts.FromJson(Json.Parse(json));
var roundtrip = Json.Parse(json, Contexts.FromJson);

// Assert
json.Should().Be("{\"foo\":{\"Bar\":42,\"Baz\":\"kek\"}}");
Expand All @@ -146,7 +146,7 @@ public void Ctor_SingleBrowserPropertySet_SerializeSingleProperty()

var actualString = sut.ToJsonString();

var actual = Contexts.FromJson(Json.Parse(actualString));
var actual = Json.Parse(actualString, Contexts.FromJson);
actual.Should().BeEquivalentTo(sut);

Assert.Equal("{\"browser\":{\"type\":\"browser\",\"name\":\"Netscape 1\"}}", actualString);
Expand All @@ -165,7 +165,7 @@ public void Ctor_SingleOperatingSystemPropertySet_SerializeSingleProperty()

var actualString = sut.ToJsonString();

var actual = Contexts.FromJson(Json.Parse(actualString));
var actual = Json.Parse(actualString, Contexts.FromJson);
actual.Should().BeEquivalentTo(sut);

Assert.Equal("{\"os\":{\"type\":\"os\",\"name\":\"BeOS 1\"}}", actualString);
Expand Down
2 changes: 1 addition & 1 deletion test/Sentry.Tests/Protocol/Context/DeviceTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ public void FromJson_NonSystemTimeZone_NoException()
const string json = "{\"type\":\"device\",\"timezone\":\"tz_id\",\"timezone_display_name\":\"tz_name\"}";

// Act
var device = Device.FromJson(Json.Parse(json));
var device = Json.Parse(json, Device.FromJson);

// Assert
device.Timezone.Should().NotBeNull();
Expand Down
2 changes: 1 addition & 1 deletion test/Sentry.Tests/Protocol/DebugImageTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ public void SerializeObject_AllPropertiesSetToNonDefault_SerializesValidObject()
"}",
actual);

var parsed = DebugImage.FromJson(Json.Parse(actual));
var parsed = Json.Parse(actual, DebugImage.FromJson);

parsed.Should().BeEquivalentTo(sut);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ public void SerializeObject_AllPropertiesSetToNonDefault_SerializesValidObject()
"}",
actual);

var parsed = SentryStackFrame.FromJson(Json.Parse(actual));
var parsed = Json.Parse(actual, SentryStackFrame.FromJson);

parsed.Should().BeEquivalentTo(sut);
}
Expand Down
2 changes: 1 addition & 1 deletion test/Sentry.Tests/Protocol/SentryEventTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ public void SerializeObject_AllPropertiesSetToNonDefault_SerializesValidObject()
"{\"type\":\"wasm\",\"debug_id\":\"900f7d1b868432939de4457478f34720\"}" +
"]}");

var actual = SentryEvent.FromJson(Json.Parse(actualString));
var actual = Json.Parse(actualString, SentryEvent.FromJson);

// Assert
actual.Should().BeEquivalentTo(sut, o =>
Expand Down
2 changes: 1 addition & 1 deletion test/Sentry.Tests/Protocol/TransactionTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ public void SerializeObject_AllPropertiesSetToNonDefault_SerializesValidObject()
// Act
var finalTransaction = new Transaction(transaction);
var actualString = finalTransaction.ToJsonString();
var actual = Transaction.FromJson(Json.Parse(actualString));
var actual = Json.Parse(actualString, Transaction.FromJson);

// Assert
actual.Should().BeEquivalentTo(finalTransaction, o =>
Expand Down