-
Notifications
You must be signed in to change notification settings - Fork 287
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
Support CosmosDb dependency tracking #2635
Conversation
/cc @sourabh1007 |
...ollector/DependencyCollector/Implementation/AzureSdk/AzureSdkDiagnosticListenerSubscriber.cs
Show resolved
Hide resolved
Why PR headline talk about Cosmos DB "Direct Mode"? is it connection mode specific changes? |
WEB/Src/DependencyCollector/DependencyCollector/Implementation/RemoteDependencyConstants.cs
Outdated
Show resolved
Hide resolved
...encyCollector/DependencyCollector/Implementation/AzureSdk/AzureSdkDiagnosticsEventHandler.cs
Outdated
Show resolved
Hide resolved
...ollector/DependencyCollector/Implementation/AzureSdk/AzureSdkDiagnosticListenerSubscriber.cs
Outdated
Show resolved
Hide resolved
Events are loosing diagnostics message while listened by appinsight..below is the log I got Application Insights Telemetry: {"name":"AppTraces","time":"2022-09-23T17:42:26.1395222Z","iKey":"2fabff39-6a32-42da-9e8f-9fcff7d99c6b","tags":{"ai.application.ver":"1.0.0.0","ai.cloud.roleInstance":"DESKTOP-I8MO142","ai.operation.id":"2239023b88bb44b60eab840e1983cded","ai.operation.parentId":"80155111011166ad","ai.internal.sdkVersion":"dotnetc:2.22.0-14","ai.internal.nodeName":"DESKTOP-I8MO142"},"data":{"baseType":"MessageData","baseData":{"ver":2,"message":"n/a","severityLevel":"Warning","properties":{"EventName":"WriteWarningEvent","EventId":"2","AspNetCoreEnvironment":"Development","DeveloperMode":"true","CategoryName":"Azure.Cosmos"}}}} Cosmosdb SDK doesn't send n/a. |
56e0a58
to
130a894
Compare
Fixed, please check |
for this we need portal team to support Azure CosmosDB (in addition to DocumentDB) - there is nothing I can do on SDK side |
but previously this icon was coming. in this latest package only it is not showing up |
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.
Looks fine to me. Thanks :)
...Src/DependencyCollector/DependencyCollector/Implementation/AzureSdk/AzureSdkEventListener.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.
LGTM.
...ollector/DependencyCollector/Implementation/AzureSdk/AzureSdkDiagnosticListenerSubscriber.cs
Show resolved
Hide resolved
...encyCollector/DependencyCollector/Implementation/AzureSdk/AzureSdkDiagnosticsEventHandler.cs
Outdated
Show resolved
Hide resolved
...encyCollector/DependencyCollector/Implementation/AzureSdk/AzureSdkDiagnosticsEventHandler.cs
Outdated
Show resolved
Hide resolved
...encyCollector/DependencyCollector/Implementation/AzureSdk/AzureSdkDiagnosticsEventHandler.cs
Show resolved
Hide resolved
...encyCollector/DependencyCollector/Implementation/AzureSdk/AzureSdkDiagnosticsEventHandler.cs
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.
LGTM, left few minor comments.
Given we do not have true e2e tests, would you update the PR description (or a new issue), to act as the documentation for this enhancement? To answer potential customer/support questions like:
What is the min version of CosmosDb package required?
Whether this works for TCP mode as well? Would we see duplicate if Http mode is used?
Also add a note that TraceTelemetry is produced for certain scenarios (this is unusual compared to other dependencies collected automatically)
How are target/name populated?
"rdddsaz" would be the sdkversion I assume..
Thank you @reyang and @cijothomas for the review! I believe I addressed all the feedback in the code. PTAL. As for documentation, I updated the description on this PR, just need to add a few details. @sourabh1007, can you please help us clarify the following:
Thanks! |
Cosmos DB SDK (Microsoft.Azure.Cosmos v3.TODO now supports dependency tracking, even when used in
Direct
(TCP) mode.You should see
DependencyTelemetry
reported for all public API calls (that involve network communication with the service). If you useGateway
(HTTP) mode, you would also see dependencies for underlying HTTP calls. ForDirect
calls, you might see a few nested HTTP dependencies too, but low-level TCP calls are not traced.Attributes reported by Cosmos SDK can be found here: Azure/azure-cosmos-dotnet-v3#3058. They are populated as custom dimensions.
New dependencies tracing public API calls should have:
Target
following{CosmosDB resource name} | {database name}
pattern, e.g.cosmos-tracing-demo | my-store
.Name
follows{container name} | {operation}
pattern, e.g.orders | Read
Type
matches Resource Provider namespace (Microsoft.DocumentDB
)sdkversion
matchesrdddsaz
.You can enable distributed tracing for Cosmos with the following API: TODO, add a link to cosmos docs.
In addition to dependency calls, Cosmos DB reports extensive diagnostics when an operation fails or takes too long. You can configure it with the following settings on the Cosmos DB client - TODO.
This diagnostics is sent as a log (
TraceTelemetry
) withWarning
severity havingCosmosDiagnostics
as JSON in the message. You can identify it usingEventName
(in custom dimensions) with theRecordDiagnosticsForRequests
value.Such
TraceTelemetry
should be a child of Cosmos DB dependency telemetry described above.Changes
CosmosDB direct (TCP mode) dependency tracking (coming in future versions of Cosmos DB v3 SDK).
Uses attributes outlined here and additional cosmos diagnostics is coming from the specific event source and is tracked as
TraceTelemetry
.EventSource verbosity is expected to be low - it will be controlled by CosmosDb customers on CosmosDb side. Cosmos would write it when an operation takes longer than a configurable threshold or when an error happens.
Checklist
Notes for reviewers:
/AzurePipelines run
will queue all builds/AzurePipelines run <pipeline-name>
will queue a specific build