Skip to content
This repository was archived by the owner on Dec 19, 2018. It is now read-only.

HostingEngine.Start shouldn't get request ID unless it begins a scope #392

Closed
AspNetSmurfLab opened this issue Oct 8, 2015 · 6 comments
Closed
Assignees
Milestone

Comments

@AspNetSmurfLab
Copy link

Generating request ID is expensive so HostingEngine.Start shouldn't be grabbing the request ID from the feature collection unless it's actually going to call BeginScope.

Effectively we need to merge line 99 into line 110.

by @DamianEdwards

@benaadams
Copy link
Contributor

Had a go at this previously ed1fd4a but it made some tests upset; would need to move some of the code out and into the feature, with the lazy eval

@DamianEdwards
Copy link
Member

Yeah we have a changed planned in Logging to make the entire BeginScope call free until somebody actually tries to log so this will likely get changed as part of that.

@benaadams
Copy link
Contributor

Pushing the FastRequestIdentifierFeature up the chain into the HttpContext gets you some of the way there: benaadams/HttpAbstractions@e2ae5ac

@benaadams
Copy link
Contributor

Should be resolved by #394 but needs the upstream commit; may need test fixes.

benaadams added a commit to benaadams/Hosting that referenced this issue Oct 12, 2015
Resolves aspnet#392
Requires aspnet/HttpAbstractions#435

Move HttpRequestIdentifierFeature to HttpAbsractions
benaadams added a commit to benaadams/Hosting that referenced this issue Oct 12, 2015
Resolves aspnet#392
Requires aspnet/HttpAbstractions#435

Move HttpRequestIdentifierFeature to HttpAbsractions
@muratg muratg added the Perf label Oct 14, 2015
@muratg muratg added this to the 1.0.0 backlog milestone Oct 14, 2015
@muratg muratg modified the milestones: 1.0.0-rc1, 1.0.0 backlog Oct 14, 2015
@muratg muratg modified the milestones: 1.0.0-rc2, 1.0.0-rc1 Oct 26, 2015
@muratg
Copy link

muratg commented Oct 26, 2015

Just talked to @lodejard. Moving is issues to RC2.

@muratg muratg assigned pakrym and unassigned lodejard Nov 23, 2015
@pakrym
Copy link
Contributor

pakrym commented Nov 23, 2015

Was solved by @loudej in f37375f

@pakrym pakrym closed this as completed Nov 23, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

6 participants