Add better OTel information into Brighter

Brighter.sln

@@ -331,6 +331,8 @@ Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Paramore.Brighter.Sqlite.Da
 EndProject
 Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Paramore.Brighter.Locking.Azure", "src\Paramore.Brighter.Locking.Azure\Paramore.Brighter.Locking.Azure.csproj", "{021F3B51-A640-4C0D-9B47-FB4E32DF6715}"
 EndProject
+Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Paramore.Brighter.Extensions.Diagnostics", "src\Paramore.Brighter.Extensions.Diagnostics\Paramore.Brighter.Extensions.Diagnostics.csproj", "{7FD4C263-74DA-4648-AD2A-7EB34FCBE18E}"
+EndProject
 Global
 	GlobalSection(SolutionConfigurationPlatforms) = preSolution
 		Debug|Any CPU = Debug|Any CPU
@@ -1889,6 +1891,18 @@ Global
 		{021F3B51-A640-4C0D-9B47-FB4E32DF6715}.Release|Mixed Platforms.Build.0 = Release|Any CPU
 		{021F3B51-A640-4C0D-9B47-FB4E32DF6715}.Release|x86.ActiveCfg = Release|Any CPU
 		{021F3B51-A640-4C0D-9B47-FB4E32DF6715}.Release|x86.Build.0 = Release|Any CPU
+		{7FD4C263-74DA-4648-AD2A-7EB34FCBE18E}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
+		{7FD4C263-74DA-4648-AD2A-7EB34FCBE18E}.Debug|Any CPU.Build.0 = Debug|Any CPU
+		{7FD4C263-74DA-4648-AD2A-7EB34FCBE18E}.Debug|Mixed Platforms.ActiveCfg = Debug|Any CPU
+		{7FD4C263-74DA-4648-AD2A-7EB34FCBE18E}.Debug|Mixed Platforms.Build.0 = Debug|Any CPU
+		{7FD4C263-74DA-4648-AD2A-7EB34FCBE18E}.Debug|x86.ActiveCfg = Debug|Any CPU
+		{7FD4C263-74DA-4648-AD2A-7EB34FCBE18E}.Debug|x86.Build.0 = Debug|Any CPU
+		{7FD4C263-74DA-4648-AD2A-7EB34FCBE18E}.Release|Any CPU.ActiveCfg = Release|Any CPU
+		{7FD4C263-74DA-4648-AD2A-7EB34FCBE18E}.Release|Any CPU.Build.0 = Release|Any CPU
+		{7FD4C263-74DA-4648-AD2A-7EB34FCBE18E}.Release|Mixed Platforms.ActiveCfg = Release|Any CPU
+		{7FD4C263-74DA-4648-AD2A-7EB34FCBE18E}.Release|Mixed Platforms.Build.0 = Release|Any CPU
+		{7FD4C263-74DA-4648-AD2A-7EB34FCBE18E}.Release|x86.ActiveCfg = Release|Any CPU
+		{7FD4C263-74DA-4648-AD2A-7EB34FCBE18E}.Release|x86.Build.0 = Release|Any CPU
 	EndGlobalSection
 	GlobalSection(SolutionProperties) = preSolution
 		HideSolutionNode = FALSE

Directory.Packages.props

@@ -57,6 +57,7 @@
     </PackageVersion>
     <PackageVersion Include="NUnit3TestAdapter" Version="4.5.0" />
     <PackageVersion Include="OpenTelemetry" Version="1.8.1" />
+    <PackageVersion Include="OpenTelemetry.Api.ProviderBuilderExtensions" Version="1.8.1" />
     <PackageVersion Include="OpenTelemetry.Exporter.Console" Version="1.8.1" />
     <PackageVersion Include="OpenTelemetry.Exporter.InMemory" Version="1.8.1" />
     <PackageVersion Include="OpenTelemetry.Exporter.Jaeger" Version="1.5.1" />

docs/adr/0010-brighter-semantic-conventions.md

