Skip to content

Commit

Permalink
Implement Error exception cause (#1277)
Browse files Browse the repository at this point in the history
This commit implements the cause property on
the exception of an APM error. Cause can capture
chained exceptions, which are the chain of
inner exceptions for .NET.

Avoid the allocation of an array and
copying of stacktrace framew when a stacktrace limit
> 0 is specified.

Closes #1267
  • Loading branch information
russcam authored Apr 29, 2021
1 parent 6da3bf1 commit b9ce335
Show file tree
Hide file tree
Showing 4 changed files with 93 additions and 12 deletions.
25 changes: 25 additions & 0 deletions src/Elastic.Apm/Api/CapturedException.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,18 +9,43 @@

namespace Elastic.Apm.Api
{
/// <summary>
/// Information about the original error
/// </summary>
public class CapturedException
{
/// <summary>
/// A collection of error exceptions representing chained exceptions.
/// The chain starts with the outermost exception, followed by its cause, and so on.
/// </summary>
public List<CapturedException> Cause { get; set; }

/// <summary>
/// Code that is set when the error happened, e.g. database error code.
/// </summary>
[MaxLength]
public string Code { get; set; }

/// <summary>
/// Indicates whether the error was caught in the code or not.
/// </summary>
// TODO: makes this nullable in 2.x
public bool Handled { get; set; }

/// <summary>
/// The originally captured error message.
/// </summary>
public string Message { get; set; }

/// <summary>
/// Stacktrace information of the captured exception.
/// </summary>
[JsonProperty("stacktrace")]
public List<CapturedStackFrame> StackTrace { get; set; }

/// <summary>
/// The type of the exception
/// </summary>
[MaxLength]
public string Type { get; set; }

Expand Down
16 changes: 7 additions & 9 deletions src/Elastic.Apm/Helpers/StacktraceHelper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -38,24 +38,22 @@ internal static List<CapturedStackFrame> GenerateApmStackTrace(StackFrame[] fram
)
{
var stackTraceLimit = configurationReader.StackTraceLimit;

if (stackTraceLimit == 0)
return null;

if (stackTraceLimit > 0)
// new StackTrace(skipFrames: n) skips frames from the top of the stack (currently executing method is top)
// the StackTraceLimit feature takes the top n frames, so unfortunately we currently capture the whole stack trace and just take
// the top `configurationReader.StackTraceLimit` frames. - This could be optimized.
frames = frames.Take(stackTraceLimit).ToArray();

var retVal = new List<CapturedStackFrame>(frames.Length);
// new StackTrace(skipFrames: n) skips frames from the top of the stack (currently executing method is top)
// the StackTraceLimit feature takes the top n frames, so unfortunately we currently capture the whole stack trace and just take
// the top `configurationReader.StackTraceLimit` frames.
var len = stackTraceLimit == -1 ? frames.Length : stackTraceLimit;
var retVal = new List<CapturedStackFrame>(len);

logger.Trace()?.Log("transform stack frames");

try
{
foreach (var frame in frames)
for (var index = 0; index < len; index++)
{
var frame = frames[index];
var className = frame?.GetMethod()
?.DeclaringType?.FullName; //see: https://github.com/elastic/apm-agent-dotnet/pull/240#discussion_r289619196

Expand Down
29 changes: 26 additions & 3 deletions src/Elastic.Apm/Model/ExecutionSegmentCommon.cs
Original file line number Diff line number Diff line change
Expand Up @@ -166,11 +166,34 @@ public static void CaptureException(
executionSegment.Outcome = Outcome.Failure;

var capturedCulprit = string.IsNullOrEmpty(culprit) ? GetCulprit(exception, configurationReader) : culprit;
var debugMessage = $"{nameof(ExecutionSegmentCommon)}.{nameof(CaptureException)}";

var capturedException = new CapturedException { Message = exception.Message, Type = exception.GetType().FullName, Handled = isHandled };
var capturedException = new CapturedException
{
Message = exception.Message,
Type = exception.GetType().FullName,
Handled = isHandled,
StackTrace = StacktraceHelper.GenerateApmStackTrace(exception, logger,
debugMessage, configurationReader, apmServerInfo)
};

var innerException = exception.InnerException;
if (innerException != null)
{
capturedException.Cause = new List<CapturedException>();
while (innerException != null)
{
capturedException.Cause.Add(new CapturedException
{
Message = innerException.Message,
Type = innerException.GetType().FullName,
StackTrace = StacktraceHelper.GenerateApmStackTrace(innerException, logger,
debugMessage, configurationReader, apmServerInfo)
});

capturedException.StackTrace = StacktraceHelper.GenerateApmStackTrace(exception, logger,
$"{nameof(Transaction)}.{nameof(CaptureException)}", configurationReader, apmServerInfo);
innerException = innerException.InnerException;
}
}

payloadSender.QueueError(new Error(capturedException, transaction, parentId ?? executionSegment?.Id, logger, labels)
{
Expand Down
35 changes: 35 additions & 0 deletions test/Elastic.Apm.Tests/ErrorTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
// Elasticsearch B.V licenses this file to you under the Apache 2.0 License.
// See the LICENSE file in the project root for more information

using System;
using System.Collections.Generic;
using System.Diagnostics;
using Elastic.Apm.Api;
Expand Down Expand Up @@ -128,5 +129,39 @@ public void ErrorOnTransactionWithEmptyHeaders()
mockPayloadSender.FirstError.Context.Response.Headers.Should().BeNull();
mockPayloadSender.FirstError.Context.InternalLabels.Value.InnerDictionary.Should().BeEmpty();
}

[Fact]
public void Includes_Cause_When_Exception_Has_InnerException()
{
var mockPayloadSender = new MockPayloadSender();
using var agent = new ApmAgent(new TestAgentComponents(payloadSender: mockPayloadSender));

agent.Tracer.CaptureTransaction("Test", "Test", t =>
{
var exception = new Exception(
"Outer exception",
new Exception("Inner exception", new Exception("Inner inner exception")));

t.CaptureException(exception);
});

mockPayloadSender.WaitForErrors();
var error = mockPayloadSender.FirstError;

error.Should().NotBeNull();
var capturedException = error.Exception;

capturedException.Should().NotBeNull();
capturedException.Message.Should().Be("Outer exception");
capturedException.Cause.Should().NotBeNull().And.HaveCount(2);

var firstCause = capturedException.Cause[0];
firstCause.Message.Should().Be("Inner exception");
firstCause.Type.Should().Be("System.Exception");

var secondCause = capturedException.Cause[1];
secondCause.Message.Should().Be("Inner inner exception");
secondCause.Type.Should().Be("System.Exception");
}
}
}

0 comments on commit b9ce335

Please sign in to comment.