Skip to content

Restore Activity with OnExecuteRequestStep where available#24

Merged
lmolkova merged 5 commits intomasterfrom
lmolkova/useOnExecuteRequestStep
Apr 25, 2018
Merged

Restore Activity with OnExecuteRequestStep where available#24
lmolkova merged 5 commits intomasterfrom
lmolkova/useOnExecuteRequestStep

Conversation

@lmolkova
Copy link
Copy Markdown

@lmolkova lmolkova commented Apr 19, 2018

Fixes microsoft/ApplicationInsights-dotnet-server#797.

Until we can restore Activity without creating a child, we have to notify tracing system about the new Activity.
Otherwise, dependency calls made in the scope of the request becomes grandchildren of the request and tracing system might not be able to properly correlate them.

This PR attempts to restore Activity in ExecuteRequestHandler step (if lost) and notifies about it in End_Request

WriteEvent(7, id, name);
}

[Event(8, Message = "Activity started, Id='{0}'", Level = EventLevel.Informational)]
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

message doesn't match renamed method. started -> restored

@@ -63,7 +63,7 @@ public static Activity RestoreCurrentActivity(Activity root)

childActivity.Start();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

would it be better to start using that reflection trick here? We clearly need to start formulating feature requests for DiagnosticsSource wrt restoring Activities and custom activity management

{
lock (sync)
{
if (!initialized)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

You do not really need protection here. What's the harm initializing it twice?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Init is called twice: Init special and Init regular. They are sequential, but I was also wondering if concurrent requests may cause Init to be called twice.
@Jinhuafei is it guaranteed that Application.Init is never called in parallel?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Even if it's called in parallel you do not do any harm by making reflection call twice

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

You can set onStepMethodInfo in the static constructor.

@lmolkova lmolkova requested a review from Jinhuafei April 24, 2018 01:32
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.

3 participants