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.

Sign up for GitHub

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

Jump to bottom

Initial pass at AppInsights sending custom events #899

Merged
merged 21 commits into from
Jun 21, 2023
Merged

Initial pass at AppInsights sending custom events #899

merged 21 commits into from
Jun 21, 2023

Conversation

IanNorris
Copy link
Contributor

Motivation and Context

Tracking high level user engagement of features can direct further development and enable measurements of active user counts. It is important to note that completions and conversations themselves should not be logged, just data useful in aggregate.

Description

This is an initial pass for review by the team.

Motivation and Context

  1. This change is essential for tracking user engagement and creating an audit trail of user activity.
  2. This change adds a basic framework for tracking key telemetry events
  3. This scenario enables management of those teams deploying instances to understand how their deployment is being used by their team.
  4. This is not related to an open issue.

Description

The overall design is to add a service accessible to controllers and other services that enables telemetry to be fired with minimal context. For this an abstract ITelemetryService interface was added, allowing systems to fire events on this from across the kernel without any dependencies on a specific implementation.

Contribution Checklist

@IanNorrisRare
Initial pass at AppInsights
39b8ab6 
### Motivation and Context

Tracking user behaviour can identify the most active users and engagement.

### Description

An initial attempt at integrating app insights and some functionally useful telemetry events that can act as a base for further work.
@IanNorrisRare
Syle fixes
1203981
@IanNorrisRare
More style fixes.
3d36113
@github-actions github-actions bot added .NET Issue or Pull requests regarding .NET code kernel.core labels May 10, 2023
@IanNorris IanNorris marked this pull request as ready for review May 10, 2023 14:04
@adrianwyatt adrianwyatt self-assigned this May 10, 2023
@adrianwyatt adrianwyatt added the PR: ready for review All feedback addressed, ready for reviews label May 10, 2023
glahaye
glahaye requested changes May 11, 2023
View reviewed changes
Copy link
Contributor

@glahaye glahaye left a comment

Choose a reason for hiding this comment

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

See comments

@IanNorris
Copy link
Contributor Author

Apologies for the delay on this, hoping to get back to fixing the defects mid next week.

@IanNorris
Copy link
Contributor Author

I've added some docs around using this including some example queries to get people started (as this may be the first time many people are experiencing KQL). I'm not sure if this is too verbose or not. Perhaps if the readme gets much longer we might want to pull the docs out into separate files

@IanNorris IanNorris requested a review from glahaye June 2, 2023 17:24
glahaye
glahaye previously approved these changes Jun 5, 2023
View reviewed changes
@adrianwyatt adrianwyatt requested review from alliscode and removed request for adrianwyatt June 16, 2023 18:04
@adrianwyatt adrianwyatt assigned alliscode and unassigned adrianwyatt Jun 16, 2023
@teresaqhoang teresaqhoang self-assigned this Jun 16, 2023
alliscode
alliscode previously approved these changes Jun 19, 2023
View reviewed changes
@IanNorris
Merge pull request #1 from alliscode/alliscode/merge-feature-expanded…
70d1fe3 
…-appinsights

Merging recent changes into proposed Application Insights additions.
@IanNorris IanNorris dismissed stale reviews from alliscode and glahaye via 70d1fe3 June 20, 2023 16:14
@IanNorris IanNorris requested review from a team as code owners June 20, 2023 16:14
@github-actions github-actions bot added kernel Issues or pull requests impacting the core kernel python Pull requests for the Python Semantic Kernel labels Jun 20, 2023
@teresaqhoang teresaqhoang removed their assignment Jun 21, 2023
@github-actions github-actions bot removed python Pull requests for the Python Semantic Kernel kernel Issues or pull requests impacting the core kernel labels Jun 21, 2023
@alliscode alliscode dismissed adrianwyatt’s stale review June 21, 2023 04:01

All requested changes have been made.

