Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow to configure log scope key #68

Merged
merged 12 commits into from Oct 6, 2023

Conversation

laurynasr
Copy link
Contributor

Allow to configure the log scope key for correlation ID property. This allows using a different key, such as ActivityId instead of hardcoded CorrelationId.

The default is CorrelationId as previously; custom key can be set when composing the application with AddCorrelate:

services.AddCorrelate(o => o.LoggingScopeKey = "ActivityId");

@codecov
Copy link

codecov bot commented Sep 27, 2023

Codecov Report

Merging #68 (2f57541) into main (b6b79d5) will increase coverage by 0.21%.
The diff coverage is 96.42%.

Additional details and impacted files
Files Coverage Δ
...orrelate.AspNetCore/AspNetCore/CorrelateOptions.cs 100.00% <ø> (ø)
...DependencyInjection/ServiceCollectionExtensions.cs 100.00% <100.00%> (ø)
src/Correlate.Core/CorrelationManagerOptions.cs 100.00% <100.00%> (ø)
src/Correlate.Core/Extensions/LoggerExtensions.cs 60.00% <100.00%> (+4.44%) ⬆️
src/Correlate.Core/RootActivity.cs 83.33% <100.00%> (+1.51%) ⬆️
...ependencyInjection/IServiceCollectionExtensions.cs 100.00% <100.00%> (ø)
src/Correlate.Core/CorrelationManager.cs 91.46% <80.00%> (-0.85%) ⬇️

@laurynasr
Copy link
Contributor Author

@skwasjer is there something else I need to do to get this merged? I see that the automated workflows are waiting for something but I don't know if I should do anything about that

@skwasjer
Copy link
Owner

@laurynasr no, I just have not had time to look at your proposal yet, will probably do over the weekend. Thanks for the submission.

src/Correlate.Core/CorrelationManager.cs Outdated Show resolved Hide resolved
src/Correlate.Core/CorrelationManager.cs Outdated Show resolved Hide resolved
Copy link
Owner

@skwasjer skwasjer left a comment

Choose a reason for hiding this comment

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

lgtm. The only thing stopping the merge is some missing coverage on the new AddCorrelate() registration extension.

@skwasjer skwasjer enabled auto-merge (squash) October 6, 2023 03:12
@sonarcloud
Copy link

sonarcloud bot commented Oct 6, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

93.8% 93.8% Coverage
0.0% 0.0% Duplication

@skwasjer
Copy link
Owner

skwasjer commented Oct 6, 2023

Thanks!

@skwasjer skwasjer merged commit 1c0ca25 into skwasjer:main Oct 6, 2023
11 checks passed
@laurynasr laurynasr deleted the custom-log-scope-name branch October 6, 2023 06:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants