Skip to content

Commit

Permalink
chore: align StackFrame's properties (#2691)
Browse files Browse the repository at this point in the history
  • Loading branch information
vaind authored Oct 3, 2023
1 parent ede4a04 commit b28cc31
Show file tree
Hide file tree
Showing 21 changed files with 67 additions and 112 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ API Changes:
- ISpanContext was removed. Use ITraceContext instead. ([#2668](https://github.com/getsentry/sentry-dotnet/pull/2668))
- Removed IHasTransactionNameSource. Use ITransactionContext instead. ([#2654](https://github.com/getsentry/sentry-dotnet/pull/2654))
- Adding `Distribution` to `IEventLike` ([#2660](https://github.com/getsentry/sentry-dotnet/pull/2660))
- Removed unused `StackFrame.InstructionOffset`. ([#2691](https://github.com/getsentry/sentry-dotnet/pull/2691))
- Change `StackFrame`'s `ImageAddress`, `InstructionAddress` and `FunctionId` to `long?`. ([#2691](https://github.com/getsentry/sentry-dotnet/pull/2691))
- Enable `CaptureFailedRequests` by default ([2688](https://github.com/getsentry/sentry-dotnet/pull/2688))

## Unreleased
Expand Down
6 changes: 1 addition & 5 deletions src/Sentry.Profiling/SampleProfileBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -178,11 +178,7 @@ private SentryStackFrame CreateStackFrame(CodeAddressIndex codeAddressIndex)
}

// TODO enable this once we implement symbolication (we will need to send debug_meta too), see StackTraceFactory.
// if (_traceLog.CodeAddresses.ILOffset(codeAddressIndex) is { } ilOffset && ilOffset >= 0)
// {
// frame.InstructionOffset = ilOffset;
// }
// else if (_traceLog.CodeAddresses.Address(codeAddressIndex) is { } address)
// if (_traceLog.CodeAddresses.Address(codeAddressIndex) is { } address)
// {
// frame.InstructionAddress = $"0x{address:x}";
// }
Expand Down
5 changes: 2 additions & 3 deletions src/Sentry/Internal/DebugStackTrace.cs
Original file line number Diff line number Diff line change
Expand Up @@ -232,8 +232,7 @@ private SentryStackFrame InternalCreateFrame(StackFrame stackFrame, bool demangl
// See https://docs.microsoft.com/en-us/dotnet/framework/unmanaged-api/metadata/cortokentype-enumeration
if (tokenType == 0x06000000) // CorTokenType.mdtMethodDef
{
var recordId = token & 0x00ffffff;
frame.FunctionId = $"0x{recordId:x}";
frame.FunctionId = token & 0x00ffffff;
}
}
catch (InvalidOperationException)
Expand Down Expand Up @@ -261,7 +260,7 @@ private SentryStackFrame InternalCreateFrame(StackFrame stackFrame, bool demangl
var ilOffset = stackFrame.GetILOffset();
if (ilOffset != StackFrame.OFFSET_UNKNOWN)
{
frame.InstructionAddress = $"0x{ilOffset:x}";
frame.InstructionAddress = ilOffset;
}

var lineNo = stackFrame.GetFileLineNumber();
Expand Down
8 changes: 4 additions & 4 deletions src/Sentry/Internal/Extensions/JsonExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,9 @@ internal static void ResetSerializerOptions()
.AddDefaultConverters();

AltSerializerOptions = new JsonSerializerOptions
{
ReferenceHandler = ReferenceHandler.Preserve
}
{
ReferenceHandler = ReferenceHandler.Preserve
}
.AddDefaultConverters();
}

Expand Down Expand Up @@ -183,7 +183,7 @@ public static void Deconstruct(this JsonProperty jsonProperty, out string name,
return double.Parse(json.ToString()!, CultureInfo.InvariantCulture);
}

public static long? GetAddressAsLong(this JsonElement json)
public static long? GetHexAsLong(this JsonElement json)
{
// If the address is in json as a number, we can just use it.
if (json.ValueKind == JsonValueKind.Number)
Expand Down
44 changes: 12 additions & 32 deletions src/Sentry/SentryStackFrame.cs
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ public sealed class SentryStackFrame : IJsonSerializable
/// Optionally an address of the debug image to reference.
/// If this is set and a known image is defined by debug_meta then symbolication can take place.
/// </summary>
public long ImageAddress { get; set; }
public long? ImageAddress { get; set; }

/// <summary>
/// An optional address that points to a symbol.
Expand All @@ -116,20 +116,9 @@ public sealed class SentryStackFrame : IJsonSerializable

/// <summary>
/// An optional instruction address for symbolication.<br/>
/// This should be a string with a hexadecimal number that includes a <b>0x</b> prefix.<br/>
/// If this is set and a known image is defined in the <see href="https://develop.sentry.dev/sdk/event-payloads/debugmeta/">Debug Meta Interface</see>, then symbolication can take place.<br/>
/// </summary>
public string? InstructionAddress { get; set; }

/// <summary>
/// The instruction offset.
/// </summary>
/// <remarks>
/// The official docs refer to it as 'The difference between instruction address and symbol address in bytes.'
/// In .NET this means the IL Offset within the assembly.
/// </remarks>
[Obsolete("This property is unused and will be removed in the future.")]
public long? InstructionOffset { get; set; }
public long? InstructionAddress { get; set; }

/// <summary>
/// Optionally changes the addressing mode. The default value is the same as
Expand All @@ -141,11 +130,9 @@ public sealed class SentryStackFrame : IJsonSerializable

/// <summary>
/// The optional Function Id.<br/>
/// This is derived from the `MetadataToken`, and should be the record id
/// of a `MethodDef`.<br/>
/// This should be a string with a hexadecimal number that includes a <b>0x</b> prefix.<br/>
/// This is derived from the `MetadataToken`, and should be the record id of a `MethodDef`.
/// </summary>
public string? FunctionId { get; set; }
public long? FunctionId { get; set; }

/// <inheritdoc />
public void WriteTo(Utf8JsonWriter writer, IDiagnosticLogger? logger)
Expand All @@ -166,14 +153,11 @@ public void WriteTo(Utf8JsonWriter writer, IDiagnosticLogger? logger)
writer.WriteBooleanIfNotNull("in_app", InApp);
writer.WriteStringIfNotWhiteSpace("package", Package);
writer.WriteStringIfNotWhiteSpace("platform", Platform);
writer.WriteStringIfNotWhiteSpace("image_addr", ImageAddress.NullIfDefault()?.ToHexString());
writer.WriteStringIfNotWhiteSpace("symbol_addr", SymbolAddress?.ToHexString());
writer.WriteStringIfNotWhiteSpace("instruction_addr", InstructionAddress);
#pragma warning disable 0618
writer.WriteNumberIfNotNull("instruction_offset", InstructionOffset);
#pragma warning restore 0618
writer.WriteStringIfNotWhiteSpace("image_addr", ImageAddress?.NullIfDefault()?.ToHexString());
writer.WriteStringIfNotWhiteSpace("symbol_addr", SymbolAddress?.NullIfDefault()?.ToHexString());
writer.WriteStringIfNotWhiteSpace("instruction_addr", InstructionAddress?.ToHexString());
writer.WriteStringIfNotWhiteSpace("addr_mode", AddressMode);
writer.WriteStringIfNotWhiteSpace("function_id", FunctionId);
writer.WriteStringIfNotWhiteSpace("function_id", FunctionId?.ToHexString());

writer.WriteEndObject();
}
Expand Down Expand Up @@ -240,12 +224,11 @@ public static SentryStackFrame FromJson(JsonElement json)
var inApp = json.GetPropertyOrNull("in_app")?.GetBoolean();
var package = json.GetPropertyOrNull("package")?.GetString();
var platform = json.GetPropertyOrNull("platform")?.GetString();
var imageAddress = json.GetPropertyOrNull("image_addr")?.GetAddressAsLong() ?? 0;
var symbolAddress = json.GetPropertyOrNull("symbol_addr")?.GetAddressAsLong();
var instructionAddress = json.GetPropertyOrNull("instruction_addr")?.GetString();
var instructionOffset = json.GetPropertyOrNull("instruction_offset")?.GetInt64();
var imageAddress = json.GetPropertyOrNull("image_addr")?.GetHexAsLong() ?? 0;
var symbolAddress = json.GetPropertyOrNull("symbol_addr")?.GetHexAsLong();
var instructionAddress = json.GetPropertyOrNull("instruction_addr")?.GetHexAsLong();
var addressMode = json.GetPropertyOrNull("addr_mode")?.GetString();
var functionId = json.GetPropertyOrNull("function_id")?.GetString();
var functionId = json.GetPropertyOrNull("function_id")?.GetHexAsLong();

return new SentryStackFrame
{
Expand All @@ -266,9 +249,6 @@ public static SentryStackFrame FromJson(JsonElement json)
ImageAddress = imageAddress,
SymbolAddress = symbolAddress,
InstructionAddress = instructionAddress,
#pragma warning disable 0618
InstructionOffset = instructionOffset,
#pragma warning restore 0618
AddressMode = addressMode,
FunctionId = functionId,
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,12 +47,11 @@
InApp: false,
Package: Sentry.NLog.Tests, Version=SCRUBBED, Culture=SCRUBBED, PublicKeyToken=SCRUBBED,
Platform: null,
ImageAddress: 0,
ImageAddress: null,
SymbolAddress: null,
InstructionAddress: ____,
InstructionOffset: null,
InstructionAddress: 2,
AddressMode: rel:0,
FunctionId: ____
FunctionId: 1
}
]
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,12 +47,11 @@
InApp: false,
Package: Sentry.NLog.Tests, Version=SCRUBBED, Culture=SCRUBBED, PublicKeyToken=SCRUBBED,
Platform: null,
ImageAddress: 0,
ImageAddress: null,
SymbolAddress: null,
InstructionAddress: ____,
InstructionOffset: null,
InstructionAddress: 2,
AddressMode: rel:0,
FunctionId: ____
FunctionId: 1
}
]
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,12 +47,11 @@
InApp: false,
Package: Sentry.NLog.Tests, Version=SCRUBBED, Culture=SCRUBBED, PublicKeyToken=SCRUBBED,
Platform: null,
ImageAddress: 0,
ImageAddress: null,
SymbolAddress: null,
InstructionAddress: ____,
InstructionOffset: null,
InstructionAddress: 2,
AddressMode: rel:0,
FunctionId: ____
FunctionId: 1
}
]
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,12 +47,11 @@
InApp: false,
Package: Sentry.NLog.Tests, Version=SCRUBBED, Culture=SCRUBBED, PublicKeyToken=SCRUBBED,
Platform: null,
ImageAddress: 0,
ImageAddress: null,
SymbolAddress: null,
InstructionAddress: ____,
InstructionOffset: null,
InstructionAddress: 2,
AddressMode: rel:0,
FunctionId: ____
FunctionId: 1
}
]
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,12 +47,11 @@
InApp: false,
Package: Sentry.NLog.Tests, Version=SCRUBBED, Culture=SCRUBBED, PublicKeyToken=SCRUBBED,
Platform: null,
ImageAddress: 0,
ImageAddress: null,
SymbolAddress: null,
InstructionAddress: ____,
InstructionOffset: null,
InstructionAddress: 2,
AddressMode: rel:0,
FunctionId: ____
FunctionId: 1
}
]
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -207,12 +207,11 @@
InApp: false,
Package: Sentry.Serilog.Tests, Version=SCRUBBED, Culture=SCRUBBED, PublicKeyToken=SCRUBBED,
Platform: null,
ImageAddress: 0,
ImageAddress: null,
SymbolAddress: null,
InstructionAddress: ____,
InstructionOffset: null,
InstructionAddress: 2,
AddressMode: rel:0,
FunctionId: ____
FunctionId: 1
}
]
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -207,12 +207,11 @@
InApp: false,
Package: Sentry.Serilog.Tests, Version=SCRUBBED, Culture=SCRUBBED, PublicKeyToken=SCRUBBED,
Platform: null,
ImageAddress: 0,
ImageAddress: null,
SymbolAddress: null,
InstructionAddress: ____,
InstructionOffset: null,
InstructionAddress: 2,
AddressMode: rel:0,
FunctionId: ____
FunctionId: 1
}
]
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -207,12 +207,11 @@
InApp: false,
Package: Sentry.Serilog.Tests, Version=SCRUBBED, Culture=SCRUBBED, PublicKeyToken=SCRUBBED,
Platform: null,
ImageAddress: 0,
ImageAddress: null,
SymbolAddress: null,
InstructionAddress: ____,
InstructionOffset: null,
InstructionAddress: 2,
AddressMode: rel:0,
FunctionId: ____
FunctionId: 1
}
]
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -207,12 +207,11 @@
InApp: false,
Package: Sentry.Serilog.Tests, Version=SCRUBBED, Culture=SCRUBBED, PublicKeyToken=SCRUBBED,
Platform: null,
ImageAddress: 0,
ImageAddress: null,
SymbolAddress: null,
InstructionAddress: ____,
InstructionOffset: null,
InstructionAddress: 2,
AddressMode: rel:0,
FunctionId: ____
FunctionId: 1
}
]
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -207,12 +207,11 @@
InApp: false,
Package: Sentry.Serilog.Tests, Version=SCRUBBED, Culture=SCRUBBED, PublicKeyToken=SCRUBBED,
Platform: null,
ImageAddress: 0,
ImageAddress: null,
SymbolAddress: null,
InstructionAddress: ____,
InstructionOffset: null,
InstructionAddress: 2,
AddressMode: rel:0,
FunctionId: ____
FunctionId: 1
}
]
},
Expand Down
4 changes: 2 additions & 2 deletions test/Sentry.Testing/VerifyExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -103,8 +103,8 @@ private class StackFrameConverter : WriteOnlyJsonConverter<SentryStackFrame>