@alliscode alliscode merged commit 18c6f46 into microsoft:main Jun 21, 2023
12 checks passed
@IanNorris IanNorris deleted the feature-expanded-appinsights branch June 21, 2023 10:28
shawncal pushed a commit to shawncal/semantic-kernel that referenced this pull request Jul 6, 2023
@IanNorris @IanNorrisRare
@adrianwyatt @glahaye @alliscode
Initial pass at AppInsights sending custom events (microsoft#899)
8dec875 
### Motivation and Context

Tracking high level user engagement of features can direct further
development and enable measurements of active user counts. It is
important to note that completions and conversations themselves should
not be logged, just data useful in aggregate.

### Description

This is an initial pass for review by the team.

### Motivation and Context
<!-- Thank you for your contribution to the semantic-kernel repo!
Please help reviewers and future users, providing the following
information:
  1. Why is this change required?
  2. What problem does it solve?
  3. What scenario does it contribute to?
  4. If it fixes an open issue, please link to the issue here.
-->

1. This change is essential for tracking user engagement and creating an
audit trail of user activity.
2. This change adds a basic framework for tracking key telemetry events
3. This scenario enables management of those teams deploying instances
to understand how their deployment is being used by their team.
4. This is not related to an open issue.

### Description
<!-- Describe your changes, the overall approach, the underlying design.
These notes will help understanding how your code works. Thanks! -->

The overall design is to add a service accessible to controllers and
other services that enables telemetry to be fired with minimal context.
For this an abstract ITelemetryService interface was added, allowing
systems to fire events on this from across the kernel without any
dependencies on a specific implementation.

### Contribution Checklist
<!-- Before submitting this PR, please make sure: -->
- [X] The code builds clean without any errors or warnings
- [X] The PR follows SK Contribution Guidelines
(https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md)
- [X] The code follows the .NET coding conventions
(https://learn.microsoft.com/dotnet/csharp/fundamentals/coding-style/coding-conventions)
verified with `dotnet format`
- [] All unit tests pass, and I have added new tests where possible -
_tests appear to fail locally for config related reasons._
- [ ] I didn't break anyone 😄 - _maybe myself, a bit..._

---------

Co-authored-by: Ian Norris <ianorr@microsoft.com>
Co-authored-by: Adrian Bonar <56417140+adrianwyatt@users.noreply.github.com>
Co-authored-by: Gil LaHaye <gillahaye@microsoft.com>
Co-authored-by: Ben Thomas <ben.thomas@microsoft.com>
golden-aries pushed a commit to golden-aries/semantic-kernel that referenced this pull request Oct 10, 2023
@IanNorris @IanNorrisRare
@adrianwyatt @glahaye @alliscode
Initial pass at AppInsights sending custom events (microsoft#899)
402d00b 
### Motivation and Context

Tracking high level user engagement of features can direct further
development and enable measurements of active user counts. It is
important to note that completions and conversations themselves should
not be logged, just data useful in aggregate.

### Description

This is an initial pass for review by the team.

### Motivation and Context
<!-- Thank you for your contribution to the semantic-kernel repo!
Please help reviewers and future users, providing the following
information:
  1. Why is this change required?
  2. What problem does it solve?
  3. What scenario does it contribute to?
  4. If it fixes an open issue, please link to the issue here.
-->

1. This change is essential for tracking user engagement and creating an
audit trail of user activity.
2. This change adds a basic framework for tracking key telemetry events
3. This scenario enables management of those teams deploying instances
to understand how their deployment is being used by their team.
4. This is not related to an open issue.

### Description
<!-- Describe your changes, the overall approach, the underlying design.
These notes will help understanding how your code works. Thanks! -->

The overall design is to add a service accessible to controllers and
other services that enables telemetry to be fired with minimal context.
For this an abstract ITelemetryService interface was added, allowing
systems to fire events on this from across the kernel without any
dependencies on a specific implementation.

### Contribution Checklist
<!-- Before submitting this PR, please make sure: -->
- [X] The code builds clean without any errors or warnings
- [X] The PR follows SK Contribution Guidelines
(https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md)
- [X] The code follows the .NET coding conventions
(https://learn.microsoft.com/dotnet/csharp/fundamentals/coding-style/coding-conventions)
verified with `dotnet format`
- [] All unit tests pass, and I have added new tests where possible -
_tests appear to fail locally for config related reasons._
- [ ] I didn't break anyone 😄 - _maybe myself, a bit..._

---------

Co-authored-by: Ian Norris <ianorr@microsoft.com>
Co-authored-by: Adrian Bonar <56417140+adrianwyatt@users.noreply.github.com>
Co-authored-by: Gil LaHaye <gillahaye@microsoft.com>
Co-authored-by: Ben Thomas <ben.thomas@microsoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alliscode alliscode alliscode approved these changes

@glahaye glahaye glahaye left review comments

@adrianwyatt adrianwyatt adrianwyatt left review comments

@shawncal shawncal Awaiting requested review from shawncal

Assignees

@alliscode alliscode

Labels
.NET Issue or Pull requests regarding .NET code PR: feedback to address Waiting for PR owner to address comments/questions
Projects
Semantic Kernel
Archived in project
Apps & Services Semantic Kernel
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@IanNorris @glahaye @alliscode @adrianwyatt @teresaqhoang @IanNorrisRare