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

Move 'Succeeded handling...' log message to JustSaying.Dispatch namespace #853

Closed
wants to merge 1 commit into from
Closed

Move 'Succeeded handling...' log message to JustSaying.Dispatch namespace #853

wants to merge 1 commit into from

Conversation

gkinsman
Copy link
Contributor

@gkinsman gkinsman commented Feb 2, 2021

The log message with template "{Status} handling message with Id '{MessageId}' of type {MessageType} in {TimeToHandle}ms." is new in JS7, and we should allow users to opt out of it by putting it in a separate namespace from everything else that happens in JS. In this PR I've moved it to JustSaying.Dispatch, so that it can be ignored in configuration.

This is the only per-message information log event that I can see that's new in JS7, so it's worth making sure we don't hammer our log cluster by accident 😁.

Perhaps this should even be off by default?

…geType} in {TimeToHandle}ms." log message into a JustSaying.Dispatch namespace so it can be ignored separately from other JS messages.
@gkinsman gkinsman requested a review from a team as a code owner February 2, 2021 17:56
@gkinsman gkinsman requested a review from iancooper February 2, 2021 17:56
@codecov
Copy link

codecov bot commented Feb 2, 2021

Codecov Report

Merging #853 (cf04529) into main (52b3c7a) will increase coverage by 0.30%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #853      +/-   ##
==========================================
+ Coverage   83.48%   83.79%   +0.30%     
==========================================
  Files         113      113              
  Lines        2907     2907              
==========================================
+ Hits         2427     2436       +9     
+ Misses        480      471       -9     
Flag Coverage Δ
linux 83.79% <100.00%> (+0.34%) ⬆️
macos 53.81% <100.00%> (+0.42%) ⬆️
windows 53.81% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...ools/MessageHandling/Dispatch/MessageDispatcher.cs 92.80% <100.00%> (+5.59%) ⬆️
...tSaying/AwsTools/MessageHandling/SqsQueueByName.cs 98.78% <0.00%> (+1.21%) ⬆️
...aying/Messaging/Monitoring/NullOpMessageMonitor.cs 80.00% <0.00%> (+10.00%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 52b3c7a...cf04529. Read the comment docs.

@martincostello
Copy link
Member

Perhaps this should even be off by default?

Maybe it could be Debug?

@martincostello martincostello added this to the v7.0.0 milestone Feb 3, 2021
@gkinsman gkinsman closed this Mar 25, 2021
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.

2 participants