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

Extending ExecuteActionFilterAsync which is now using GetCorrelationI… #1

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

dandudek
Copy link

…d method from Request to get the CorrelationId.

…d method from Request to get the CorrelationId.
@ivanz
Copy link
Member

ivanz commented Jul 24, 2016

From email:

Hi Ivan,

I found your CorrelationSharp framework very useful and I wanted to add it to one of our concept projects that we are starting in our company.
During working with your framework in our solution I found that in ExecuteActionFilterAsync method in CorrelationIdActionFilter you are checking heders and if the correlationId is null or empty then you creates new guid. In our case the header was always empty as the "X-Correlation-Id" header wasn’t on the list of headers. As this is this is ActionFilter and you have access to HttpActionContext there is one more possibility to get the correlationId and this is by calling GetCorrelationId on the request which is available on the actionContext.
I crated fork from your CorrelatorSharp.WebApi and I added my changes. I’ve send you pull request. Please review and let me know if this change is acceptable for you and if so please also let me know if you might push this changes into NuGet package.

@ivanz
Copy link
Member

ivanz commented Jul 24, 2016

I am not familiar with the GetCorrelationId method myself and the documentation on MSDN is not very good. I looked at its code and it appears that it does a couple of things:

  1. Looks for a “MS_RequestId” key in the request properties
  2. Uses System.Diagnostics to get Trace.CorrelationManager.ActivityId
  3. If neither is set – creates a new Guid and adds it under the “MS_RequestId” key in the request properties.

Point 2. is a problem, because “Trace.CorrelationManager.ActivityId” could potentially end up overshadowing CorrelatorSharp's filters in that case.

Are your systems doing correlation based on the “MS_RequestId” request property? Where does the value come from?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants