Skip to content

Commit

Permalink
Add support for custom JsonConverters (#1934)
Browse files Browse the repository at this point in the history
  • Loading branch information
mattjohnsonpint committed Sep 20, 2022
1 parent 4260f15 commit d0056ba
Show file tree
Hide file tree
Showing 9 changed files with 85 additions and 2 deletions.
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);
}
}

0 comments on commit d0056ba

Please sign in to comment.