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 custom JsonConverters #1934

Merged
merged 5 commits into from
Sep 20, 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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
- Add `TransactionNameSource` annotation ([#1910](https://github.com/getsentry/sentry-dotnet/pull/1910))
- Use URL path in transaction names instead of "Unknown Route" ([#1919](https://github.com/getsentry/sentry-dotnet/pull/1919))
- Add `User.Segment` property ([#1920](https://github.com/getsentry/sentry-dotnet/pull/1920))
- Add support for custom `JsonConverter`s ([#1934](https://github.com/getsentry/sentry-dotnet/pull/1934))

## Fixes

Expand Down
9 changes: 7 additions & 2 deletions src/Sentry/Internal/Extensions/JsonExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,12 @@ internal static class JsonExtensions
{
// The Json options with a preset of rules that will remove dangerous and problematic
// data from the serialized object.
private static JsonSerializerOptions serializerOption = new()
internal static JsonSerializerOptions SerializerOptions { get; private set; } = GetSerializerOptions();

// For testing, we need a way to reset the options instance when we add custom converters.
internal static void ResetSerializerOptions() => SerializerOptions = GetSerializerOptions();

private static JsonSerializerOptions GetSerializerOptions() => new()
{
Converters =
{
Expand Down Expand Up @@ -344,7 +349,7 @@ public static void WriteDynamicValue(
}
else
{
JsonSerializer.Serialize(writer, value, serializerOption);
JsonSerializer.Serialize(writer, value, SerializerOptions);
}
}

Expand Down
46 changes: 46 additions & 0 deletions src/Sentry/SentryOptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,13 @@
using System.Linq;
using System.Net;
using System.Net.Http;
using System.Text.Json;
using System.Text.Json.Serialization;
using Sentry.Extensibility;
using Sentry.Http;
using Sentry.Integrations;
using Sentry.Internal;
using Sentry.Internal.Extensions;
using Sentry.Internal.Http;
using Sentry.Internal.ScopeStack;
using Sentry.PlatformAbstractions;
Expand Down Expand Up @@ -679,6 +682,49 @@ public StackTraceMode StackTraceMode
/// </summary>
public bool KeepAggregateException { get; set; }

/// <summary>
/// Adds a <see cref="JsonConverter"/> to be used when serializing or deserializing
/// objects to JSON with this SDK. For example, when custom context data might use
/// a data type that requires custom serialization logic.
/// </summary>
/// <param name="converter">The <see cref="JsonConverter"/> to add.</param>
/// <remarks>
/// This currently modifies a static list, so will affect any instance of the Sentry SDK.
/// If that becomes problematic, we will have to refactor all serialization code to be
/// able to accept an instance of <see cref="SentryOptions"/>.
/// </remarks>
public void AddJsonConverter(JsonConverter converter)
{
// protect against null because user may not have nullability annotations enabled
if (converter == null!)
{
throw new ArgumentNullException(nameof(converter));
}

// only add if we don't have this instance already
var converters = JsonExtensions.SerializerOptions.Converters;
if (converters.Contains(converter))
{
return;
}

try
{
converters.Add(converter);
}
catch (InvalidOperationException)
{
// If we've already started using the serializer, then it's too late to add more converters.
// The following exception message may occur (depending on STJ version):
// "Serializer options cannot be changed once serialization or deserialization has occurred."
// We'll swallow this, because it's likely to only have occurred in our own unit tests,
// or in a scenario where the Sentry SDK has been initialized multiple times,
// in which case we have the converter from the first initialization already.
// TODO: .NET 8 is getting an IsReadOnly flag we could check instead of catching
// See https://github.com/dotnet/runtime/pull/74431
}
}

/// <summary>
/// Provides a mechanism to convey network status to the caching transport, so that it does not attempt
/// to send cached events to Sentry when the network is offline. Used internally by some integrations.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -521,6 +521,7 @@ namespace Sentry
public System.Func<Sentry.TransactionSamplingContext, double?>? TracesSampler { get; set; }
public Sentry.Extensibility.ITransport? Transport { get; set; }
public bool UseAsyncFileIO { get; set; }
public void AddJsonConverter(System.Text.Json.Serialization.JsonConverter converter) { }
}
public static class SentryOptionsExtensions
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -520,6 +520,7 @@ namespace Sentry
public System.Func<Sentry.TransactionSamplingContext, double?>? TracesSampler { get; set; }
public Sentry.Extensibility.ITransport? Transport { get; set; }
public bool UseAsyncFileIO { get; set; }
public void AddJsonConverter(System.Text.Json.Serialization.JsonConverter converter) { }
}
public static class SentryOptionsExtensions
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -521,6 +521,7 @@ namespace Sentry
public System.Func<Sentry.TransactionSamplingContext, double?>? TracesSampler { get; set; }
public Sentry.Extensibility.ITransport? Transport { get; set; }
public bool UseAsyncFileIO { get; set; }
public void AddJsonConverter(System.Text.Json.Serialization.JsonConverter converter) { }
}
public static class SentryOptionsExtensions
{
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
null
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
"test"
26 changes: 26 additions & 0 deletions test/Sentry.Tests/SerializationTests.cs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
using System.Text.Json;
using System.Text.Json.Serialization;
using Sentry.Testing;

namespace Sentry.Tests;
Expand Down Expand Up @@ -33,5 +35,29 @@ public static IEnumerable<object[]> GetData()
yield return new object[] {"nested nullable IntPtr", new {Value = (IntPtr?)3}};
yield return new object[] {"nested UIntPtr", new {Value = (UIntPtr)3}};
yield return new object[] {"nested nullable UIntPtr", new {Value = (IntPtr?)3}};

JsonExtensions.ResetSerializerOptions();
new SentryOptions().AddJsonConverter(new CustomObjectConverter());
yield return new object[] {"custom object with value", new CustomObject("test")};
yield return new object[] {"custom object with null", new CustomObject(null)};
}

public class CustomObject
{
public CustomObject(string value)
{
Value = value;
}

internal string Value { get; }
}

public class CustomObjectConverter : JsonConverter<CustomObject>
{
public override CustomObject Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
=> new(reader.GetString());

public override void Write(Utf8JsonWriter writer, CustomObject value, JsonSerializerOptions options)
=> writer.WriteStringValue(value.Value);
}
}