feat: add Azure Functions middleware for HTTP correlation ID management#562
Merged
feat: add Azure Functions middleware for HTTP correlation ID management#562
Conversation
samtrion
added
the
state:ready for merge
samtrion
added
the
type:feature
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (41)
WalkthroughReorganized namespaces into component-specific modules, added Azure Functions isolated worker middleware and DI integration with tests, adjusted project and solution files (package/version, internals visibility, root namespace), and updated editor/ignore configurations and test imports. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (7)
.editorconfig (1)
277-277: Fix misleading inline comment in snapshot block.At Line 277, the comment says “migrations” but this section is for
*.{received,verified}.*snapshot-like files. Please rename the comment to avoid confusion.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.editorconfig at line 277, Update the misleading inline comment that currently reads "Disable all style rules for migrations" to accurately describe the block for snapshot-like files (e.g. use "Disable all style rules for snapshot files (e.g. *.{received,verified}.*)") so the comment matches the section handling files matching *.{received,verified}.*; edit the comment in the .editorconfig snapshot block accordingly.src/NetEvolve.Http.Correlation.AspNetCore/HttpCorrelationMiddleware.cs (1)
52-53: Consider null-safety improvement for the accessor resolution.While
ApplicationBuilderExtensions.UseHttpCorrelationvalidates thatIHttpCorrelationAccessoris registered at startup, using the null-forgiving operator (!) onGetServiceis a potential maintenance risk if the validation is ever bypassed.🛡️ Optional: Use GetRequiredService for explicit intent
- var accessor = context.RequestServices.GetService<IHttpCorrelationAccessor>()!; + var accessor = context.RequestServices.GetRequiredService<IHttpCorrelationAccessor>();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/NetEvolve.Http.Correlation.AspNetCore/HttpCorrelationMiddleware.cs` around lines 52 - 53, The current middleware resolves IHttpCorrelationAccessor with context.RequestServices.GetService<IHttpCorrelationAccessor>()! which uses the null-forgiving operator and risks NRE if validation is bypassed; change resolution to use GetRequiredService<IHttpCorrelationAccessor>() from Microsoft.Extensions.DependencyInjection (or perform an explicit null-check and throw a clear InvalidOperationException) so HttpCorrelationMiddleware sets accessor.HeaderName only when a non-null accessor is returned—update the line in HttpCorrelationMiddleware where accessor is obtained to use GetRequiredService or add a guard that references IHttpCorrelationAccessor and provides a clear error message.src/NetEvolve.Http.Correlation.Abstractions/Abstractions/IHttpCorrelationAccessor.cs (1)
10-13: XML documentation is outdated for the updated interface scope.The documentation states "Gets the current Correlation Id from the
HttpContext", but with this PR the interface is now also implemented byFunctionsCorrelationAccessorwhich retrieves the correlation ID fromFunctionContext.InvocationId, notHttpContext.Consider updating the documentation to be more generic:
📝 Suggested documentation update
/// <summary> - /// Gets the current Correlation Id from the <see cref="HttpContext"/>. + /// Gets or sets the current Correlation Id. /// </summary> string CorrelationId { get; internal set; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/NetEvolve.Http.Correlation.Abstractions/Abstractions/IHttpCorrelationAccessor.cs` around lines 10 - 13, The XML doc for the IHttpCorrelationAccessor.CorrelationId property is too specific to HttpContext; update the summary to a generic description since implementations include FunctionsCorrelationAccessor which uses FunctionContext.InvocationId (not HttpContext). Locate the CorrelationId property on the IHttpCorrelationAccessor interface and change the XML summary to something like "Gets or sets the current correlation id for the current execution context." and ensure it does not reference HttpContext so it applies to both HTTP and function-based implementations.src/NetEvolve.Http.Correlation.Azure.Functions/FunctionsCorrelationAccessor.cs (1)
18-18: Inconsistent default value forHeaderName.
HttpCorrelationAccessorinitializesHeaderNametoCorrelationConstants.HeaderName1, butFunctionsCorrelationAccessorusesstring.Empty. This inconsistency could lead to unexpected behavior ifHeaderNameis accessed before the middleware sets it.♻️ Suggested fix for consistency
- public string HeaderName { get; set; } = string.Empty; + public string HeaderName { get; set; } = CorrelationConstants.HeaderName1;This requires adding a using directive:
using Microsoft.Azure.Functions.Worker; using NetEvolve.Http.Correlation.Abstractions; +using static NetEvolve.Http.Correlation.Abstractions.CorrelationConstants;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/NetEvolve.Http.Correlation.Azure.Functions/FunctionsCorrelationAccessor.cs` at line 18, FunctionsCorrelationAccessor currently defaults the HeaderName property to string.Empty which is inconsistent with HttpCorrelationAccessor; change the default to CorrelationConstants.HeaderName1 on the HeaderName auto-property in FunctionsCorrelationAccessor and add the appropriate using for the namespace that defines CorrelationConstants so the symbol resolves (ensure no other code relies on the empty-string sentinel).src/NetEvolve.Http.Correlation.Azure.Functions/FunctionsWorkerApplicationBuilderExtensions.cs (1)
23-31: Consider documenting theBuildServiceProvidertrade-off.Calling
BuildServiceProvider()during configuration is generally discouraged as it creates an intermediate container. However, this pattern is acceptable here for fail-fast validation since the scope is immediately disposed and no singleton state is shared with the runtime container.If this triggers analyzer warnings (e.g.,
ASP0000), consider suppressing with a comment explaining the validation intent.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/NetEvolve.Http.Correlation.Azure.Functions/FunctionsWorkerApplicationBuilderExtensions.cs` around lines 23 - 31, Add an explanatory comment around the temporary BuildServiceProvider()/CreateScope() usage in FunctionsWorkerApplicationBuilderExtensions to document the trade-off: this intermediate container is intentionally created only for fail-fast validation of IHttpCorrelationAccessor and is immediately disposed so it won’t share singleton state with the runtime container; if your analyzer emits warnings (e.g., ASP0000) add a suppression comment referencing the validation intent and point to ServiceCollectionExtensions.AddHttpCorrelation as the expected registration path.tests/NetEvolve.Http.Correlation.Azure.Functions.Tests.Integration/TestBase.cs (1)
74-79: Expose response metadata from the test harness to cover header propagation.At Line 74 and Line 78,
RunAsync(...)/TestRunResultonly return accessor state. That prevents direct integration assertions for the middleware’s response-header write behavior afternext. Consider returning response headers (or response object) inTestRunResultso tests can validate end-to-end correlation header propagation.Also applies to: 127-127
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/NetEvolve.Http.Correlation.Azure.Functions.Tests.Integration/TestBase.cs` around lines 74 - 79, Return response metadata from RunAsync so tests can assert header propagation: after calling middleware.Invoke(context, next) capture the response headers (or the Response object) from context.Response and include them in the TestRunResult; update the TestRunResult type (and its constructor/consumers) to carry the response headers (e.g., a HeaderDictionary or response object) in addition to nextCalled and FunctionsCorrelationAccessor values, and adjust callers to use the new property for end-to-end header assertions involving FunctionsCorrelationAccessor, RunAsync and middleware.Invoke.tests/NetEvolve.Http.Correlation.Azure.Functions.Tests.Unit/HttpCorrelationBuilderExtensionsTests.cs (1)
40-51: Consider extracting the repeated DI registration assertion into a helper.The predicate/assertion block at Line 40–51 and Line 83–94 is duplicated. A small helper (e.g.,
AssertProviderRegistration(...)) would make future generator additions cheaper to maintain.Refactor sketch
+ private static Task AssertProviderRegistration(IServiceCollection services, string expectedFullName) => + Assert.That(services).Contains(s => + s.ServiceType == typeof(IHttpCorrelationIdProvider) + && s.Lifetime == ServiceLifetime.Singleton + && !string.IsNullOrEmpty(s.ImplementationType!.FullName) + && s.ImplementationType.FullName.Equals(expectedFullName, StringComparison.Ordinal) + ); ... - _ = await Assert - .That(services) - .Contains(s => - s.ServiceType == typeof(IHttpCorrelationIdProvider) - && s.Lifetime == ServiceLifetime.Singleton - && !string.IsNullOrEmpty(s.ImplementationType!.FullName) - && s.ImplementationType.FullName.Equals( - "NetEvolve.Http.Correlation.Generators.GuidCorrelationIdProvider", - StringComparison.Ordinal - ) - ); + await AssertProviderRegistration( + services, + "NetEvolve.Http.Correlation.Generators.GuidCorrelationIdProvider" + ); ... - _ = await Assert - .That(services) - .Contains(s => - s.ServiceType == typeof(IHttpCorrelationIdProvider) - && s.Lifetime == ServiceLifetime.Singleton - && !string.IsNullOrEmpty(s.ImplementationType!.FullName) - && s.ImplementationType.FullName.Equals( - "NetEvolve.Http.Correlation.Generators.GuidV7CorrelationIdProvider", - StringComparison.Ordinal - ) - ); + await AssertProviderRegistration( + services, + "NetEvolve.Http.Correlation.Generators.GuidV7CorrelationIdProvider" + );Also applies to: 83-94
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/NetEvolve.Http.Correlation.Azure.Functions.Tests.Unit/HttpCorrelationBuilderExtensionsTests.cs` around lines 40 - 51, Extract the duplicated DI registration assertion into a small helper to avoid repetition: create a method like AssertProviderRegistration(IServiceCollection services, Type serviceType, string expectedImplementationFullName) (or a specialized AssertProviderRegistrationForCorrelation(IServiceCollection services, string expectedImplementationFullName)) that encapsulates the predicate currently passed to Assert.That(...).Contains(...) and the subsequent Assert.That(services.Count).IsEqualTo(1); then replace both duplicated blocks in HttpCorrelationBuilderExtensionsTests.cs with calls to this helper using serviceType = typeof(IHttpCorrelationIdProvider) and the implementation full name "NetEvolve.Http.Correlation.Generators.GuidCorrelationIdProvider".
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/NetEvolve.Http.Correlation.Azure.Functions/FunctionsCorrelationAccessor.cs`:
- Line 10: The CorrelationId getter currently dereferences the FunctionContext
property (Context) which is initialized to default! and can be null; update the
FunctionsCorrelationAccessor to defensively check Context before accessing
Context.InvocationId and throw a clear InvalidOperationException (or
ArgumentNullException) with an explanatory message if Context is not set, or
alternatively make Context nullable (FunctionContext?) and document the
initialization requirement—locate the Context property and the CorrelationId
getter to implement the null check and ensure callers get a meaningful exception
instead of a NullReferenceException.
In `@src/NetEvolve.Http.Correlation.Ulid/HttpCorrelationBuilderExtensions.cs`:
- Line 1: Public extension type HttpCorrelationBuilderExtensions was moved to a
new namespace which breaks binary/compiled consumers; restore compatibility by
adding a compatibility shim in the original namespace that forwards to the new
implementation (either re-declare an extension class/methods in the old
namespace that call the methods on the new
NetEvolve.Http.Correlation.Ulid.HttpCorrelationBuilderExtensions, or add proper
TypeForwardedTo attributes if using assembly-level type forwarding). Ensure the
shim exposes the identical public signatures (same extension method names and
parameter types) so existing compiled callers continue to resolve to
HttpCorrelationBuilderExtensions.
---
Nitpick comments:
In @.editorconfig:
- Line 277: Update the misleading inline comment that currently reads "Disable
all style rules for migrations" to accurately describe the block for
snapshot-like files (e.g. use "Disable all style rules for snapshot files (e.g.
*.{received,verified}.*)") so the comment matches the section handling files
matching *.{received,verified}.*; edit the comment in the .editorconfig snapshot
block accordingly.
In
`@src/NetEvolve.Http.Correlation.Abstractions/Abstractions/IHttpCorrelationAccessor.cs`:
- Around line 10-13: The XML doc for the IHttpCorrelationAccessor.CorrelationId
property is too specific to HttpContext; update the summary to a generic
description since implementations include FunctionsCorrelationAccessor which
uses FunctionContext.InvocationId (not HttpContext). Locate the CorrelationId
property on the IHttpCorrelationAccessor interface and change the XML summary to
something like "Gets or sets the current correlation id for the current
execution context." and ensure it does not reference HttpContext so it applies
to both HTTP and function-based implementations.
In `@src/NetEvolve.Http.Correlation.AspNetCore/HttpCorrelationMiddleware.cs`:
- Around line 52-53: The current middleware resolves IHttpCorrelationAccessor
with context.RequestServices.GetService<IHttpCorrelationAccessor>()! which uses
the null-forgiving operator and risks NRE if validation is bypassed; change
resolution to use GetRequiredService<IHttpCorrelationAccessor>() from
Microsoft.Extensions.DependencyInjection (or perform an explicit null-check and
throw a clear InvalidOperationException) so HttpCorrelationMiddleware sets
accessor.HeaderName only when a non-null accessor is returned—update the line in
HttpCorrelationMiddleware where accessor is obtained to use GetRequiredService
or add a guard that references IHttpCorrelationAccessor and provides a clear
error message.
In
`@src/NetEvolve.Http.Correlation.Azure.Functions/FunctionsCorrelationAccessor.cs`:
- Line 18: FunctionsCorrelationAccessor currently defaults the HeaderName
property to string.Empty which is inconsistent with HttpCorrelationAccessor;
change the default to CorrelationConstants.HeaderName1 on the HeaderName
auto-property in FunctionsCorrelationAccessor and add the appropriate using for
the namespace that defines CorrelationConstants so the symbol resolves (ensure
no other code relies on the empty-string sentinel).
In
`@src/NetEvolve.Http.Correlation.Azure.Functions/FunctionsWorkerApplicationBuilderExtensions.cs`:
- Around line 23-31: Add an explanatory comment around the temporary
BuildServiceProvider()/CreateScope() usage in
FunctionsWorkerApplicationBuilderExtensions to document the trade-off: this
intermediate container is intentionally created only for fail-fast validation of
IHttpCorrelationAccessor and is immediately disposed so it won’t share singleton
state with the runtime container; if your analyzer emits warnings (e.g.,
ASP0000) add a suppression comment referencing the validation intent and point
to ServiceCollectionExtensions.AddHttpCorrelation as the expected registration
path.
In
`@tests/NetEvolve.Http.Correlation.Azure.Functions.Tests.Integration/TestBase.cs`:
- Around line 74-79: Return response metadata from RunAsync so tests can assert
header propagation: after calling middleware.Invoke(context, next) capture the
response headers (or the Response object) from context.Response and include them
in the TestRunResult; update the TestRunResult type (and its
constructor/consumers) to carry the response headers (e.g., a HeaderDictionary
or response object) in addition to nextCalled and FunctionsCorrelationAccessor
values, and adjust callers to use the new property for end-to-end header
assertions involving FunctionsCorrelationAccessor, RunAsync and
middleware.Invoke.
In
`@tests/NetEvolve.Http.Correlation.Azure.Functions.Tests.Unit/HttpCorrelationBuilderExtensionsTests.cs`:
- Around line 40-51: Extract the duplicated DI registration assertion into a
small helper to avoid repetition: create a method like
AssertProviderRegistration(IServiceCollection services, Type serviceType, string
expectedImplementationFullName) (or a specialized
AssertProviderRegistrationForCorrelation(IServiceCollection services, string
expectedImplementationFullName)) that encapsulates the predicate currently
passed to Assert.That(...).Contains(...) and the subsequent
Assert.That(services.Count).IsEqualTo(1); then replace both duplicated blocks in
HttpCorrelationBuilderExtensionsTests.cs with calls to this helper using
serviceType = typeof(IHttpCorrelationIdProvider) and the implementation full
name "NetEvolve.Http.Correlation.Generators.GuidCorrelationIdProvider".
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8d0e0098-9fd8-407b-b49c-b19d92e946d5
📒 Files selected for processing (41)
.csharpierignore.editorconfigDirectory.Build.propsDirectory.Packages.propsHttp.Correlation.slnxsrc/NetEvolve.Http.Correlation.Abstractions/Abstractions/IHttpCorrelationAccessor.cssrc/NetEvolve.Http.Correlation.Abstractions/HttpCorrelationBuilder.cssrc/NetEvolve.Http.Correlation.Abstractions/HttpCorrelationBuilderExtensions.cssrc/NetEvolve.Http.Correlation.Abstractions/NetEvolve.Http.Correlation.Abstractions.csprojsrc/NetEvolve.Http.Correlation.AspNetCore/ApplicationBuilderExtensions.cssrc/NetEvolve.Http.Correlation.AspNetCore/HttpCorrelationAccessor.cssrc/NetEvolve.Http.Correlation.AspNetCore/HttpCorrelationMiddleware.cssrc/NetEvolve.Http.Correlation.AspNetCore/NetEvolve.Http.Correlation.AspNetCore.csprojsrc/NetEvolve.Http.Correlation.AspNetCore/ServiceCollectionExtensions.cssrc/NetEvolve.Http.Correlation.Azure.Functions/FunctionsCorrelationAccessor.cssrc/NetEvolve.Http.Correlation.Azure.Functions/FunctionsCorrelationMiddleware.cssrc/NetEvolve.Http.Correlation.Azure.Functions/FunctionsWorkerApplicationBuilderExtensions.cssrc/NetEvolve.Http.Correlation.Azure.Functions/NetEvolve.Http.Correlation.Azure.Functions.csprojsrc/NetEvolve.Http.Correlation.Azure.Functions/README.mdsrc/NetEvolve.Http.Correlation.Azure.Functions/ServiceCollectionExtensions.cssrc/NetEvolve.Http.Correlation.HttpClient/HttpClientBuilderExtensions.cssrc/NetEvolve.Http.Correlation.HttpClient/HttpCorrelationIdHandler.cssrc/NetEvolve.Http.Correlation.TestGenerator/HttpCorrelationBuilderExtensions.cssrc/NetEvolve.Http.Correlation.TestGenerator/TestGeneratorCorrelationIdProvider.cssrc/NetEvolve.Http.Correlation.Ulid/HttpCorrelationBuilderExtensions.cssrc/NetEvolve.Http.Correlation.Ulid/ULIDCorrelationIdProvider.cstests/NetEvolve.Http.Correlation.AspNetCore.Tests.Integration/HttpCorrelationMiddlewareTests.cstests/NetEvolve.Http.Correlation.AspNetCore.Tests.Integration/TestBase.cstests/NetEvolve.Http.Correlation.AspNetCore.Tests.Unit/ApplicationBuilderExtensionsTests.cstests/NetEvolve.Http.Correlation.AspNetCore.Tests.Unit/HttpCorrelationBuilderExtensionsTests.cstests/NetEvolve.Http.Correlation.AspNetCore.Tests.Unit/ServiceCollectionExtensionsTests.cstests/NetEvolve.Http.Correlation.Azure.Functions.Tests.Integration/FunctionsCorrelationMiddlewareTests.cstests/NetEvolve.Http.Correlation.Azure.Functions.Tests.Integration/NetEvolve.Http.Correlation.Azure.Functions.Tests.Integration.csprojtests/NetEvolve.Http.Correlation.Azure.Functions.Tests.Integration/TestBase.cstests/NetEvolve.Http.Correlation.Azure.Functions.Tests.Unit/FunctionsWorkerApplicationBuilderExtensionsTests.cstests/NetEvolve.Http.Correlation.Azure.Functions.Tests.Unit/HttpCorrelationBuilderExtensionsTests.cstests/NetEvolve.Http.Correlation.Azure.Functions.Tests.Unit/NetEvolve.Http.Correlation.Azure.Functions.Tests.Unit.csprojtests/NetEvolve.Http.Correlation.Azure.Functions.Tests.Unit/ServiceCollectionExtensionsTests.cstests/NetEvolve.Http.Correlation.HttpClient.Tests.Unit/HttpClientBuilderExtensionsTests.cstests/NetEvolve.Http.Correlation.TestGenerator.Tests.Unit/HttpCorrelationBuilderExtensionsTests.cstests/NetEvolve.Http.Correlation.Ulid.Tests.Unit/HttpCorrelationBuilderExtensionsTests.cs
💤 Files with no reviewable changes (1)
- Directory.Build.props
src/NetEvolve.Http.Correlation.Azure.Functions/FunctionsCorrelationAccessor.cs
Show resolved
Hide resolved
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #562 +/- ##
===========================================
- Coverage 100.00% 98.22% -1.78%
===========================================
Files 14 18 +4
Lines 114 169 +55
Branches 10 22 +12
===========================================
+ Hits 114 166 +52
- Misses 0 2 +2
- Partials 0 1 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
samtrion
force-pushed
the
feature/azure-functions
branch
from
9f4f906 to
a96edd2
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Documentation
Refactor
Chores
Tests