-
-
Notifications
You must be signed in to change notification settings - Fork 219
WIP: Sentry Logs #4158
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
base: feat/logs
Are you sure you want to change the base?
WIP: Sentry Logs #4158
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,155 @@ | ||
using Sentry.Extensibility; | ||
using Sentry.Infrastructure; | ||
using Sentry.Internal.Extensions; | ||
|
||
#pragma warning disable CS1591 // Missing XML comment for publicly visible type or member | ||
|
||
namespace Sentry.Experimental; | ||
|
||
[Experimental(DiagnosticId.ExperimentalSentryLogs)] | ||
public sealed class SentryLog : ISentryJsonSerializable | ||
{ | ||
private Dictionary<string, ValueTypePair>? _attributes; | ||
private int _severityNumber = -1; | ||
|
||
[SetsRequiredMembers] | ||
internal SentryLog(SentrySeverity level, string message, object[]? parameters = null) | ||
{ | ||
Timestamp = DateTimeOffset.UtcNow; | ||
TraceId = SentryId.Empty; | ||
Level = level; | ||
Message = message; | ||
Parameters = parameters; | ||
} | ||
|
||
public required DateTimeOffset Timestamp { get; init; } | ||
|
||
public required SentryId TraceId { get; init; } | ||
|
||
public SentrySeverity Level | ||
{ | ||
get => SentrySeverityExtensions.FromSeverityNumber(_severityNumber); | ||
set => _severityNumber = SentrySeverityExtensions.ToSeverityNumber(value); | ||
} | ||
|
||
public required string Message { get; init; } | ||
|
||
//public Dictionary<string, object>? Attributes { get { return _attributes; } } | ||
|
||
public string? Template { get; init; } | ||
|
||
public object[]? Parameters { get; init; } | ||
|
||
public required int SeverityNumber | ||
{ | ||
get => _severityNumber; | ||
set | ||
{ | ||
// | ||
SentrySeverityExtensions.ThrowIfOutOfRange(value); | ||
_severityNumber = value; | ||
} | ||
} | ||
|
||
public void SetAttribute(string key, string value) | ||
{ | ||
_attributes ??= new Dictionary<string, ValueTypePair>(); | ||
_attributes[key] = new ValueTypePair(value, "string"); | ||
} | ||
|
||
public void SetAttribute(string key, bool value) | ||
{ | ||
_attributes ??= new Dictionary<string, ValueTypePair>(); | ||
_attributes[key] = new ValueTypePair(value, "boolean"); | ||
} | ||
|
||
public void SetAttribute(string key, int value) | ||
{ | ||
_attributes ??= new Dictionary<string, ValueTypePair>(); | ||
_attributes[key] = new ValueTypePair(value, "integer"); | ||
} | ||
|
||
public void SetAttribute(string key, double value) | ||
{ | ||
_attributes ??= new Dictionary<string, ValueTypePair>(); | ||
_attributes[key] = new ValueTypePair(value, "double"); | ||
} | ||
|
||
public void WriteTo(Utf8JsonWriter writer, IDiagnosticLogger? logger) | ||
{ | ||
_attributes = new Dictionary<string, ValueTypePair> | ||
{ | ||
{ "sentry.environment", new ValueTypePair("production", "string")}, | ||
{ "sentry.release", new ValueTypePair("1.0.0", "string")}, | ||
{ "sentry.trace.parent_span_id", new ValueTypePair("b0e6f15b45c36b12", "string")}, | ||
}; | ||
|
||
writer.WriteStartObject(); | ||
writer.WriteStartArray("items"); | ||
writer.WriteStartObject(); | ||
|
||
writer.WriteNumber("timestamp", Timestamp.ToUnixTimeSeconds()); | ||
writer.WriteString("trace_id", TraceId); | ||
writer.WriteString("level", Level.ToLogString()); | ||
writer.WriteString("body", Message); | ||
|
||
writer.WritePropertyName("attributes"); | ||
writer.WriteStartObject(); | ||
|
||
if (Template is not null) | ||
{ | ||
writer.WriteSerializable("sentry.message.template", new ValueTypePair(Template, "string"), null); | ||
} | ||
|
||
if (Parameters is not null) | ||
{ | ||
for (var index = 0; index < Parameters.Length; index++) | ||
{ | ||
var type = "string"; | ||
writer.WriteSerializable($"sentry.message.parameters.{index}", new ValueTypePair(Parameters[index], type), null); | ||
} | ||
} | ||
|
||
if (_attributes is not null) | ||
{ | ||
foreach (var attribute in _attributes) | ||
{ | ||
writer.WriteSerializable(attribute.Key, attribute.Value, null); | ||
} | ||
} | ||
|
||
writer.WriteEndObject(); | ||
|
||
if (SeverityNumber != -1) | ||
{ | ||
writer.WriteNumber("severity_number", SeverityNumber); | ||
} | ||
|
||
writer.WriteEndObject(); | ||
writer.WriteEndArray(); | ||
writer.WriteEndObject(); | ||
} | ||
} | ||
|
||
//TODO: remove? perhaps a simple System.ValueTuple`2 suffices | ||
internal readonly struct ValueTypePair : ISentryJsonSerializable | ||
{ | ||
public ValueTypePair(object value, string type) | ||
{ | ||
Value = value.ToString()!; | ||
Type = type; | ||
} | ||
|
||
public string Value { get; } | ||
public string Type { get; } | ||
|
||
public void WriteTo(Utf8JsonWriter writer, IDiagnosticLogger? logger) | ||
{ | ||
writer.WriteStartObject(); | ||
|
||
writer.WriteString("value", Value); | ||
writer.WriteString("type", Type); | ||
|
||
writer.WriteEndObject(); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,74 @@ | ||
using Sentry.Infrastructure; | ||
|
||
#pragma warning disable CS1591 // Missing XML comment for publicly visible type or member | ||
|
||
namespace Sentry.Experimental; | ||
|
||
//TODO: QUESTION: not sure about the name | ||
// this is a bit different to Sentry.SentryLevel and Sentry.BreadcrumbLevel | ||
[Experimental(DiagnosticId.ExperimentalSentryLogs)] | ||
public enum SentrySeverity : short | ||
{ | ||
Trace, | ||
Debug, | ||
Info, | ||
Warn, | ||
Error, | ||
Fatal, | ||
} | ||
|
||
[Experimental(DiagnosticId.ExperimentalSentryLogs)] | ||
internal static class SentrySeverityExtensions | ||
{ | ||
internal static string ToLogString(this SentrySeverity severity) | ||
{ | ||
return severity switch | ||
{ | ||
SentrySeverity.Trace => "trace", | ||
SentrySeverity.Debug => "debug", | ||
SentrySeverity.Info => "info", | ||
SentrySeverity.Warn => "warn", | ||
SentrySeverity.Error => "error", | ||
SentrySeverity.Fatal => "fatal", | ||
_ => throw new ArgumentOutOfRangeException(nameof(severity), severity, null), | ||
}; | ||
} | ||
|
||
internal static SentrySeverity FromSeverityNumber(int severityNumber) | ||
{ | ||
ThrowIfOutOfRange(severityNumber); | ||
|
||
return severityNumber switch | ||
{ | ||
>= 1 and <= 4 => SentrySeverity.Trace, | ||
>= 5 and <= 8 => SentrySeverity.Debug, | ||
>= 9 and <= 12 => SentrySeverity.Info, | ||
>= 13 and <= 16 => SentrySeverity.Warn, | ||
>= 17 and <= 20 => SentrySeverity.Error, | ||
>= 21 and <= 24 => SentrySeverity.Fatal, | ||
_ => throw new UnreachableException(), | ||
}; | ||
} | ||
|
||
internal static int ToSeverityNumber(SentrySeverity severity) | ||
{ | ||
return severity switch | ||
{ | ||
SentrySeverity.Trace => 1, | ||
SentrySeverity.Debug => 5, | ||
SentrySeverity.Info => 9, | ||
SentrySeverity.Warn => 13, | ||
SentrySeverity.Error => 17, | ||
SentrySeverity.Fatal => 21, | ||
_ => throw new ArgumentOutOfRangeException(nameof(severity), severity, null) | ||
}; | ||
} | ||
|
||
internal static void ThrowIfOutOfRange(int severityNumber) | ||
{ | ||
if (severityNumber is < 1 or > 24) | ||
{ | ||
throw new ArgumentOutOfRangeException(nameof(severityNumber), severityNumber, "SeverityNumber must be between 1 (inclusive) and 24 (inclusive)."); | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,29 @@ | ||
#if !NET8_0_OR_GREATER | ||
// ReSharper disable CheckNamespace | ||
// ReSharper disable ConvertToPrimaryConstructor | ||
namespace System.Diagnostics.CodeAnalysis; | ||
|
||
[AttributeUsage(AttributeTargets.Assembly | | ||
AttributeTargets.Module | | ||
AttributeTargets.Class | | ||
AttributeTargets.Struct | | ||
AttributeTargets.Enum | | ||
AttributeTargets.Constructor | | ||
AttributeTargets.Method | | ||
AttributeTargets.Property | | ||
AttributeTargets.Field | | ||
AttributeTargets.Event | | ||
AttributeTargets.Interface | | ||
AttributeTargets.Delegate, Inherited = false)] | ||
internal sealed class ExperimentalAttribute : Attribute | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @SimonCropp has a shimming library that might have this, no? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It is indeed! Now I got two questions:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Updating Polyfill in #4167. |
||
{ | ||
public ExperimentalAttribute(string diagnosticId) | ||
{ | ||
DiagnosticId = diagnosticId; | ||
} | ||
|
||
public string DiagnosticId { get; } | ||
|
||
public string? UrlFormat { get; set; } | ||
} | ||
#endif |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,22 @@ | ||
#if !NET7_0_OR_GREATER | ||
// ReSharper disable CheckNamespace | ||
namespace System.Diagnostics; | ||
|
||
internal sealed class UnreachableException : Exception | ||
{ | ||
public UnreachableException() | ||
: base("The program executed an instruction that was thought to be unreachable.") | ||
{ | ||
} | ||
|
||
public UnreachableException(string? message) | ||
: base(message ?? "The program executed an instruction that was thought to be unreachable.") | ||
{ | ||
} | ||
|
||
public UnreachableException(string? message, Exception? innerException) | ||
: base(message ?? "The program executed an instruction that was thought to be unreachable.", innerException) | ||
{ | ||
} | ||
} | ||
#endif |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does it need to be different? I wonder if we can use SentryLevel instead?
Because we don't have
Trace
there? :~We could add it as
-1
😅There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, not necessarily.
And adding it as
-1
would not break any existing behavior - but may warrant some documentation - like:sentry-dotnet/src/Sentry.Extensions.Logging/LogLevelExtensions.cs
Line 21 in 8978bc4
But reusing
Sentry.SentryLevel
could be confusing:sentry-dotnet/src/Sentry/SentryLevel.cs
Line 6 in 8978bc4
uses the
EnumMemberAttribute
, with e.g.[EnumMember(Value = "warning")] Warning,
. But the OTELSeverityText
field and our Protocol expects"warn"
instead. Well, we would still have the separateToSentryProtocolLevel
(or similar) extension method that does the right thing, but the re-use may cause confusion.Also, this would have a side effect onto existing usages, via
SentryOptions.DiagnosticLevel
:sentry-dotnet/src/Sentry/Infrastructure/DiagnosticLogger.cs
Line 8 in 8978bc4
Do we want to extend the "Log-Level" for existing internal/diagnostic logging?
On the other hand, adding yet another "Log-Level" enum, that is similar, but not quite the same, is similarly confusing.
And now I am thinking if we could get away with making this new enum type
internal
instead, since we do have separateLog{Level}()
methods, and application code could still change the severity via theint
-basedSeverityNumber
through theBeforeSendLog
-callback. (see Log Severity Number)To, for now, reduce the public surface area, and we could think about which approach to chose when/if the issue arises later.