-
Notifications
You must be signed in to change notification settings - Fork 343
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
Update Telemetry client with data outlined in spec #3737
Conversation
src/client/Microsoft.Identity.Client/Internal/RequestContext.cs
Outdated
Show resolved
Hide resolved
src/client/Microsoft.Identity.Client/Internal/Requests/RequestBase.cs
Outdated
Show resolved
Hide resolved
src/client/Microsoft.Identity.Client/Internal/Requests/RequestBase.cs
Outdated
Show resolved
Hide resolved
src/client/Microsoft.Identity.Client/Internal/RequestContext.cs
Outdated
Show resolved
Hide resolved
src/client/Microsoft.Identity.Client/TelemetryCore/TelemetryClient/TelemetryCacheConstants.cs
Outdated
Show resolved
Hide resolved
src/client/Microsoft.Identity.Client/TelemetryCore/TelemetryClient/TelemetryCacheConstants.cs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Naming is only blocking change, code looks ok otherwise with minor changes suggested.
src/client/Microsoft.Identity.Client/TelemetryCore/TelemetryClient/TelemetryCacheConstants.cs
Outdated
Show resolved
Hide resolved
src/client/Microsoft.Identity.Client/Internal/Requests/RequestBase.cs
Outdated
Show resolved
Hide resolved
src/client/Microsoft.Identity.Client/TelemetryCore/TelemetryClient/TelemetryDatapoints.cs
Outdated
Show resolved
Hide resolved
src/client/Microsoft.Identity.Client/TelemetryCore/TelemetryClient/TelemetryDatapoints.cs
Outdated
Show resolved
Hide resolved
src/client/Microsoft.Identity.Client/Internal/Requests/RequestBase.cs
Outdated
Show resolved
Hide resolved
Removed the CacheUsed variable as this can be inferred as:
|
src/client/Microsoft.Identity.Client/TelemetryCore/TelemetryConstants.cs
Outdated
Show resolved
Hide resolved
src/client/Microsoft.Identity.Client/TelemetryCore/TelemetryClient/TelemetryDatapoints.cs
Outdated
Show resolved
Hide resolved
src/client/Microsoft.Identity.Client/TelemetryCore/TelemetryClient/TelemetryDatapoints.cs
Outdated
Show resolved
Hide resolved
src/client/Microsoft.Identity.Client/Internal/Requests/RequestBase.cs
Outdated
Show resolved
Hide resolved
src/client/Microsoft.Identity.Client/Internal/Requests/RequestBase.cs
Outdated
Show resolved
Hide resolved
src/client/Microsoft.Identity.Client/Internal/Requests/RequestBase.cs
Outdated
Show resolved
Hide resolved
cc @neha-bhargava for one final look |
src/client/Microsoft.Identity.Client/Internal/Requests/RequestBase.cs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved with comments.
@neha-bhargava - can you please also show Travis how you tested this E2E so that we can see some values in Kusto ? Some manual validation would be great.
src/client/Microsoft.Identity.Client/AppConfig/ConfidentialClientApplicationBuilder.cs
Show resolved
Hide resolved
src/client/Microsoft.Identity.Client/Internal/Requests/RequestBase.cs
Outdated
Show resolved
Hide resolved
src/client/Microsoft.Identity.Client/Internal/Requests/RequestBase.cs
Outdated
Show resolved
Hide resolved
src/client/Microsoft.Identity.Client/Internal/Requests/RequestBase.cs
Outdated
Show resolved
Hide resolved
src/client/Microsoft.Identity.Client/TelemetryCore/TelemetryClient/TelemetryDatapoints.cs
Outdated
Show resolved
Hide resolved
@trwalke - please create a work item on ID.Web to track the work in the cache project. |
src/client/Microsoft.Identity.Client/TelemetryCore/AssertionType.cs
Outdated
Show resolved
Hide resolved
src/client/Microsoft.Identity.Client/TelemetryCore/AssertionType.cs
Outdated
Show resolved
Hide resolved
src/client/Microsoft.Identity.Client/TelemetryCore/TelemetryConstants.cs
Show resolved
Hide resolved
src/client/Microsoft.Identity.Client/Internal/Requests/RequestBase.cs
Outdated
Show resolved
Hide resolved
tests/Microsoft.Identity.Test.Unit/PublicApiTests/TelemetryClientTests.cs
Outdated
Show resolved
Hide resolved
tests/Microsoft.Identity.Test.Unit/PublicApiTests/TelemetryClientTests.cs
Show resolved
Hide resolved
Co-authored-by: Peter M <34331512+pmaytak@users.noreply.github.com>
Refactoring
# Conflicts: # tests/Microsoft.Identity.Test.Integration.netfx/HeadlessTests/PoPTests.cs # tests/Microsoft.Identity.Test.Unit/PublicApiTests/ConfidentialClientApplicationTests.cs
Fixes #3596, fixes #4119.
Changes proposed in this request
Testing
Performance impact
Documentation