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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Application Insights Telemetry - Handle Client Termination #1367

Closed
wants to merge 1 commit into from

Conversation

davidmrdavid
Copy link
Contributor

Currently a 1-line PR 馃槃 , will include more changes if necessary.

This is a follow-up to #1301
In short, this PR handles the case where an activity makes a forceful termination of the orchestrator via client.TerminateAsync. A reproducer may be found here.

Previously, we did not log anything to Application Insights when this occurred, giving the impression that the orchestration was still running. Now we report that the orchestration was terminated.

To discuss:

  • Are we ok with making a distinction between Terminated and Completed in the logs? May make it cumbersome for users to write queries about whether or not their orchestrations are done.

@davidmrdavid
Copy link
Contributor Author

davidmrdavid commented May 28, 2020

appinsightsx
This is how it looks, the FullName will correspond to the activity that called the termination task

@anthonychu
Copy link
Member

I'm okay with making them separate. Thanks!

@anthonychu
Copy link
Member

Updated our demo instance with the code and it's been running for 3 hours. Looks like it's working as I see some Terminated entries.

Is is difficult to get all orchestrator executions to output the dimensions like we discussed, so we don't get some that don't have them?

@davidmrdavid
Copy link
Contributor Author

davidmrdavid commented May 28, 2020

I spent some time today trying to get that working.

Our biggest blocker to get custom properties working is the fact that the Activity Tags can only be updated once per orchestrator invocation; if you recall, this is due to that whole issue with tags being an array and only the first value of that array getting sent to Application Insights.

So, since I can't predict whether or not an orchestrator will sleep at an await Task statement, I can't know for sure if I should tag it as "Running" or "Completed".

This would not be a problem if we make that 1-line fix in azure-functions-host to let us override tags and, if we're just using this for demo'ing purposes, we should be able to expedite that process. I'll look into this tomorrow

@davidmrdavid
Copy link
Contributor Author

Also, I should point out that termination happens asynchronously...so it's possible (and common in my testing) for an orchestrator to run just one more time after being scheduled for termination, before actually terminating. So you may (?) see some request events for an orchestration happen after termination is reported in Application Insights

@anthonychu
Copy link
Member

OK let's leave it the way it is. I'll try to write the queries tomorrow to see if they give us what we need (looks like they should!). Thanks!

@davidmrdavid davidmrdavid deleted the djusto/appInsightsTelemetry branch October 1, 2021 17:44
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.

None yet

2 participants