-
Notifications
You must be signed in to change notification settings - Fork 494
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
Tracing: Adds ITrace as the default tracing implementation for CosmosDiagnostics #2097
Conversation
Existing diagnostics was reasonable. Similar we had on Java and working great. |
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.
Waiting for the goals review meeting follow-up.
Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.EmulatorTests/CosmosDiagnosticsTests.cs
Outdated
Show resolved
Hide resolved
Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.EmulatorTests/CosmosDiagnosticsTests.cs
Outdated
Show resolved
Hide resolved
Microsoft.Azure.Cosmos/src/Diagnostics/CosmosTraceDiagnostics.cs
Outdated
Show resolved
Hide resolved
...icrosoft.Azure.Cosmos.Tests/BaselineTest/TestBaseline/TraceWriterBaselineTests.TraceData.xml
Outdated
Show resolved
Hide resolved
...icrosoft.Azure.Cosmos.Tests/BaselineTest/TestBaseline/TraceWriterBaselineTests.TraceData.xml
Outdated
Show resolved
Hide resolved
...icrosoft.Azure.Cosmos.Tests/BaselineTest/TestBaseline/TraceWriterBaselineTests.TraceData.xml
Outdated
Show resolved
Hide resolved
...icrosoft.Azure.Cosmos.Tests/BaselineTest/TestBaseline/TraceWriterBaselineTests.TraceData.xml
Outdated
Show resolved
Hide resolved
...icrosoft.Azure.Cosmos.Tests/BaselineTest/TestBaseline/TraceWriterBaselineTests.TraceData.xml
Outdated
Show resolved
Hide resolved
Sync-up with Samer, Fabian, Jake, Tim
|
…/github.com/Azure/azure-cosmos-dotnet-v3 into users/brchon/TracingRemoveCosmosDiagnostics
…s was not included in exception scenarios (#2375) The ITrace was only adding the client side request stats to the ITrace on success scenarios. If there was any exception the ITrace would not be included. This changes the logic to always add the client side request stats to the ITrace. The regression was introduced in 3.17.0 with PR #2097
Tracing: Adds ITrace as the default tracing implementation for CosmosDiagnostics
Description
This PR removes CosmosDiagnosticsContext in favor of ITrace, which has an easier to read pretty print along with other benefits mentioned here:
#1841
For Compute, they will need to update their diagnostics visitors, which should be simple, since ITrace is a tree like structure, so visibility is a first class concept. ITrace will then expose unstructured data like QueryMetrics and ClientSideRequestStats through the Data properties. Users can then visit on the TraceDatum class to get the derived class and it's appropriate properties.
Follow up work items are to expose ITrace in all our async APIs. Currently we only provide it in our FeedOperations, but eventually there will be a need for point operations.
Tests have been updated and new baseline tests are added, so that we can see what the text output looks like at all times.
Here is the text output for each of the TraceData:
Point Operation Statistics
QueryMetrics
Client Side Request Stats
CPU History