Skip to content

Commit

Permalink
chore: merge branch 'master' into kafka_fixes
Browse files Browse the repository at this point in the history
  • Loading branch information
iancooper committed Jan 6, 2025
2 parents b6655bf + 31d194d commit c9e8f37
Show file tree
Hide file tree
Showing 113 changed files with 1,217 additions and 491 deletions.
16 changes: 8 additions & 8 deletions CONTRIBUTORS.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,19 +22,18 @@ Brighter contributors (sorted alphabeticaly)
* New ASB Transport

**[Ian Cooper](https://github.com/iancooper)**
* Wrote first version of Command Processor, Policy, RavenDb Outbox, RMQ gateway, RestMS Core, RestMS Server, RestMS Gateway and ServiceActivator, Control Bus
* Wrote first version of Command Processor, Policy, RavenDb Outbox, Redis Messaging Gateway, RMQ gateway, RestMS Core, RestMS Server, RestMS Gateway and ServiceActivator, Control Bus
* Various fixes
* Project Technical Lead
* Project Owner
* Committer
* Redis Messaging Gateway

**[Derek Comartin](https://github.com/dcomartin)**
* MySQL outbox and inbox

**[Jamie Clayton](https://github.com/mit-jamie-clayton)**
* Documentation fixes

**[ACraven](https://github.com/acraven)**
**[A Craven](https://github.com/acraven)**
* Various fixes

**[Bob Gregory](https://github.com/BobFromHuddle)**
Expand All @@ -44,9 +43,6 @@ Brighter contributors (sorted alphabeticaly)
**[Scott Hanselman](https://github.com/shanselman)**
* Various fixes

**[Thijmen Stavenuiter](https://github.com/Thijmen)**
* Various fixes

**[Toby Henderson](https://github.com/holytshirt)**
* Various fixes
* Significant work on RabbitMQ Gateway and Service Activator reliability
Expand Down Expand Up @@ -143,7 +139,8 @@ Brighter contributors (sorted alphabeticaly)
* New ASB Transport

**[Tim Salva](https://github.com/jtsalva)
* AWS fixes
* AWS fixes
* Metrics

**[Tim Seaward](https://github.com/Drawaes)**
* Fix unecessary allocations
Expand All @@ -164,6 +161,9 @@ Brighter contributors (sorted alphabeticaly)
* Command Sourcing Once-Only improvements
* Inbox improvements

**[Thijmen Stavenuiter](https://github.com/Thijmen)**
* Various fixes

**[Yiannis Triantafyllopoulos](https://github.com/yiannistri)**
* Various Fixes

Expand Down
19 changes: 10 additions & 9 deletions Directory.Packages.props
Original file line number Diff line number Diff line change
Expand Up @@ -4,19 +4,19 @@
<CentralPackageTransitivePinningEnabled>false</CentralPackageTransitivePinningEnabled>
</PropertyGroup>
<ItemGroup>
<PackageVersion Include="AWSSDK.DynamoDBv2" Version="3.7.404.11" />
<PackageVersion Include="AWSSDK.DynamoDBv2" Version="3.7.404.13" />
<PackageVersion Include="AWSSDK.Extensions.NETCore.Setup" Version="3.7.301" />
<PackageVersion Include="AWSSDK.S3" Version="3.7.410.12" />
<PackageVersion Include="AWSSDK.SecurityToken" Version="3.7.401.19" />
<PackageVersion Include="AWSSDK.SimpleNotificationService" Version="3.7.400.70" />
<PackageVersion Include="AWSSDK.SQS" Version="3.7.400.70" />
<PackageVersion Include="AWSSDK.S3" Version="3.7.411" />
<PackageVersion Include="AWSSDK.SecurityToken" Version="3.7.401.21" />
<PackageVersion Include="AWSSDK.SimpleNotificationService" Version="3.7.400.72" />
<PackageVersion Include="AWSSDK.SQS" Version="3.7.400.72" />
<PackageVersion Include="Azure.Identity" Version="1.13.1" />
<PackageVersion Include="Azure.Messaging.ServiceBus" Version="7.18.2" />
<PackageVersion Include="Azure.Storage.Blobs" Version="12.23.0" />
<PackageVersion Include="Confluent.Kafka" Version="2.6.1" />
<PackageVersion Include="Confluent.SchemaRegistry" Version="2.6.1" />
<PackageVersion Include="Confluent.SchemaRegistry.Serdes.Json" Version="2.6.1" />
<PackageVersion Include="coverlet.collector" Version="6.0.2">
<PackageVersion Include="coverlet.collector" Version="6.0.3">
<PrivateAssets>all</PrivateAssets>
<IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
</PackageVersion>
Expand Down Expand Up @@ -67,6 +67,7 @@
<PackageVersion Include="OpenTelemetry.Exporter.InMemory" Version="1.10.0" />
<PackageVersion Include="OpenTelemetry.Exporter.Jaeger" Version="1.5.1" />
<PackageVersion Include="OpenTelemetry.Exporter.OpenTelemetryProtocol" Version="1.10.0" />
<PackageVersion Include="OpenTelemetry.Exporter.Prometheus.AspNetCore" Version="1.10.0-beta.1" />
<PackageVersion Include="OpenTelemetry.Exporter.Zipkin" Version="1.9.0" />
<PackageVersion Include="OpenTelemetry.Extensions.Hosting" Version="1.10.0" />
<PackageVersion Include="OpenTelemetry.Instrumentation.AspNetCore" Version="1.10.1" />
Expand All @@ -78,9 +79,9 @@
<PackageVersion Include="Polly.Contrib.WaitAndRetry" Version="1.1.1" />
<PackageVersion Include="Polly.Extensions.Http" Version="3.0.0" />
<PackageVersion Include="RabbitMQ.Client" Version="7.0.0" />
<PackageVersion Include="Serilog" Version="4.1.0" />
<PackageVersion Include="Serilog.Extensions.Hosting" Version="8.0.0" />
<PackageVersion Include="Serilog.Extensions.Logging" Version="8.0.0" />
<PackageVersion Include="Serilog" Version="4.2.0" />
<PackageVersion Include="Serilog.Extensions.Hosting" Version="9.0.0" />
<PackageVersion Include="Serilog.Extensions.Logging" Version="9.0.0" />
<PackageVersion Include="Serilog.Sinks.Console" Version="6.0.0" />
<PackageVersion Include="Serilog.Sinks.TestCorrelator" Version="4.0.0" />
<PackageVersion Include="ServiceStack.Redis.Core" Version="8.5.2" />
Expand Down
2 changes: 2 additions & 0 deletions docker-compose-prometheus.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,5 @@ services:
- ./prometheus.yml:/etc/prometheus/prometheus.yml
ports:
- "9090:9090"
extra_hosts:
- "host.docker.internal:host-gateway"
2 changes: 2 additions & 0 deletions docker-compose.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,8 @@ services:
- ./prometheus.yml:/etc/prometheus/prometheus.yml
ports:
- "9090:9090"
extra_hosts:
- "host.docker.internal:host-gateway"

networks:
kafka:
Expand Down
90 changes: 90 additions & 0 deletions docs/adr/0022-generate-metrics-from-traces.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
# 22. Generate Metrics from Traces

Date: 2024-xx-xx

## Status

Proposed

## Terminology
For consistency OpenTelemetry terms are used over .NET terms:
- Span over Activity
- Attribute over Tag

## Context
[adr/0010-brighter-semantic-conventions](0010-brighter-semantic-conventions.md#command-processor-operations) outlines semantic conventions of interest and how OpenTelemetry traces will be implemented in Brighter. Traces can answer questions to how an individual request performed, it's also useful to see how an operation performs in aggregate. Brighter does not yet provide this out of the box.

## Decision

### Metrics from Traces
OpenTelemetry defines common metrics for several instrumentation domains, the ones applicable to Brighter being messaging and databases.

#### Messaging metrics

| Name | Instrument Type | Unit (UCUM) | Description |
|------|----------------|-------------|-------------|
| `messaging.client.operation.duration` | Histogram | `s` | Duration of messaging operation initiated by a producer or consumer client |
| `messaging.client.sent.messages` | Counter | `{message}` | Number of messages producer attempted to send to the broker |
| `messaging.client.received.messages` | Counter | `{message}` | Number of messages that were delivered to the application |
| `messaging.process.duration` | Histogram | `s` | Duration of processing operation |

#### Database metrics

| Name | Instrument Type | Unit (UCUM) | Description |
|------|----------------|-------------|-------------|
| `db.client.operation.duration` | Histogram | `s` | Duration of database client operations |

Brighter can implement a custom [processor](https://github.com/open-telemetry/opentelemetry-collector/blob/main/processor/README.md) to generate metrics from spans. The spans defined in [adr/0010-brighter-semantic-conventions](0010-brighter-semantic-conventions.md) lists operations that can be mapped to metrics. For example, a publish span will be mapped to both the `messaging.client.sent.messages` and `messaging.client.operation.duration` metrics.

### Enriching Metrics
By design, common metric names are not namespaced by application. Metrics should be enriched with service data to enable filtering metrics by application. OpenTelemetry defines [resource attributes](https://opentelemetry.io/docs/specs/semconv/resource/#service) which can be used to identify the service:

| Attribute | Type | Description |
|-----------|------|-------------|
| `service.name` | string | Logical name of the service |
| `service.instance.id` | string | The string ID of the service instance |
| `service.namespace` | string | A namespace for `service.name` |
| `service.version` | string | The version string of the service API or implementation. The format is not defined by these conventions. |

### Managing Cardinality
Spans can have high cardinality attributes like `paramore.brighter.requestid` which can over strain metric collectors such as Prometheus. [adr/0010-brighter-semantic-conventions](0010-brighter-semantic-conventions.md) also allows for custom attributes meaning a denylist defined by Brighter will not catch all attributes which should be filtered out. An allowlist containing the default low cardinality attributes set by Brighter are enough until there is a specific case where span attributes given by the application need passing to metrics.

### Bucket Boundaries
By default, OpenTelemetry [implements the following buckets](https://opentelemetry.io/docs/specs/semconv/messaging/messaging-metrics/#common-metrics) `[ 0.005, 0.01, 0.025, 0.05, 0.075, 0.1, 0.25, 0.5, 0.75, 1, 2.5, 5, 7.5, 10 ]` which the application can override with [views](https://opentelemetry.io/docs/concepts/signals/metrics/#views).

### Sampling
Traces are often sampled to control costs ranging from application performance to vendor ingestion. Metrics are not exposed to costs in the same way traces are, with the main lever for histograms being bucket boundaries. [Head-based sampling](https://opentelemetry.io/docs/concepts/sampling/#head-sampling) will distort metrics, applications should prefer to apply a [tail-based sampler](https://opentelemetry.io/docs/concepts/sampling/#tail-sampling) anywhere in the [pipeline](https://opentelemetry.io/docs/collector/architecture/#pipelines) after the Brighter metrics processor.

This means sampling will not reduce costs belonging to tracing before export.

### Meters
[Microsoft docs state](https://learn.microsoft.com/en-us/dotnet/core/diagnostics/metrics-instrumentation#best-practices):
> Each library or library subcomponent can (and often should) create its own Meter. Consider creating a new Meter rather than reusing an existing one if you anticipate app developers would appreciate being able to easily enable and disable the groups of metrics separately.
All existing spans created by Brighter are will likely want to be toggled together. Therefore, all of these metrics will be under a single meter `Paramore.Brighter`.

### Instrumentation
`AddBrighterInstrumentation` is used to instrument both metrics and traces. A `BrighterMetricsFromTracesProcessor` is registered by default and is only enabled when the brighter meter is registered.
```
services.AddOpenTelemetry()
...
.WithTracing(builder =>
{
// available now
// modified to register BrighterMetricsFromTracesProcessor by default
builder.AddBrighterInstrumentation()
// proposed equivalent for .SetSampler()
builder.SetTailSampler(new AlwaysOnSampler()) // Alernatively SetTailSampler<AlwaysOnSampler>()
})
.WithMetrics(builder =>
{
// proposed - follow the same pattern
builder.AddBrighterInstrumentation()
})
```

## Consequences
- Follow existing standards and patterns where available.
- Applications register Brighter meters to consume metrics
- Brighter uses sensible defaults that are overridable
19 changes: 19 additions & 0 deletions docs/adr/0023-Scoping-dependencies-inline-with-lifetime-scope.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
# 23. Scoping dependencies inline with lifetime scope

Date: 2025-01-03

## Status

Proposed

## Context

As it stands dependency Lifetime Scopes are wrapped around the `Command Processor` meaning that all options performed by an instance of the Command processor will share the same scope, this becomes problematic when using the `Publish` method as this allows for multiple `Request Handlers` to be subscribed to a single Event, this will mean that all handlers share dependencies in the same scope which is unexpected behavior.

## Decision

When the Handler Factories are configured to not be a singleton Scopes will be created for each Lifetime, and a new lifetime will be given for each registered subscriber.

## Consequences

We will no longer require a `Command processor provider` as this was only created for scoping, and Handler factories will require the lifetime scope to be passed in to all methods so it can use this for managing scopes.
9 changes: 9 additions & 0 deletions prometheus.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,12 @@ scrape_configs:
- job_name: 'otel-collector'
static_configs:
- targets: ['otel-collector:9090']

- job_name: 'brighter-sample-app'
scheme: https
tls_config:
insecure_skip_verify: true
static_configs:
- targets:
- 'host.docker.internal:5001'
metrics_path: /metrics
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
<PackageReference Include="FluentMigrator.Runner"/>
<PackageReference Include="OpenTelemetry.Exporter.OpenTelemetryProtocol"/>
<PackageReference Include="OpenTelemetry.Exporter.Console"/>
<PackageReference Include="OpenTelemetry.Exporter.Prometheus.AspNetCore" />
<PackageReference Include="OpenTelemetry.Extensions.Hosting"/>
<PackageReference Include="OpenTelemetry.Instrumentation.AspNetCore"/>
<PackageReference Include="Paramore.Darker.AspNetCore"/>
Expand Down
10 changes: 8 additions & 2 deletions samples/WebAPI/WebAPI_Dapper/GreetingsWeb/Startup.cs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
using Paramore.Brighter;
using Paramore.Brighter.Extensions.DependencyInjection;
using Paramore.Brighter.Extensions.Diagnostics;
using Paramore.Brighter.Observability;
using Paramore.Darker.AspNetCore;
using Paramore.Darker.Policies;
using Paramore.Darker.QueryLogging;
Expand Down Expand Up @@ -47,6 +48,8 @@ public void Configure(IApplicationBuilder app, IWebHostEnvironment env)
app.UseRouting();

app.UseEndpoints(endpoints => { endpoints.MapControllers(); });

app.UseOpenTelemetryPrometheusScrapingEndpoint();
}


Expand Down Expand Up @@ -153,12 +156,13 @@ private void ConfigureObservability(IServiceCollection services)
serviceName: "GreetingsWeb",
serviceVersion: typeof(Program).Assembly.GetName().Version?.ToString() ?? "unknown",
serviceInstanceId: Environment.MachineName);
}).WithTracing(builder =>
})
.WithTracing(builder =>
{
builder
.AddBrighterInstrumentation()
.AddSource("RabbitMQ.Client.*")
.SetSampler(new AlwaysOnSampler())
.SetTailSampler<AlwaysOnSampler>()
.AddAspNetCoreInstrumentation()
.AddConsoleExporter()
.AddOtlpExporter(options =>
Expand All @@ -169,7 +173,9 @@ private void ConfigureObservability(IServiceCollection services)
.WithMetrics(builder => builder
.AddAspNetCoreInstrumentation()
.AddConsoleExporter()
.AddPrometheusExporter()
.AddOtlpExporter()
.AddBrighterInstrumentation()
);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ static void ConfigureObservability(IServiceCollection services)
builder
.AddBrighterInstrumentation()
.AddSource("RabbitMQ.Client.*")
.SetSampler(new AlwaysOnSampler())
.SetTailSampler<AlwaysOffSampler>()
.AddAspNetCoreInstrumentation()
.AddConsoleExporter()
.AddOtlpExporter(options =>
Expand Down
Loading

0 comments on commit c9e8f37

Please sign in to comment.