public override void Write(VerifyJsonWriter writer, SentryStackFrame obj)
{
obj.FunctionId = ScrubAlphaNum(obj.FunctionId);
obj.InstructionAddress = ScrubAlphaNum(obj.InstructionAddress);
obj.FunctionId = obj.FunctionId is null ? null : 1;
obj.InstructionAddress = obj.InstructionAddress is null ? null : 2;
obj.Package = obj.Package.Replace(PackageRegex, "=SCRUBBED");

if (RuntimeInfo.GetRuntime().IsMono())
Expand Down
8 changes: 3 additions & 5 deletions test/Sentry.Tests/ApiApprovalTests.Run.Core3_1.verified.txt
Original file line number Diff line number Diff line change
Expand Up @@ -809,12 +809,10 @@ namespace Sentry
public string? FileName { get; set; }
public System.Collections.Generic.IList<int> FramesOmitted { get; }
public string? Function { get; set; }
public string? FunctionId { get; set; }
public long ImageAddress { get; set; }
public long? FunctionId { get; set; }
public long? ImageAddress { get; set; }
public bool? InApp { get; set; }
public string? InstructionAddress { get; set; }
[System.Obsolete("This property is unused and will be removed in the future.")]
public long? InstructionOffset { get; set; }
public long? InstructionAddress { get; set; }
public int? LineNumber { get; set; }
public string? Module { get; set; }
public string? Package { get; set; }
Expand Down
8 changes: 3 additions & 5 deletions test/Sentry.Tests/ApiApprovalTests.Run.DotNet6_0.verified.txt
Original file line number Diff line number Diff line change
Expand Up @@ -810,12 +810,10 @@ namespace Sentry
public string? FileName { get; set; }
public System.Collections.Generic.IList<int> FramesOmitted { get; }
public string? Function { get; set; }
public string? FunctionId { get; set; }
public long ImageAddress { get; set; }
public long? FunctionId { get; set; }
public long? ImageAddress { get; set; }
public bool? InApp { get; set; }
public string? InstructionAddress { get; set; }
[System.Obsolete("This property is unused and will be removed in the future.")]
public long? InstructionOffset { get; set; }
public long? InstructionAddress { get; set; }
public int? LineNumber { get; set; }
public string? Module { get; set; }
public string? Package { get; set; }
Expand Down
8 changes: 3 additions & 5 deletions test/Sentry.Tests/ApiApprovalTests.Run.DotNet7_0.verified.txt
Original file line number Diff line number Diff line change
Expand Up @@ -810,12 +810,10 @@ namespace Sentry
public string? FileName { get; set; }
public System.Collections.Generic.IList<int> FramesOmitted { get; }
public string? Function { get; set; }
public string? FunctionId { get; set; }
public long ImageAddress { get; set; }
public long? FunctionId { get; set; }
public long? ImageAddress { get; set; }
public bool? InApp { get; set; }
public string? InstructionAddress { get; set; }
[System.Obsolete("This property is unused and will be removed in the future.")]
public long? InstructionOffset { get; set; }
public long? InstructionAddress { get; set; }
public int? LineNumber { get; set; }
public string? Module { get; set; }
public string? Package { get; set; }
Expand Down
8 changes: 3 additions & 5 deletions test/Sentry.Tests/ApiApprovalTests.Run.Net4_8.verified.txt
Original file line number Diff line number Diff line change
Expand Up @@ -808,12 +808,10 @@ namespace Sentry
public string? FileName { get; set; }
public System.Collections.Generic.IList<int> FramesOmitted { get; }
public string? Function { get; set; }
public string? FunctionId { get; set; }
public long ImageAddress { get; set; }
public long? FunctionId { get; set; }
public long? ImageAddress { get; set; }
public bool? InApp { get; set; }
public string? InstructionAddress { get; set; }
[System.Obsolete("This property is unused and will be removed in the future.")]
public long? InstructionOffset { get; set; }
public long? InstructionAddress { get; set; }
public int? LineNumber { get; set; }
public string? Module { get; set; }
public string? Package { get; set; }
Expand Down
Loading

0 comments on commit b28cc31

Please # to comment.