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

Implement hash for App Insights session ID #3317

Merged
merged 2 commits into from
Jan 30, 2020
Merged

Implement hash for App Insights session ID #3317

merged 2 commits into from
Jan 30, 2020

Conversation

garypretty
Copy link
Contributor

@garypretty garypretty commented Jan 30, 2020 •
edited
Loading

Addresses this issue reported in JS microsoft/botbuilder-js#1512

App Insights has a maximum session ID of 64 characters, but in some instances for some channels (such as reported with Teams) this may be exceeded due to conversation ID currently being used for session ID. This PR hashes the conversation ID and sets this as the session ID. It also adds an additional telemetry property to ensure we retain the original conversation ID within the telemetry.

The hashed ID is only used for Application Insights and the original conversation ID and activity are left untouched.

@garypretty garypretty closed this Jan 30, 2020
@garypretty garypretty reopened this Jan 30, 2020
@coveralls
Copy link
Collaborator

coveralls commented Jan 30, 2020 •
edited
Loading

Pull Request Test Coverage Report for Build 102649

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 7 unchanged lines in 1 file lost coverage.
  • Overall coverage increased (+1.02%) to 79.077%
Files with Coverage Reduction New Missed Lines %
/libraries/integration/Microsoft.Bot.Builder.Integration.ApplicationInsights.Core/TelemetryBotIdInitializer.cs 7 86.21%
Totals Coverage Status
Change from base Build 102541: 1.02%
Covered Lines: 17249
Relevant Lines: 21813
💛 - Coveralls

@garypretty garypretty mentioned this pull request Jan 30, 2020
Merged
@fuselabs
Copy link
Collaborator

✔️ No Binary Compatibility issues for Microsoft.Bot.Builder.dll compared against version 4.6.3
✔️ No Binary Compatibility issues for Microsoft.Bot.Builder.AI.Luis.dll compared against version 4.6.3
✔️ No Binary Compatibility issues for Microsoft.Bot.Builder.AI.QnA.dll compared against version 4.6.3
✔️ No Binary Compatibility issues for Microsoft.Bot.Builder.ApplicationInsights.dll compared against version 4.6.3
✔️ No Binary Compatibility issues for Microsoft.Bot.Builder.Azure.dll compared against version 4.6.3
✔️ No Binary Compatibility issues for Microsoft.Bot.Builder.Dialogs.dll compared against version 4.6.3
✔️ No Binary Compatibility issues for Microsoft.Bot.Builder.Integration.ApplicationInsights.Core.dll compared against version 4.6.3
✔️ No Binary Compatibility issues for Microsoft.Bot.Builder.Integration.AspNet.Core.dll compared against version 4.6.3
✔️ No Binary Compatibility issues for Microsoft.Bot.Builder.TemplateManager.dll compared against version 4.6.3
✔️ No Binary Compatibility issues for Microsoft.Bot.Builder.Testing.dll compared against version 4.6.3
✔️ No Binary Compatibility issues for Microsoft.Bot.Configuration.dll compared against version 4.6.3
✔️ No Binary Compatibility issues for Microsoft.Bot.Connector.dll compared against version 4.6.3
✔️ No Binary Compatibility issues for Microsoft.Bot.Schema.dll compared against version 4.6.3
✔️ No Binary Compatibility issues for Microsoft.Bot.Streaming.dll compared against version 4.6.3

1 similar comment
@fuselabs
Copy link
Collaborator

✔️ No Binary Compatibility issues for Microsoft.Bot.Builder.dll compared against version 4.6.3
✔️ No Binary Compatibility issues for Microsoft.Bot.Builder.AI.Luis.dll compared against version 4.6.3
✔️ No Binary Compatibility issues for Microsoft.Bot.Builder.AI.QnA.dll compared against version 4.6.3
✔️ No Binary Compatibility issues for Microsoft.Bot.Builder.ApplicationInsights.dll compared against version 4.6.3
✔️ No Binary Compatibility issues for Microsoft.Bot.Builder.Azure.dll compared against version 4.6.3
✔️ No Binary Compatibility issues for Microsoft.Bot.Builder.Dialogs.dll compared against version 4.6.3
✔️ No Binary Compatibility issues for Microsoft.Bot.Builder.Integration.ApplicationInsights.Core.dll compared against version 4.6.3
✔️ No Binary Compatibility issues for Microsoft.Bot.Builder.Integration.AspNet.Core.dll compared against version 4.6.3
✔️ No Binary Compatibility issues for Microsoft.Bot.Builder.TemplateManager.dll compared against version 4.6.3
✔️ No Binary Compatibility issues for Microsoft.Bot.Builder.Testing.dll compared against version 4.6.3
✔️ No Binary Compatibility issues for Microsoft.Bot.Configuration.dll compared against version 4.6.3
✔️ No Binary Compatibility issues for Microsoft.Bot.Connector.dll compared against version 4.6.3
✔️ No Binary Compatibility issues for Microsoft.Bot.Schema.dll compared against version 4.6.3
✔️ No Binary Compatibility issues for Microsoft.Bot.Streaming.dll compared against version 4.6.3

@johnataylor johnataylor deleted the gary/appinsights-sessionid-length branch January 30, 2020 18:30
@johnataylor johnataylor restored the gary/appinsights-sessionid-length branch January 30, 2020 19:39
@johnataylor johnataylor reopened this Jan 30, 2020
@johnataylor johnataylor merged commit 0f401dd into master Jan 30, 2020
This was referenced Jan 30, 2020
Closed
Closed
Closed
@garypretty garypretty deleted the gary/appinsights-sessionid-length branch February 28, 2020 11:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@johnataylor johnataylor johnataylor approved these changes

@daveta daveta Awaiting requested review from daveta

@cleemullins cleemullins Awaiting requested review from cleemullins

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@garypretty @coveralls @fuselabs @johnataylor