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

Support for getContextPropertiesAsync #22

Closed
baywet opened this issue Aug 2, 2017 · 5 comments
Closed

Support for getContextPropertiesAsync #22

baywet opened this issue Aug 2, 2017 · 5 comments

Comments

@baywet
Copy link

baywet commented Aug 2, 2017

Hi,
As an improvement, it'd be nice to support getContextPropertiesAsync (adding a new parameter).
This way we'd be able to implement custom tracing that reads in the body of a POST request for example.
The impact should be limited to HttpRequestTrackingMiddleware (TraceRequest, ctor, props, Invoke) as well as AppBuilderExtension (UseApplicationInsights).
Let me know if you need more information.

@marcinbudny
Copy link
Owner

Hi,
This seems reasonable. I've been thinking about changing the way getContextProperties works anyway, so that full OwinContext is passed into it, instead of request and response separately. I hope I'll have some time to look into it over the weekend.

@marcinbudny
Copy link
Owner

The work is in progress, see here. Still need to update docs and changelog though, then push to nuget.

One thing about logging the POST body: be careful. Once you read the stream, you cannot rewind it for the next middleware (like WebAPI). Recently a person told me they substitute the original stream with a MemoryStream containing copy of the data. It seemed to work, but I think it is extremely hacky. This also prevents streaming of requests.

@baywet
Copy link
Author

baywet commented Aug 7, 2017

Thanks for working on that on such a short delay, and thanks for the heads up. We'll only be implementing that for the QA, front end devs are building new components and sending requests to our API that are sometimes causing issues. Logging those would help us understand what's going wrong when something goes wrong.

@marcinbudny
Copy link
Owner

The version 0.5 containing the changes is now released on nuget

@baywet
Copy link
Author

baywet commented Aug 23, 2017

Thanks for the update and sorry for not answering earlier. I took some time off.
You're right: either the AI middleware executes before the WebAPI one and reading the stream messes up things, replacing it is hacky (made iisexpress crash on my dev box) or it executes after and then you basically cannot access the data.
I guess a proper way to do it would be to implement an attribute that executes before each method of the controller and serializes all parameters to the current operation.
Closing this one. Thanks for your help

@baywet baywet closed this as completed Aug 23, 2017
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

No branches or pull requests

2 participants