Skip to content

Add OTel to the InMemoryInbox#3562

Merged
iancooper merged 4 commits into
BrighterCommand:masterfrom
dhickie:inbox_otel
Mar 14, 2025
Merged

Add OTel to the InMemoryInbox#3562
iancooper merged 4 commits into
BrighterCommand:masterfrom
dhickie:inbox_otel

Conversation

@dhickie

@dhickie dhickie commented Mar 13, 2025

Copy link
Copy Markdown
Contributor

This PR adds Open Telemetry tracing to the InMemoryInbox, by reusing the OutboxSpanInfo and OutboxDbOperation class and enum, with appropriate renaming and updates to support the Exists operation. It also tidies up XML comments across all Inbox implementations.

@codescene-delta-analysis codescene-delta-analysis Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gates Failed
Enforce advisory code health rules (4 files with Excess Number of Function Arguments, Complex Method)

Gates Passed
3 Quality Gates Passed

See analysis details in CodeScene

Reason for failure
Enforce advisory code health rules Violations Code Health Impact
InMemoryInbox.cs 1 advisory rule 8.82 → 8.55 Suppress
BrighterSpanExtensions.cs 1 advisory rule 9.47 → 9.45 Suppress
DynamoDbInbox.cs 1 advisory rule no change Suppress
MongoDbInbox.cs 1 advisory rule no change Suppress

Quality Gate Profile: Clean Code Collective
Want more control? Customize Code Health rules or catch issues early with our IDE extension and CLI tool.

Comment on lines +164 to +166
public Task AddAsync<T>(T command, string contextKey, RequestContext? requestContext,
int timeoutInMilliseconds = -1, CancellationToken cancellationToken = default)
where T : class, IRequest

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❌ New issue: Excess Number of Function Arguments
AddAsync has 5 arguments, threshold = 4

Suppress