@@ -4,7 +4,7 @@ Date: 2024-04-29
 
 ## Status
 
-Proposed
+Accepted
 
 ## Context
 
@@ -96,7 +96,7 @@ The span kind will always be Internal. This is because the command processor is
 | `deposit` | A request is transfomed into a message and stored in an Outbox                      |
 | `clear`     | Requests in the Outbox are dispatched to a messaging broker via a messaging gateway |
 
-Note that we Publish, Deposit and Clear may be batch operations which result in multiple invocations of our pipeline. In a batch we will create a parent span (itself probably a child of another span that triggered it) and add each item within the batch as an activity via an activity link on the parent span. 
+Note that we Publish, Deposit and Clear may be batch operations which result in multiple invocations of our pipeline. In a batch we will create a parent `create` span (itself probably a child of another span that triggered it) and add each item within the batch as an activity via an activity link on the parent span. 
 
 #### Deposit Operation Spans, External Call Spans
 
@@ -106,15 +106,29 @@ The Command Processor span for a Deposit covers the entire transform and mapper
 
 Storing the message in an Outbox should create a span, with low-cardinality (i.e. name of Outbox or Inbox operation) as per the [Otel specification](https://opentelemetry.io/docs/specs/semconv/database/database-spans/).
 
-A transformer is middleware used in the message mapping pipeline that turns a request into a message. A transformer may call externally, for exammple to object storage or a schema registry. These external calls should create a new span that has the Deposit Operation Span as a parent.
+A transformer is middleware used in the message mapping pipeline that turns a request into a message. A transformer may call externally, for example to object storage or a schema registry. These external calls should create a new span that has the Deposit Operation Span as a parent.
 
 In some cases semantic conventions will exist for these external calls. *For example see object storage: See the [OTel documentation for S3]*
 
 #### Clear Operation Spans, Publish and Create Spans
 
+During a Clear we retrieve a message from the Db, and then produce a message. There are existing conventions around producing and clearing.
+
+Because the CommandProcessor performs both these operations, and both involve a span that has it's own semantics, they need to be the child of a CommandProcessor span which spawns them. This span is named
+
+* `clear`
+
+We don't know the `channel` so we cannot provide more detail in the name
+
+When we Clear we always use a batch, so we may well be looping over a number of clear operations, each of which forms part of a batch, so this implies that each `clear` span is the child of another span
+
+* `create`
+
+Both the `create` span and the `clear` span our internal.
+
 During a Clear the Command Processor acts as a Producer. There are existing [Messaging](https://opentelemetry.io/docs/specs/semconv/messaging/messaging-spans/) semantic conventions for a Producer.
 
-We should create a span for producing a message, that is a child of the Command Processor span. The span is named:
+We should create a span for producing a message, that is a child of the Command Processor `clear` span. The span is named:
 
 * `<destination name>` `<operation name>`
 
@@ -123,7 +137,9 @@ where the destination name is the name of the channel and the operation name is
 * Create Message => span name: `<channel> create` span kind: producer
 * Publish Message => span name: `<channel> publish` span kind: producer
 
-Producing a message is a Publish operation, unless the operation is within a Batch in which case the batch is a Publish with each message in the batch a Create span.
+Producing a message is a `publish` operation, unless the operation is within a Batch in which case the batch is a `publish` with each message in the batch a `create` span.
+
+Kind of the span is `producer` for the creator of the message. So a `publish` span is a `producer` span for a single message but a `client` for a batch, with the `create` being the `producer` in that case. 
 
 [Cloud Events](https://opentelemetry.io/docs/specs/semconv/cloudevents/cloudevents-spans/#attributes) offers alternative names the producer and consumer spans:
 
@@ -181,8 +197,8 @@ Other attributes are available in Brighter today:
 
 We may also wish to make the payload available (although it is not part of the Semantic Conventions).
 
-* `messaging.message.body`: what is the message payload?
-* `messaging.message.headers`: what are the message headers?
+* `messaging.messagebody`: what is the message payload?
+* `messaging.messageheaders`: what are the message headers?
 
 We should check Activity.IsAllDataRequested and only add the attributes if it is. Likely options we would need:
 

docs/adr/0011-brighter-support-for-bulk-messaging-operations.md

@@ -0,0 +1,87 @@
+# 11. Support for Bulk Messaging Operations 
+
+Date: 2019-08-01
+
+## Status
+
+Proposed
+
+## Context
+
+To support Transactional Messaging the Command Processor uses an Outbox when producing a message via an external bus. This means that the CommandProcessor has two associated calls:
+
+* A `DepositPost` call to translate the `IRequest` into a `Message` and store it in an `Outbox`
+* A `ClearOutbox` call to retrieve the `Message` from the `Outbox` and dispatch it via a `Producer`
+
+Within these two steps we perform a number of activities.
+
+For `DepositPost` we:
+
+* Lookup the `IAmAProducer` associated with a `Request`
+* Translate an `IRequest` into the `Message`, executing the `IAmAMessageTransform` and `IAmAMessageMapper` pipeline
+* Store the resulting `Message` in an `Outbox`
+
+For `ClearOutbox` we:
+
+* Retrieve the `Message` from the `Outbox`
+* Lookup the `IAmAProducer` associated with a `Request`
+* We then produce the `Message` via the `Producer`
+
+For the sake of efficiency we chose to support passing an array of `IRequest` to `DepositPost`. We want to bulk write this set of messages to the Outbox, over writing each one individually. For this reason we would like to be able to batch up our writes to the `Outbox` in `DepositPost`
+
+We always pass an array of `Message` identifiers to `ClearOutbox`. For efficiency we obtain the set of matching messages by id from the `Outbox`
+
+A question arises from the relationship between the single `DepositPost` and the bulk `DepositPost`.
+
+The bulk version of `DepositPost` requires two modifications to the deposit behavior:
+
+* As it is an `IEnumerable<IRequest>` the collection passed to the bulk `DepositPost` does not bind a generic parameter of the derived type of `IRequest` that we can use to lookup the associated mapper/transform pipeline and producer.
+* As it is a batch, we want to accumulate all the writes to the `Outbox` over making each one individually.
+
+In addition though we would like to re-use the `Deposit Post` functionality as far as possible to avoid divergent code paths reducing modifiability such as when adding OTel or fixing bugs. 
+
+In V9 our solution was as follows:
+
+* Bucket any batch of `IRequest` by their type
+* By batch late bind a `BulkMessageMap` and then run it through the collection
+* Feed the resulting set of messages to a bulk version of the `Outbox` via `AddToOutbox`
+
+The main issue with this approach is that the flow for `DepositPost` differs between bulk and single versions, which makes it more complex to modify. 
+
+## Decision
+
+It is worth noting the following:
+
+* The `BulkMessageMap` is a bulk, iterative operation; it's main purpose is late-binding of the type from an `IEnumerable<IRequest>`
+* Producing a `Message` via a `Producer` is not a bulk operation in Brighter
+
+The only bulk operations we have in the flow are the read and write from the `Outbox`
+
+* All reads from the `Outbox` are a bulk operation in V9, that iterate over the `Message` collection and pass it to the `Producer`
+
+It is possible for us to late bind the bulk `DepositPost` to a single `DepositPost` call using reflection to invoke the method. This would resolve the binding issue. We can tag the method with an attribute to indicate that it may be called via late binding for documentation and to simplify the reflection code.
+
+This leaves us with the main difference being the bulk write to the `Outbox`
+
+We have two alternatives here:
+
+* Always use a bulk `Add` method on the `Outbox`; as with `Clear` simply treat the first parameter as a collection of `IRequest`
+* Provide a mechanism to indicate that the `DepositPost` should be considered as part of a batch.
+
+Whilst the first seems simpler, two complications arise. The first is the need to do late-binding, the second is the different semantics for a bulk operation in OTel (this might also push us to add a single `Clear` operation and call that repeatedly from a batch parent).
+
+So in this case we intend to opt for the second option.
+
+* In a bulk `DepositPost`, where we have a batch, begin a new batch via `StartBatch` that returns a `BatchId`
+* Pass an optional `BatchId` to a late bound call to the single `IRequest` version of `DepositPost` (which provides late binding) 
+* Where we have an optional `BatchId` within the individual `DepositPost` call `AddToBatch` on the `Outbox` instead of `Add`
+* In the batch `DepositPost` call `EndBatch` to write the batch
+
+Both `StartBatch` and `EndBatch` are `Outbox` code. As they will be shared by all `Outbox` types they need to be implemented in an abstract base class.
+
+`EndBatch` may need to group the writes by `RoutingKey` as a batch endpoint might be called for multiple topics.
+
+## Consequences
+
+* We have a single version of `DepositPost` for the pipeline, but we can call it as part of a batch
+* We should consider having a single message id `Clear` that does not create a batch span

docs/adr/0012-provide-in-memory-and-simple-implementations.md

@@ -0,0 +1,56 @@
+# 12. Provide In-Memory and Simple Implementations 
+
+Date: 2019-08-01
+
+## Status
+
+Accepted
+
+## Context
+
+Brighter provides interfaces to:
+
+* Transports such as `IAmAProducer` and `IAmAMessageConsumer` 
+* Outboxes such as `IAmAnOutbox` or Inboxes such as `IAmAnInbox`
+* Factories such as `IAmAHandlerFactory` 
+
+All of these abstractions allow us or end users to extend Brighter.
+
+When writing tests it is useful to have implementations of these that provide a simple, or in-memory, version of the functionality. A typical option is to provide a Test Double, such as a Fake, Stub or Mock version within our test suite, with simple functionality tailored to that test, for example a FakeProducer; we also might provide a Simple version of a factory such that a test does not need to use a full IoC container to run. 
+
+But we have a number of problems from this strategy:
+
+* These types are not exposed to end users, who might wish to use them in their own tests
+* We do not have fully functioning in memory versions of these, with their own tests, so they may alter the behavior of types for whom they are a dependency when under test.
+
+## Decision
+
+We should provide in-memory or simple versions of libraries that intend to use to extend Brighter. These should have tests in `paramore.brighter.inmemorytests` that prove they provide standard functionality. They should be usable by end users for writing their own tests.
+
+An incomplete list of these include:
+
+* `SimpleHandlerFactoryAsync`
+* `SimpleHandlerFactorySync`
+* `SimpleMessageMapperFactory`
+* `SimpleMessagemapperFactoryAsync`
+* `SimpleMessageTransformerFactory`
+* `SimpleMessageTransformerFactoryAsync`
+* `InMemoryOutbox`
+* `InMemoryInbox`
+* `InMemoryProducer`
+* `InMemoryConsumer`
+* `InMemoryArchiveProvider`
+
+As we review tests we should look for where we might want to promote useful test doubles into lightweight implementations that can be used for testing.
+
+Similar examples in dotnet include `FakeTimeProvider` which helps with Time related tests. I would tend to avoid the `Fake` name over In-Memory or Simple as this is our branding for those helpers.
+
+## Consequences
+
+In some cases it may be appropriate to use the in-memory or simple versions as an internal default when no override is provided, as this enables parts of Brighter to rely on their existence, even if the user does not provide them.
+
+In tests always prefer the usage of these in-memory or simple classes over new Test Doubles. Only use a Test Double where you want to simulate error conditions, or would have to adjust the normal behavior of the in-memory or simple version to test a specific case. The purpose of the in-memory or simple versions is to provide a standard, simple, implementation so it would subvert that to force it to behave differently just to accomodate tests.
+
+This means we have removed some common test doubles such as FakeProducer and FakeOutbox for their in-memory or simple versions.
+
+We occassionally have large PRs because a change has to flow through all the transports or outbox/inboxes. With this strategy we can use the in-memory or simple versions in the core tests to 'prove' the approach, with only signature changes needed for others (which may be fake). We can then merge that and proceed to the concrete implementations, one-by-one.

samples/OpenTelemetry/Sweeper/Program.cs

@@ -7,7 +7,8 @@
 using Paramore.Brighter;
 using Paramore.Brighter.Extensions.DependencyInjection;
 using Paramore.Brighter.Extensions.Hosting;
-using Sweeper.Doubles;
+
+const string topic = "Test.Topic";
 
 Console.WriteLine("Hello, World!");
 
@@ -29,13 +30,16 @@
 
 IAmAProducerRegistry producerRegistry = new ProducerRegistry(new Dictionary<string, IAmAMessageProducer>
 {
-    {"default", new FakeMessageProducer{Publication = {  }}}
+    {"default", new InMemoryProducer(new InternalBus(), TimeProvider.System){Publication = { Topic  = new RoutingKey(topic)}}}
 });
 
+var requestContextFactory = new InMemoryRequestContextFactory();
+
 builder.Services.AddBrighter()
     .UseExternalBus((configure) =>
     {
         configure.ProducerRegistry = producerRegistry;
+        configure.RequestContextFactory = requestContextFactory;
     })
     .UseOutboxSweeper(options =>
     {
@@ -51,8 +55,9 @@
 
 outBox.Add(
     new Message(
-        new MessageHeader(Guid.NewGuid().ToString(), "Test.Topic", MessageType.MT_COMMAND, timeStamp:DateTime.UtcNow),
-        new MessageBody("Hello"))
+        new MessageHeader(Guid.NewGuid().ToString(), topic, MessageType.MT_COMMAND, timeStamp:DateTime.UtcNow),
+        new MessageBody("Hello")),
+    requestContextFactory.Create()
     );
 
 app.Run();

src/Paramore.Brighter.Extensions.DependencyInjection/BrighterOptions.cs

@@ -1,5 +1,6 @@
 using Microsoft.Extensions.DependencyInjection;
 using Paramore.Brighter.FeatureSwitch;
+using Paramore.Brighter.Observability;
 using Polly.Registry;
 
 namespace Paramore.Brighter.Extensions.DependencyInjection
@@ -22,6 +23,16 @@ public class BrighterOptions : IBrighterOptions
         /// </summary>
         public ServiceLifetime HandlerLifetime { get; set; } = ServiceLifetime.Transient;
 
+        /// <summary>
+        /// Configures how verbose our instrumentation is
+        /// InstrumentationOptions.None - no instrumentation
+        /// InstrumentationOptions.RequestInformation - just the request id, request type and operation
+        /// InstrumentationOptions.RequestBody - the request body
+        /// InstrumentationOptions.RequestContext - the request context
+        /// InstrumentationOptions.All - all of the above
+        /// </summary>
+        public InstrumentationOptions InstrumentationOptions { get; set; }
+
         /// <summary>
         /// Configures the lifetime of mappers. Defaults to Singleton
         /// </summary>
@@ -62,6 +73,16 @@ public interface IBrighterOptions
         /// Configures the lifetime of the Handlers.
         /// </summary>
         ServiceLifetime HandlerLifetime { get; set; }
+
+        /// <summary>
+        /// What depth of instrumentation do we need
+        /// InstrumentationOptions.None - no instrumentation
+        /// InstrumentationOptions.RequestInformation - just the request id, request type and operation
+        /// InstrumentationOptions.RequestBody - the request body
+        /// InstrumentationOptions.RequestContext - the request context
+        /// InstrumentationOptions.All - all of the above
+        /// </summary> 
+        InstrumentationOptions InstrumentationOptions { get; set; } 
 
         /// <summary>
         /// Configures the lifetime of mappers.