track telemetry on api requests #3355
Conversation
src/Microsoft.Health.Dicom.Api/Features/Telemetry/HttpDicomTelemetryClient.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.Health.Dicom.Api/Features/Telemetry/DicomTelemetry.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.Health.Dicom.Api/Features/Telemetry/HttpDicomTelemetryClient.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.Health.Dicom.Api/Logging/DicomTelemetryInitializer.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.Health.Dicom.Api/Logging/DicomTelemetryInitializer.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.Health.Dicom.Api/Registration/DicomServerServiceCollectionExtensions.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.Health.Dicom.Core/Extensions/DicomTelemetryClientExtensions.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.Health.Dicom.Core/Features/Telemetry/IDicomTelemetryClient.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.Health.Dicom.SqlServer/Features/Schema/Model/V56.Generated.cs
Outdated
Show resolved
Hide resolved
| foreach ((string key, string value) in properties) | ||
| { | ||
| if (requestTelemetry.Properties.ContainsKey(key)) | ||
| requestTelemetry.Properties["DuplicateDimension"] = bool.TrueString; |
There was a problem hiding this comment.
why do we need this DuplicateDimension? Are you expecting any of the dimension to be already present?
There was a problem hiding this comment.
yes - it's not guaranteed that we'd get a single one so we have to carefully filter out and not try to add duplicates. We're guaranteed at least one, but could have duplicates. We follow the same pattern elsewhere
| public void Initialize(ITelemetry telemetry) | ||
| { | ||
| if (telemetry is RequestTelemetry requestTelemetry) | ||
| AddPropertiesFromHttpContextItems(requestTelemetry); |
There was a problem hiding this comment.
Will this step gets executed for all the health check requests too?
There was a problem hiding this comment.
yes - I don't see any reason to treat healthchecks differently, it can cause confusion when we go do our queries. Do you foresee issues with treating all requests the same?
There was a problem hiding this comment.
That is just some extra unwanted processing, since health check happens every 15-30 seconds. You can keep it if you need these columns to be populated in RequestTelemetry for health checks.
| public void Initialize(ITelemetry telemetry) | ||
| { | ||
| if (telemetry is RequestTelemetry requestTelemetry) | ||
| AddPropertiesFromHttpContextItems(requestTelemetry); |
There was a problem hiding this comment.
That is just some extra unwanted processing, since health check happens every 15-30 seconds. You can keep it if you need these columns to be populated in RequestTelemetry for health checks.
Description
Start tracking additional telemetry api requests:
min, max and total bytes and instance count.
Example of how it shows up in telemetry:
Related issues
Addresses [AB#105723].
Testing
Describe how this change was tested.