/// <param name="timeoutInMilliseconds">Ignored as commands are stored in-memory</param>
/// <param name="cancellationToken">Allow the sender to cancel the operation, if the parameter is supplied</param>
/// <returns><see cref="Task{true}"/> if it exists, otherwise <see cref="Task{false}"/>.</returns>
public Task<bool> ExistsAsync<T>(string id, string contextKey, RequestContext? requestContext, int timeoutInMilliseconds = -1,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❌ New issue: Excess Number of Function Arguments
ExistsAsync has 5 arguments, threshold = 4

Suppress

/// <param name="timeoutInMilliseconds">Ignored as commands are stored in-memory</param>
/// <param name="cancellationToken">Allow the sender to cancel the operation, if the parameter is supplied</param>
/// <returns><see cref="Task{T}"/>.</returns>
public Task<T> GetAsync<T>(string id, string contextKey, RequestContext? requestContext, int timeoutInMilliseconds = -1,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❌ New issue: Excess Number of Function Arguments
GetAsync has 5 arguments, threshold = 4

Suppress

/// Provide a string representation of the outbox operation
/// Provide a string representation of the inbox/outbox operation
/// </summary>
public static string ToSpanName(this OutboxDbOperation span) => span switch

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❌ Getting worse: Complex Method
ToSpanName increases in cyclomatic complexity from 9 to 10, threshold = 9

Suppress

/// <param name="timeoutInMilliseconds">Timeout is ignored as DynamoDB handles timeout and retries</param>
/// <param name="cancellationToken">Allow the sender to cancel the operation, if the parameter is supplied</param>
/// <returns><see cref="Task"/>.</returns>
public async Task AddAsync<T>(T command, string contextKey, RequestContext requestContext, int timeoutInMilliseconds = -1, CancellationToken cancellationToken = default) where T : class, IRequest

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❌ New issue: Excess Number of Function Arguments
AddAsync has 5 arguments, threshold = 4

Suppress

/// <param name="timeoutInMilliseconds">Timeout is ignored as DynamoDB handles timeout and retries</param>
/// <param name="cancellationToken">Allow the sender to cancel the operation, if the parameter is supplied</param>
/// <returns><see cref="Task{T}"/>.</returns>
public async Task<T> GetAsync<T>(string id, string contextKey, RequestContext requestContext, int timeoutInMilliseconds = -1, CancellationToken cancellationToken = default) where T : class, IRequest

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❌ New issue: Excess Number of Function Arguments
GetAsync has 5 arguments, threshold = 4

Suppress

Comment on lines +138 to +149
public async Task<bool> ExistsAsync<T>(string id, string contextKey, RequestContext requestContext, int timeoutInMilliseconds = -1, CancellationToken cancellationToken = default) where T : class, IRequest
{
try
{
var command = await GetCommandAsync<T>(id, contextKey, cancellationToken).ConfigureAwait(ContinueOnCapturedContext);
return command != null;
}
catch (RequestNotFoundException<T>)
{
return false;
}
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❌ New issue: Excess Number of Function Arguments
ExistsAsync has 5 arguments, threshold = 4

Suppress

/// <param name="timeoutInMilliseconds">Timeout is ignored as the timeout is handled by the MongoDb SDK</param>
/// <param name="cancellationToken">Allow the sender to cancel the operation, if the parameter is supplied</param>
/// <returns><see cref="Task"/>.</returns>
public async Task AddAsync<T>(T command, string contextKey, RequestContext? requestContext, int timeoutInMilliseconds = -1,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❌ New issue: Excess Number of Function Arguments
AddAsync has 5 arguments, threshold = 4

Suppress

/// <param name="timeoutInMilliseconds">Timeout is ignored as the timeout is handled by the MongoDb SDK</param>
/// <param name="cancellationToken">Allow the sender to cancel the operation, if the parameter is supplied</param>
/// <returns><see cref="Task{T}"/>.</returns>
public async Task<T> GetAsync<T>(string id, string contextKey, RequestContext? requestContext, int timeoutInMilliseconds = -1,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❌ New issue: Excess Number of Function Arguments
GetAsync has 5 arguments, threshold = 4

Suppress

/// <param name="timeoutInMilliseconds">Timeout is ignored as the timeout is handled by the MongoDb SDK</param>
/// <param name="cancellationToken">Allow the sender to cancel the operation, if the parameter is supplied</param>
/// <returns><see cref="Task{true}"/> if it exists, otherwise <see cref="Task{false}"/>.</returns>
public async Task<bool> ExistsAsync<T>(string id, string contextKey, RequestContext? requestContext, int timeoutInMilliseconds = -1,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❌ New issue: Excess Number of Function Arguments
ExistsAsync has 5 arguments, threshold = 4

Suppress

@dhickie dhickie marked this pull request as ready for review March 13, 2025 15:30

@iancooper iancooper left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a lot here, but this looks good.

@iancooper

Copy link
Copy Markdown
Member

We just need to check the Sqlite test is not failing because of this change

@codescene-delta-analysis codescene-delta-analysis Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gates Failed
Enforce advisory code health rules (4 files with Excess Number of Function Arguments, Complex Method)

Gates Passed
3 Quality Gates Passed

See analysis details in CodeScene

Reason for failure
Enforce advisory code health rules Violations Code Health Impact
InMemoryInbox.cs 1 advisory rule 8.82 → 8.55 Suppress
BrighterSpanExtensions.cs 1 advisory rule 9.47 → 9.45 Suppress
DynamoDbInbox.cs 1 advisory rule no change Suppress
MongoDbInbox.cs 1 advisory rule no change Suppress

Quality Gate Profile: Clean Code Collective
Want more control? Customize Code Health rules or catch issues early with our IDE extension and CLI tool.

@dhickie

dhickie commented Mar 14, 2025

Copy link
Copy Markdown
Contributor Author

We just need to check the Sqlite test is not failing because of this change

@iancooper This was actually broken because of the previous Outbox OTel PR - the constructor signature on the relational database outbox had changed so it wasn't setting the name of the outbox table correctly in the test. I've fixed it here.

@iancooper

Copy link
Copy Markdown
Member

Thanks. Will merge

@iancooper iancooper merged commit 6f39d7a into BrighterCommand:master Mar 14, 2025
@dhickie dhickie deleted the inbox_otel branch March 21, 2025 13:48
DevJonny pushed a commit to DevJonny/Brighter that referenced this pull request Feb 28, 2026
* Start of adding OTel to InMemoryInbox

* Update persistent inboxes with updated interface spec, tidy up XML comments & update inbox tests

* Fix broken SQLite test
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants