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

[Adaptive] Support telemetry client in adaptive dialog #2406

Closed
vishwacsena opened this issue Aug 13, 2019 · 9 comments
Closed

[Adaptive] Support telemetry client in adaptive dialog #2406

vishwacsena opened this issue Aug 13, 2019 · 9 comments
Assignees
Labels
investigate Needs more information in order to proceed P0 Must Fix. Release-blocker R8 Release 8 - March 16th, 2020
Projects
Milestone

Comments

@vishwacsena
Copy link
Contributor

vishwacsena commented Aug 13, 2019

This item tracks work to add support for telemetry client in adaptive dialog

@vishwacsena vishwacsena added the R7 Release 7 - December 10th, 2019 label Oct 20, 2019
@vishwacsena vishwacsena added this to P0 in Adaptive Oct 20, 2019
@vishwacsena vishwacsena changed the title [Adaptive] Support telemetry client in adaptive dialogs, LG [Adaptive] Support telemetry client in adaptive dialog Oct 20, 2019
@vishwacsena
Copy link
Contributor Author

@github-actions
Copy link

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days

@github-actions github-actions bot added the stale The issue hasn't been updated in a long time and will be automatically closed. label Nov 22, 2019
@tomlm tomlm added the investigate Needs more information in order to proceed label Nov 26, 2019
@vishwacsena vishwacsena added R8 Release 8 - March 16th, 2020 and removed R7 Release 7 - December 10th, 2019 labels Nov 27, 2019
@vishwacsena vishwacsena removed this from P0 in Adaptive Nov 27, 2019
@github-actions github-actions bot closed this as completed Dec 3, 2019
@vishwacsena vishwacsena removed the stale The issue hasn't been updated in a long time and will be automatically closed. label Dec 11, 2019
@vishwacsena vishwacsena reopened this Dec 11, 2019
@vishwacsena vishwacsena added the P0 Must Fix. Release-blocker label Dec 18, 2019
@vishwacsena vishwacsena added this to P0 in Adaptive Dec 19, 2019
@github-actions
Copy link

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days

@github-actions github-actions bot added the stale The issue hasn't been updated in a long time and will be automatically closed. label Jan 18, 2020
@vishwacsena vishwacsena removed the stale The issue hasn't been updated in a long time and will be automatically closed. label Jan 18, 2020
@vishwacsena
Copy link
Contributor Author

Chatted with @scheyal and here is what we landed with for scoping -

The bullet list below is the suggested instrumentation field name, properties should be configured to be accessible via customdimensions

  1. All new recognizers should participate and log recognition result anytime the recognizer is run. This should be similar to how the existing luisrecognizer participates in instrumentation. For all recognizers, log the recognizer configuration itself as well as the actual recognition result on recognition.
    • CrossTrainedRecognizerSet
    • MultiLanguageRecognizer
    • RecognizerSet
    • RegexRecognizer
    • ValueRecognizer
  2. For adaptive dialog instrumentation, we should get to parity with waterfall dialogs and instrument the following
    • AdaptiveDialogStart
    • AdaptiveDialogCancel
    • AdaptiveDialogComplete
    • AdaptiveDialogAction
      • Include type of action in properties as kind
      • instance properties are all included in the property bag.
      • log this when an action begins.
      • All inputs are considered actions.
    • AdaptiveDialogTrigger

@tomlm

@cleemullins
Copy link
Contributor

@tomlm, this is a P0 R8 bug. Can you please update the thread?

@garypretty
Copy link
Contributor

Spoke with @vishwacsena and I am happy to pick this up with the other telemetry work. Makes sense to tackle them as a group.

@garypretty garypretty self-assigned this Feb 26, 2020
@tomlm tomlm removed their assignment Feb 27, 2020
@tomlm
Copy link
Contributor

tomlm commented Feb 27, 2020

Cool, a couple of thoughts

  1. TelemetryLoggerMiddleware should add IBotTelemetryClient to context.TurnState.Add() so it is available to any component via Context.TurnState.Get()
  2. We should consider instrumenting DialogContext.BeginDialog() EndDialg() etc. as that would give us telemetry for all dialogs regardless if they add additional telemetry or not.
  3. I don't know if we are going to be able to log all instance properties of a declarative dialog, as they can be complex and also expressions. Thoughts?

@garypretty
Copy link
Contributor

garypretty commented Feb 27, 2020

@tomlm Thanks.

  1. Sounds sensible - all being well it will be the new abstract class as opposed to IBotTelemetryClient, depending on feedback on my open PR for enabling page views (Update TelemetryClient to enable tracking of dialog ('page') views #3440). We will still need to have the explicit properties on the dialogs for it though, as TelemetryLoggerMiddleware is optional.

  2. I think this will make sense for some events. i.e. the new page views event we will have once we figure out the other PR. We probably still need to have tracking code in AdaptiveDialog / WaterfallDialog though as we track specific events for adaptive vs waterfall.

  3. I haven't dug into the adaptive bits much yet - so I will comment on that as I get into it tomorrow, although I suspect your assertion is likely correct.

@garypretty
Copy link
Contributor

Closing as P0 functionality has now been merged.

Adaptive automation moved this from P0 to Complete Mar 5, 2020
@munozemilio munozemilio added this to the R8 milestone Jul 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
investigate Needs more information in order to proceed P0 Must Fix. Release-blocker R8 Release 8 - March 16th, 2020
Projects
No open projects
Adaptive
Complete
Development

No branches or pull requests

6 participants