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.

Sign up for GitHub

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

Jump to bottom

[geneva] Nullable annotations for TLDExporter folder #2085

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

[geneva] Nullable annotations for TLDExporter folder #2085

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
28 changes: 15 additions & 13 deletions src/OpenTelemetry.Exporter.Geneva/TLDExporter/JsonSerializer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
// Copyright The OpenTelemetry Authors
// SPDX-License-Identifier: Apache-2.0

#nullable enable

using System.Globalization;
using System.Runtime.CompilerServices;
using System.Text;
Expand Down Expand Up @@ -46,7 +48,7 @@ public static int SerializeNull(byte[] buffer, int cursor)
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
public static string SerializeString(string value)
public static string SerializeString(string? value)
{
var buffer = ThreadLocalBuffer.Value;
if (buffer == null)
Expand All @@ -60,7 +62,7 @@ public static string SerializeString(string value)
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
public static int SerializeString(byte[] buffer, int cursor, string value)
public static int SerializeString(byte[] buffer, int cursor, string? value)
{
if (value == null)
{
Expand Down Expand Up @@ -90,7 +92,7 @@ public static int SerializeString(byte[] buffer, int cursor, ReadOnlySpan<char>
#endif

[MethodImpl(MethodImplOptions.AggressiveInlining)]
public static string SerializeArray<T>(T[] array)
public static string SerializeArray<T>(T[]? array)
{
var buffer = ThreadLocalBuffer.Value;
if (buffer == null)
Expand All @@ -104,7 +106,7 @@ public static string SerializeArray<T>(T[] array)
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
public static int SerializeArray<T>(byte[] buffer, int cursor, T[] array)
public static int SerializeArray<T>(byte[] buffer, int cursor, T[]? array)
{
if (array == null)
{
Expand All @@ -128,7 +130,7 @@ public static int SerializeArray<T>(byte[] buffer, int cursor, T[] array)
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
public static string SerializeMap(IEnumerable<KeyValuePair<string, object>> map)
public static string SerializeMap(IEnumerable<KeyValuePair<string, object?>>? map)
{
var buffer = ThreadLocalBuffer.Value;
if (buffer == null)
Expand All @@ -142,7 +144,7 @@ public static string SerializeMap(IEnumerable<KeyValuePair<string, object>> map)
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
public static byte[] SerializeKeyValuePairsListAsBytes(List<KeyValuePair<string, object>> listKVp, out int count)
public static byte[] SerializeKeyValuePairsListAsBytes(List<KeyValuePair<string, object?>>? listKVp, out int count)
{
var buffer = ThreadLocalBuffer.Value;
if (buffer == null)
Expand All @@ -156,7 +158,7 @@ public static byte[] SerializeKeyValuePairsListAsBytes(List<KeyValuePair<string,
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
public static int SerializeMap(byte[] buffer, int cursor, IEnumerable<KeyValuePair<string, object>> map)
public static int SerializeMap(byte[] buffer, int cursor, IEnumerable<KeyValuePair<string, object?>>? map)
{
if (map == null)
{
Expand All @@ -183,7 +185,7 @@ public static int SerializeMap(byte[] buffer, int cursor, IEnumerable<KeyValuePa
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
public static int SerializeKeyValuePairList(byte[] buffer, int cursor, List<KeyValuePair<string, object>> listKvp)
public static int SerializeKeyValuePairList(byte[] buffer, int cursor, List<KeyValuePair<string, object?>>? listKvp)
{
if (listKvp == null)
{
Expand All @@ -210,7 +212,7 @@ public static int SerializeKeyValuePairList(byte[] buffer, int cursor, List<KeyV
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
public static string Serialize(object obj)
public static string Serialize(object? obj)
{
var buffer = ThreadLocalBuffer.Value;
if (buffer == null)
Expand All @@ -224,7 +226,7 @@ public static string Serialize(object obj)
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
public static int Serialize(byte[] buffer, int cursor, object obj)
public static int Serialize(byte[] buffer, int cursor, object? obj)
{
if (obj == null)
{
Expand All @@ -247,7 +249,7 @@ public static int Serialize(byte[] buffer, int cursor, object obj)
case float:
case double:
Span<char> tmp = stackalloc char[MAX_STACK_ALLOC_SIZE_IN_BYTES / sizeof(char)];
(obj as ISpanFormattable).TryFormat(tmp, out int charsWritten, default, CultureInfo.InvariantCulture);
((ISpanFormattable)obj).TryFormat(tmp, out int charsWritten, default, CultureInfo.InvariantCulture);
return WriteString(buffer, cursor, tmp.Slice(0, charsWritten));
case DateTime dt:
tmp = stackalloc char[MAX_STACK_ALLOC_SIZE_IN_BYTES / sizeof(char)];
Expand Down Expand Up @@ -297,7 +299,7 @@ public static int Serialize(byte[] buffer, int cursor, object obj)
return SerializeArray(buffer, cursor, vdtarray);
case string v:
return SerializeString(buffer, cursor, v);
case IEnumerable<KeyValuePair<string, object>> v:
case IEnumerable<KeyValuePair<string, object?>> v:
return SerializeMap(buffer, cursor, v);
case object[] v:
return SerializeArray(buffer, cursor, v);
Expand All @@ -314,7 +316,7 @@ public static int Serialize(byte[] buffer, int cursor, object obj)
#endif

default:
string repr;
string? repr;
try
{
repr = Convert.ToString(obj, CultureInfo.InvariantCulture);
Expand Down
9 changes: 7 additions & 2 deletions src/OpenTelemetry.Exporter.Geneva/TLDExporter/TldExporter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
// Copyright The OpenTelemetry Authors
// SPDX-License-Identifier: Apache-2.0

#nullable enable

using System.Diagnostics;
using System.Globalization;
using System.Runtime.CompilerServices;
using System.Text;
Expand Down Expand Up @@ -35,6 +38,8 @@ internal abstract class TldExporter
[MethodImpl(MethodImplOptions.AggressiveInlining)]
protected static void Serialize(EventBuilder eb, string key, object value)
{
Debug.Assert(value != null, "value was null");

switch (value)
{
case bool vb:
Expand Down Expand Up @@ -124,11 +129,11 @@ protected static void Serialize(EventBuilder eb, string key, object value)
string repr;
try
{
repr = Convert.ToString(value, CultureInfo.InvariantCulture);
repr = Convert.ToString(value, CultureInfo.InvariantCulture) ?? string.Empty;
}
catch
{
repr = $"ERROR: type {value.GetType().FullName} is not supported";
repr = $"ERROR: type {value!.GetType().FullName} is not supported";
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this can throw an exception if value = null!

Copy link
Member Author

Choose a reason for hiding this comment

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

We could but we would have to pay for a null check for every field serialized. I went and checked, seems like none of the upstream code actually will ever pass a null (also added the assert to validate) so this at the end of the day is just being defensive. Even with a null check users could do something silly like this...

activity.SetTag("MyTag", new MyType());

class MyType
{
   public override string? ToString() => null;
}

...which would cause the Convert.ToString call there to return null even though value itself was something real! Spooky 👻

Copy link
Contributor

Choose a reason for hiding this comment

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

okay, if you checked the upstream code then I'm fine continuing as is. :)

}

eb.AddCountedAnsiString(key, repr, Encoding.UTF8, 0, Math.Min(repr.Length, StringLengthLimit));
Expand Down
Loading
Loading