Skip to content

Commit

Permalink
Add basic instrumentation (#14)
Browse files Browse the repository at this point in the history
* Add operation instrumentation
* Add additional test detail
* Add exception instrumentation to operation invoker

Remove a bunch of comments from the test now that these things are captured in GitHub issue #13

Refactored tests partly to enable sharing between the two sets of instrumentation tests, but also to clarify which bits of shared functionality the non-instrumentation tests are relying on. (It had become a bit hazy.)
  • Loading branch information
idg10 authored and HowardvanRooijen committed Dec 3, 2019
1 parent 8f15a1e commit 5f653f3
Show file tree
Hide file tree
Showing 30 changed files with 967 additions and 130 deletions.
1 change: 1 addition & 0 deletions Solutions/Menes.Abstractions/Menes.Abstractions.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
<PackageReference Include="Corvus.ContentHandling" Version="0.12.0" />
<PackageReference Include="Corvus.Extensions" Version="0.7.0" />
<PackageReference Include="Corvus.Extensions.Newtonsoft.Json" Version="0.9.0" />
<PackageReference Include="Corvus.Monitoring.Instrumentation.Abstractions" Version="0.2.0-preview.7" />
<PackageReference Include="Microsoft.CSharp" Version="4.5.0" />
<PackageReference Include="Microsoft.Extensions.Logging" Version="2.2.0" />
<PackageReference Include="Microsoft.OpenApi.Readers" Version="1.1.4" />
Expand Down
28 changes: 21 additions & 7 deletions Solutions/Menes.Hosting/Menes/Internal/OpenApiOperationInvoker.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ namespace Menes.Internal
using System.Reflection;
using System.Threading.Tasks;
using Corvus.Extensions;
using Corvus.Monitoring.Instrumentation;
using Menes.Auditing;
using Menes.Exceptions;
using Microsoft.Extensions.Logging;
Expand All @@ -29,6 +30,8 @@ public class OpenApiOperationInvoker<TRequest, TResponse> : IOpenApiOperationInv
private readonly IAuditContext auditContext;
private readonly ILogger<OpenApiOperationInvoker<TRequest, TResponse>> logger;
private readonly IOpenApiConfiguration configuration;
private readonly IOperationsInstrumentation<OpenApiOperationInvoker<TRequest, TResponse>> operationsInstrumentation;
private readonly IExceptionsInstrumentation<OpenApiOperationInvoker<TRequest, TResponse>> exceptionsInstrumentation;

/// <summary>
/// Creates an instance of the <see cref="OpenApiOperationInvoker{TRequest, TResponse}"/>.
Expand All @@ -41,6 +44,8 @@ public class OpenApiOperationInvoker<TRequest, TResponse> : IOpenApiOperationInv
/// <param name="configuration">The <see cref="IOpenApiConfiguration"/>.</param>
/// <param name="auditContext">The audit context.</param>
/// <param name="logger">The logger.</param>
/// <param name="operationsInstrumentation">Operations instrumentation.</param>
/// <param name="exceptionsInstrumentation">Exceptions instrumentation.</param>
public OpenApiOperationInvoker(
IOpenApiServiceOperationLocator operationLocator,
IOpenApiParameterBuilder<TRequest> parameterBuilder,
Expand All @@ -49,7 +54,9 @@ public OpenApiOperationInvoker(
IOpenApiResultBuilder<TResponse> resultBuilder,
IOpenApiConfiguration configuration,
IAuditContext auditContext,
ILogger<OpenApiOperationInvoker<TRequest, TResponse>> logger)
ILogger<OpenApiOperationInvoker<TRequest, TResponse>> logger,
IOperationsInstrumentation<OpenApiOperationInvoker<TRequest, TResponse>> operationsInstrumentation,
IExceptionsInstrumentation<OpenApiOperationInvoker<TRequest, TResponse>> exceptionsInstrumentation)
{
this.operationLocator = operationLocator;
this.parameterBuilder = parameterBuilder;
Expand All @@ -58,23 +65,31 @@ public OpenApiOperationInvoker(
this.resultBuilder = resultBuilder;
this.auditContext = auditContext;
this.logger = logger;
this.operationsInstrumentation = operationsInstrumentation;
this.configuration = configuration;
this.exceptionsInstrumentation = exceptionsInstrumentation;
}

/// <inheritdoc/>
public async Task<TResponse> InvokeAsync(string path, string method, TRequest request, OpenApiOperationPathTemplate operationPathTemplate, IOpenApiContext context)
{
if (!this.operationLocator.TryGetOperation(operationPathTemplate.Operation.OperationId, out OpenApiServiceOperation openApiServiceOperation))
string operationId = operationPathTemplate.Operation.OperationId;
if (!this.operationLocator.TryGetOperation(operationId, out OpenApiServiceOperation openApiServiceOperation))
{
throw new OpenApiServiceMismatchException(
$"The service's formal definition includes an operation '{operationPathTemplate.Operation.GetOperationId()}', but the service class does not provide an implementation");
}

string operationName = openApiServiceOperation.GetName();
this.logger.LogInformation(
"Executing operation [{openApiServiceOperation}] for [{path}] [{method}]",
openApiServiceOperation.GetName(),
operationName,
path,
method);
var instrumentationDetail = new AdditionalInstrumentationDetail { Properties = { { "Menes.OperationId", operationId } } };
using IOperationInstance instrumentationOperation = this.operationsInstrumentation.StartOperation(
operationName,
instrumentationDetail);

try
{
Expand All @@ -86,7 +101,7 @@ public async Task<TResponse> InvokeAsync(string path, string method, TRequest re
context.CurrentTenantId = tenantId;
}

await this.CheckAccessPoliciesAsync(context, path, method, operationPathTemplate.Operation.OperationId).ConfigureAwait(false);
await this.CheckAccessPoliciesAsync(context, path, method, operationId).ConfigureAwait(false);
object result = openApiServiceOperation.Execute(context, namedParameters);

if (this.logger.IsEnabled(LogLevel.Debug))
Expand Down Expand Up @@ -146,6 +161,8 @@ public async Task<TResponse> InvokeAsync(string path, string method, TRequest re
}
catch (Exception ex)
{
this.exceptionsInstrumentation.ReportException(ex, instrumentationDetail);

try
{
this.logger.LogError(
Expand Down Expand Up @@ -174,9 +191,6 @@ private async Task CheckAccessPoliciesAsync(
string method,
string operationId)
{
// TODO: why do we create one of these and throw it away?
_ = new AccessCheckOperationDescriptor(path, operationId, method);

AccessControlPolicyResult result = await this.accessChecker.CheckAccessPolicyAsync(context, path, operationId, method).ConfigureAwait(false);

if (result.ResultType == AccessControlPolicyResultType.NotAuthenticated)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,8 @@ public static IServiceCollection AddOpenApiHosting<TRequest, TResponse>(this ISe

services.AddJsonSerializerSettings();

services.AddInstrumentation();

services.AddOpenApiAuditing();
services.AddAuditLogSink<ConsoleAuditLogSink>();

Expand Down
24 changes: 18 additions & 6 deletions Solutions/Menes.Specs/Bindings/MenesContainerBindings.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,11 @@

namespace Marain.Claims.SpecFlow.Bindings
{
using Corvus.Monitoring.Instrumentation;
using Corvus.SpecFlow.Extensions;
using Menes;
using Menes.Specs.Fakes;
using Menes.Specs.Steps.TestClasses;
using Microsoft.Extensions.DependencyInjection;
using TechTalk.SpecFlow;

Expand All @@ -16,19 +19,28 @@ namespace Marain.Claims.SpecFlow.Bindings
public static class MenesContainerBindings
{
/// <summary>
/// Initializes the container before each feature's tests are run.
/// Initializes the container before each scenario's tests are run.
/// </summary>
/// <param name="featureContext">The SpecFlow test context.</param>
[BeforeFeature("@setupContainer", Order = ContainerBeforeFeatureOrder.PopulateServiceCollection)]
public static void InitializeContainer(FeatureContext featureContext)
/// <param name="scenarioContext">The SpecFlow test context.</param>
[BeforeScenario("@perScenarioContainer", Order = ContainerBeforeFeatureOrder.PopulateServiceCollection)]
public static void InitializeContainer(ScenarioContext scenarioContext)
{
ContainerBindings.ConfigureServices(
featureContext,
scenarioContext,
serviceCollection =>
{
serviceCollection.AddLogging();
serviceCollection.AddOpenApiHttpRequestHosting<SimpleOpenApiContext>(null);

var instrumentationProvider = new FakeInstrumentationProvider();
serviceCollection.AddSingleton(instrumentationProvider);
serviceCollection.AddSingleton<IOperationsInstrumentation>(instrumentationProvider);
serviceCollection.AddSingleton<IExceptionsInstrumentation>(instrumentationProvider);
serviceCollection.AddInstrumentation();
serviceCollection.AddInstrumentationSourceTagging();

OperationInvokerTestContext.AddServices(serviceCollection);
});
}
}
}
}
77 changes: 77 additions & 0 deletions Solutions/Menes.Specs/Bindings/TestOperationBindings.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
namespace Menes.Specs.Bindings
{
using System.Collections.Generic;
using System.Reflection;
using System.Threading.Tasks;
using Corvus.SpecFlow.Extensions;
using Menes.Internal;
using Menes.Specs.Fakes;
using Menes.Specs.Steps.TestClasses;
using Microsoft.Extensions.DependencyInjection;
using Moq;
using NUnit.Framework;
using TechTalk.SpecFlow;

/// <summary>
/// Provides tests with the ability to invoke simple test OpenApi operations by specifying the
/// <c>@useZeroArgumentTestOperations</c> tag. Must be used in conjunction with <c>@perScenarioContainer</c>.
/// Also defines a test operation used by some tests.
/// </summary>
/// <remarks>
/// <para>
/// When the <c>@useZeroArgumentTestOperations</c> tag is specified, the scenario binding in this class configures
/// the <see cref="IOpenApiParameterBuilder{TRequest}"/> mock to return an empty dictionary regardless of its
/// inputs. This is necessary to enable the <see cref="OpenApiOperationInvoker{TRequest, TResponse}"/> to be able
/// to invoke the operation. (The default behaviour if this particular mock is unconfigured is that it will return
/// null, causing the attempt to invoke the operation to fail. In normal non-test operation, the real builder works
/// out what to do based on the contents of the OpenApi spec.)
/// </para>
/// </remarks>
[Binding]
public class TestOperationBindings : IOpenApiService
{
private static readonly MethodInfo operationMethodInfo = typeof(TestOperationBindings).GetMethod(nameof(TestOperation), BindingFlags.NonPublic | BindingFlags.Instance);
private readonly ScenarioContext scenarioContext;

public TestOperationBindings(ScenarioContext scenarioContext)
{
this.scenarioContext = scenarioContext;
}

[BeforeScenario("@useZeroArgumentTestOperations", Order = ContainerBeforeFeatureOrder.ServiceProviderAvailable)]
public static void InitializeMocksToEnableInvocation(ScenarioContext scenarioContext)
{
OperationInvokerTestContext invokerContext = ContainerBindings.GetServiceProvider(scenarioContext).GetRequiredService<OperationInvokerTestContext>();

invokerContext.ParameterBuilder
.Setup(pb => pb.BuildParametersAsync(It.IsAny<object>(), It.IsAny<OpenApiOperationPathTemplate>()))
.ReturnsAsync(new Dictionary<string, object>());
}

[Given("the operation locator maps the operation id '(.*)' to an operation named '(.*)'")]
public void GivenTheOperationLocatorMapsTheOperationIdToAnOperationNamed(string operationId, string operationName)
{
Assert.AreEqual(nameof(TestOperation), operationName, "Menes uses the method name as the operation name, so this test can only work if the operation specified in the spec matches the name of the operation method supplied");

var operation = new OpenApiServiceOperation(
this,
operationMethodInfo,
new Mock<IOpenApiConfiguration>().Object);
this.InvokerContext.OperationLocator
.Setup(m => m.TryGetOperation(operationId, out operation))
.Returns(true);
}

protected private OperationInvokerTestContext InvokerContext
=> ContainerBindings.GetServiceProvider(this.scenarioContext).GetRequiredService<OperationInvokerTestContext>();

protected private FakeInstrumentationProvider InstrumentationProvider
=> ContainerBindings.GetServiceProvider(this.scenarioContext).GetRequiredService<FakeInstrumentationProvider>();

private Task TestOperation()
{
this.InvokerContext.ReportedOperationCountWhenOperationBodyInvoked = this.InstrumentationProvider.Operations.Count;
return this.InvokerContext.OperationCompletionSource.GetTask();
}
}
}
42 changes: 42 additions & 0 deletions Solutions/Menes.Specs/Fakes/ExceptionDetail.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
namespace Menes.Specs.Fakes
{
using System;
using Corvus.Monitoring.Instrumentation;

/// <summary>
/// Describes exception instrumentation that was delivered to one of our fakes.
/// </summary>
/// <remarks>
/// <para>
/// This represents a single call to <see cref="IExceptionsInstrumentation.ReportException(Exception, AdditionalInstrumentationDetail)"/>.
/// </para>
/// </remarks>
public class ExceptionDetail
{
public ExceptionDetail(
Exception x,
AdditionalInstrumentationDetail additionalDetail,
OperationDetail operationInProgressAtTime)
{
this.Exception = x;
this.AdditionalDetail = additionalDetail;
this.OperationInProgressAtTime = operationInProgressAtTime;
}

/// <summary>
/// Gets the exception passed to the call to <see cref="IExceptionsInstrumentation.ReportException(Exception, AdditionalInstrumentationDetail)"/>.
/// </summary>
public Exception Exception { get; }

/// <summary>
/// Gets the additional instrumentation detail (if any) passed to the call to
/// <see cref="IExceptionsInstrumentation.ReportException(Exception, AdditionalInstrumentationDetail)"/>.
/// </summary>
public AdditionalInstrumentationDetail AdditionalDetail { get; }

/// <summary>
/// Gets the <see cref="OperationDetail"/> that was in progress at the instant this exception was reported.
/// </summary>
public OperationDetail OperationInProgressAtTime { get; }
}
}
53 changes: 53 additions & 0 deletions Solutions/Menes.Specs/Fakes/FakeInstrumentationProvider.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
// <copyright file="FakeInstrumentationProvider.cs" company="Endjin">
// Copyright (c) Endjin. All rights reserved.
// </copyright>

namespace Menes.Specs.Fakes
{
using System;
using System.Collections.Generic;
using Corvus.Monitoring.Instrumentation;
using NUnit.Framework;

/// <summary>
/// Implements instrumentation interfaces, and reports what they see.
/// </summary>
internal class FakeInstrumentationProvider : IOperationsInstrumentation, IExceptionsInstrumentation
{
private readonly List<OperationDetail> operations = new List<OperationDetail>();
private readonly List<ExceptionDetail> exceptions = new List<ExceptionDetail>();
private readonly Stack<OperationDetail> operationsInProgress = new Stack<OperationDetail>();

/// <summary>
/// Gets a list of the operations supplied to the fake <see cref="IOperationsInstrumentation"/> implementation.
/// </summary>
public IReadOnlyList<OperationDetail> Operations => this.operations;

/// <summary>
/// Gets a list of the exceptions supplied to the fake <see cref="IExceptionsInstrumentation"/> implementation.
/// </summary>
public IReadOnlyList<ExceptionDetail> Exceptions => this.exceptions;

/// <inheritdoc/>
public IOperationInstance StartOperation(string name, AdditionalInstrumentationDetail additionalDetail = null)
{
var result = new OperationDetail(
name,
additionalDetail,
detailJustPopped =>
{
OperationDetail current = this.operationsInProgress.Pop();
Assert.AreSame(detailJustPopped, current);
});
this.operationsInProgress.Push(result);
this.operations.Add(result);
return result;
}

/// <inheritdoc/>
public void ReportException(Exception x, AdditionalInstrumentationDetail additionalDetail = null)
{
this.exceptions.Add(new ExceptionDetail(x, additionalDetail, this.operationsInProgress.Peek()));
}
}
}
Loading

0 comments on commit 5f653f3

Please sign in to comment.