Skip to content
This repository has been archived by the owner on Jun 30, 2022. It is now read-only.

[DO NOT MERGE] Enterprise Template TelemetryClient/AppInsights conversation analytics capabilities #425

Merged
merged 15 commits into from
Dec 13, 2018

Conversation

ryanisgrig
Copy link
Collaborator

@ryanisgrig ryanisgrig commented Dec 10, 2018

Description

  • Updated ET to work with new 4.2 SDK which will support more App Insights telemetry out-of-the-box.
  • OnboardingDialog is a sample of how the WaterfallDialog telemetry works by passing ITelemetryClient from parent->child dialog.
  • Refactored Middleware properties to all use camelCasing standard.

Related Issue

#134
#354
https://github.com/daveta/analytics/blob/master/va_reports.md

Testing Steps

Build VSIX project and get .vsix from /bin/debug folder. Install the new template and have a working bot.

Checklist

  • I have commented my code, particularly in hard-to-understand areas
  • I have added or updated the appropriate unit tests
  • I have updated related documentation

If this contains changes that needs to be replicated between the Enterprise Template <-> Virtual Assistant

{ TelemetryConstants.ActivityIDProperty, activity.ReplyToId },
{ TelemetryConstants.ReplyActivityIDProperty, activity.Id },
{ TelemetryConstants.ChannelProperty, activity.ChannelId },
{ TelemetryConstants.ChannelIdProperty, activity.ChannelId },
Copy link
Collaborator Author

@ryanisgrig ryanisgrig Dec 10, 2018

Choose a reason for hiding this comment

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

@daveta FYI: I have not seen the channel id populate in SDK telemetry, so I'm providing in the BotReceived/Send events.

Copy link
Contributor

@lzc850612 lzc850612 left a comment

Choose a reason for hiding this comment

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

We should apply the changes here to VA as needed.

ryanisgrig and others added 4 commits December 10, 2018 18:11
* fix for teams auth

* removed dev exception page

* updated teams auth middleware

* fixed
LuisServices.Add(service.Id, new TelemetryLuisRecognizer(luisApp));
break;
InstrumentationKey = appInsights.InstrumentationKey,
};
Copy link
Contributor

Choose a reason for hiding this comment

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

In samples, I was removing this - actually wasn't referencing. So I kept the validation checks, but didn't actually create the TelemetryClient since that is effectively done by the extension methods in startup (and registered in DI) as BotTelemetryClient.

@@ -16,7 +16,6 @@ public static void Main(string[] args)

public static IWebHost BuildWebHost(string[] args) =>
WebHost.CreateDefaultBuilder(args)
.UseApplicationInsights()
Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, what this method does is already being done in other methods subsumed by our overrides so we get the sequencing right.

@daveta
Copy link
Contributor

daveta commented Dec 13, 2018

Nice work Ryan!
FYI: (low priority) For your VSIX build, @cleemullins (and I think @darrenj) cooked up some steps where you don't have to check in the zip file. Check out the bot builder vsix csproj.. They add a PreBuildEvent..

@ryanisgrig ryanisgrig merged commit c633739 into master Dec 13, 2018
@lauren-mills lauren-mills deleted the ryanlengel/235_enterprisetemplate branch December 17, 2018 16:30
lauren-mills pushed a commit that referenced this pull request Apr 18, 2019
…sation analytics capabilities (#425)

* Introduced all 4.2 telemetry changes
Pointing to daily build 78

* Removed .pbit and updated README to point to AI/solutions/analytics directory where this will be hosted now.

* Fixed formatting

* Fixed typo in telemetryclient variable.

* Addressed PR comments

* Updated to version 4.2.0

* fix for teams auth (#410)

* fix for teams auth

* removed dev exception page

* updated teams auth middleware

* fixed

* Update readme.md

* Introduced all 4.2 telemetry changes
Pointing to daily build 78

* Removed .pbit and updated README to point to AI/solutions/analytics directory where this will be hosted now.

* Fixed formatting

* Fixed typo in telemetryclient variable.

* Addressed PR comments

* Updated to version 4.2.0
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants