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.

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

track telemetry on api requests #3355

Merged
merged 13 commits into from
Feb 12, 2024
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
// -------------------------------------------------------------------------------------------------
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License (MIT). See LICENSE in the repo root for license information.
// -------------------------------------------------------------------------------------------------

namespace Microsoft.Health.Dicom.Api.Features.Telemetry;
esmadau marked this conversation as resolved.
Show resolved Hide resolved
internal static class DicomTelemetry
{
public const string ContextItemPrefix = "Dicom_";
}
esmadau marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
// -------------------------------------------------------------------------------------------------
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License (MIT). See LICENSE in the repo root for license information.
// -------------------------------------------------------------------------------------------------

using EnsureThat;
using Microsoft.ApplicationInsights;
using Microsoft.AspNetCore.Http;
using Microsoft.Health.Dicom.Core.Features.Telemetry;

namespace Microsoft.Health.Dicom.Api.Features.Telemetry;

internal class HttpDicomTelemetryClient : IDicomTelemetryClient
{
private readonly TelemetryClient _telemetryClient;
private readonly IHttpContextAccessor _httpContextAccessor;

public HttpDicomTelemetryClient(TelemetryClient telemetryClient, IHttpContextAccessor httpContextAccessor)
{
_telemetryClient = EnsureArg.IsNotNull(telemetryClient, nameof(telemetryClient));
_httpContextAccessor = EnsureArg.IsNotNull(httpContextAccessor, nameof(httpContextAccessor));
}

// Note: Context Items are prefixed so that the telemetry initializer knows which items to include in the telemetry

esmadau marked this conversation as resolved.
Show resolved Hide resolved
public void TrackMetric(string name, int value)
{
_httpContextAccessor.HttpContext.Items[DicomTelemetry.ContextItemPrefix + name] = value;
_telemetryClient.GetMetric(name).TrackValue(value);
}

public void TrackMetric(string name, long value)
{
_httpContextAccessor.HttpContext.Items[DicomTelemetry.ContextItemPrefix + name] = value;
_telemetryClient.GetMetric(name).TrackValue(value);
}
}
esmadau marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
// -------------------------------------------------------------------------------------------------
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License (MIT). See LICENSE in the repo root for license information.
// -------------------------------------------------------------------------------------------------

using System;
using System.Collections.Generic;
using System.Linq;
using EnsureThat;
using Microsoft.ApplicationInsights.Channel;
using Microsoft.ApplicationInsights.DataContracts;
using Microsoft.ApplicationInsights.Extensibility;
using Microsoft.AspNetCore.Http;

namespace Microsoft.Health.Dicom.Api.Logging;

public class DicomTelemetryInitializer : ITelemetryInitializer
{
private readonly IHttpContextAccessor _httpContextAccessor;

private const string DicomPrefix = "Dicom_";
esmadau marked this conversation as resolved.
Show resolved Hide resolved

public DicomTelemetryInitializer(IHttpContextAccessor httpContextAccessor)
{
_httpContextAccessor = EnsureArg.IsNotNull(httpContextAccessor, nameof(httpContextAccessor));
}

public void Initialize(ITelemetry telemetry)
{
if (telemetry is RequestTelemetry requestTelemetry)
AddPropertiesFromHttpContextItems(requestTelemetry);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will this step gets executed for all the health check requests too?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

}

private void AddPropertiesFromHttpContextItems(RequestTelemetry requestTelemetry)
{
if (_httpContextAccessor.HttpContext == null)
{
return;
}

IEnumerable<(string Key, string Value)> properties = _httpContextAccessor.HttpContext
.Items
.Select(x => (Key: x.Key.ToString(), x.Value))
.Where(x => x.Key.StartsWith(DicomPrefix, StringComparison.Ordinal))
.Select(x => (x.Key[DicomPrefix.Length..], x.Value?.ToString()));

foreach ((string key, string value) in properties)
{
if (requestTelemetry.Properties.ContainsKey(key))
requestTelemetry.Properties["DuplicateDimension"] = bool.TrueString;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do we need this DuplicateDimension? Are you expecting any of the dimension to be already present?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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


requestTelemetry.Properties[key] = value;
}
}
}
esmadau marked this conversation as resolved.
Show resolved Hide resolved
19 changes: 18 additions & 1 deletion src/Microsoft.Health.Dicom.Api/Logging/TelemetryInitializer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
using Microsoft.ApplicationInsights.Extensibility;
using Microsoft.AspNetCore.Http;
using Microsoft.AspNetCore.Mvc.Versioning;
using Microsoft.Extensions.Options;
using Microsoft.Health.Dicom.Core.Configs;

namespace Microsoft.Health.Dicom.Api.Logging;

Expand All @@ -18,11 +20,22 @@ namespace Microsoft.Health.Dicom.Api.Logging;
internal class TelemetryInitializer : ITelemetryInitializer
{
private readonly IHttpContextAccessor _httpContextAccessor;
private readonly bool _enableDataPartitions;
private readonly bool _enableExport;
private readonly bool _enableExternalStore;
private const string ApiVersionColumnName = "ApiVersion";
private const string EnableDataPartitions = "EnableDataPartitions";
private const string EnableExport = "EnableExport";
private const string EnableExternalStore = "EnableExternalStore";
private const string UserAgent = "UserAgent";

public TelemetryInitializer(IHttpContextAccessor httpContextAccessor)
public TelemetryInitializer(IHttpContextAccessor httpContextAccessor, IOptions<FeatureConfiguration> featureConfiguration)
{
_httpContextAccessor = EnsureArg.IsNotNull(httpContextAccessor, nameof(httpContextAccessor));
EnsureArg.IsNotNull(featureConfiguration?.Value, nameof(featureConfiguration));
_enableDataPartitions = featureConfiguration.Value.EnableDataPartitions;
_enableExport = featureConfiguration.Value.EnableExport;
_enableExternalStore = featureConfiguration.Value.EnableExternalStore;
}

public void Initialize(ITelemetry telemetry)
Expand Down Expand Up @@ -52,5 +65,9 @@ private void AddApiVersionColumn(ITelemetry telemetry)
}

requestTelemetry.Properties[ApiVersionColumnName] = version;
requestTelemetry.Properties[EnableDataPartitions] = _enableDataPartitions.ToString();
requestTelemetry.Properties[EnableExport] = _enableExport.ToString();
requestTelemetry.Properties[EnableExternalStore] = _enableExternalStore.ToString();
requestTelemetry.Properties[UserAgent] = _httpContextAccessor.HttpContext.Request.Headers.UserAgent;
esmadau marked this conversation as resolved.
Show resolved Hide resolved
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,14 @@
using Microsoft.Health.Dicom.Api.Features.Partitioning;
using Microsoft.Health.Dicom.Api.Features.Routing;
using Microsoft.Health.Dicom.Api.Features.Swagger;
using Microsoft.Health.Dicom.Api.Features.Telemetry;
using Microsoft.Health.Dicom.Api.Logging;
using Microsoft.Health.Dicom.Core.Configs;
using Microsoft.Health.Dicom.Core.Extensions;
using Microsoft.Health.Dicom.Core.Features.Context;
using Microsoft.Health.Dicom.Core.Features.FellowOakDicom;
using Microsoft.Health.Dicom.Core.Features.Routing;
using Microsoft.Health.Dicom.Core.Features.Telemetry;
using Microsoft.Health.Dicom.Core.Registration;
using Microsoft.Health.Encryption.Customer.Configs;
using Microsoft.Health.Encryption.Customer.Extensions;
Expand Down Expand Up @@ -174,6 +176,9 @@ public static IDicomServerBuilder AddDicomServer(
services.AddRecyclableMemoryStreamManager(configurationRoot);

services.AddSingleton<ITelemetryInitializer, TelemetryInitializer>();
// track common Dicom server specific requests telemetry
services.AddSingleton<ITelemetryInitializer, DicomTelemetryInitializer>();
esmadau marked this conversation as resolved.
Show resolved Hide resolved
services.AddSingleton<IDicomTelemetryClient, HttpDicomTelemetryClient>();

CustomDicomImplementation.SetDicomImplementationClassUIDAndVersion();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ public class DicomStoreServiceTests
TelemetryChannel = Substitute.For<ITelemetryChannel>(),
});

private readonly IDicomTelemetryClient _dicomTelemetryClient = Substitute.For<IDicomTelemetryClient>();
private readonly StoreService _storeService;
private readonly StoreService _storeServiceDropData;

Expand All @@ -94,6 +95,7 @@ public DicomStoreServiceTests()
_storeMeter,
NullLogger<StoreService>.Instance,
Options.Create(new FeatureConfiguration { }),
_dicomTelemetryClient,
_telemetryClient);

_storeServiceDropData = new StoreService(
Expand All @@ -104,6 +106,7 @@ public DicomStoreServiceTests()
_storeMeter,
NullLogger<StoreService>.Instance,
Options.Create(new FeatureConfiguration { }),
_dicomTelemetryClient,
_telemetryClient);

DicomValidationBuilderExtension.SkipValidation(null);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
// -------------------------------------------------------------------------------------------------
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License (MIT). See LICENSE in the repo root for license information.
// -------------------------------------------------------------------------------------------------

using EnsureThat;
using Microsoft.Health.Dicom.Core.Features.Telemetry;

namespace Microsoft.Health.Dicom.Core.Extensions;

internal static class DicomTelemetryClientExtensions
{
public static void TrackInstanceCount(this IDicomTelemetryClient telemetryClient, int count)
{
EnsureArg.IsNotNull(telemetryClient, nameof(telemetryClient));
telemetryClient.TrackMetric("InstanceCount", count);
}

public static void TrackTotalInstanceBytes(this IDicomTelemetryClient telemetryClient, long bytes)
{
EnsureArg.IsNotNull(telemetryClient, nameof(telemetryClient));
telemetryClient.TrackMetric("TotalInstanceBytes", bytes);
}

public static void TrackMinInstanceBytes(this IDicomTelemetryClient telemetryClient, long bytes)
{
EnsureArg.IsNotNull(telemetryClient, nameof(telemetryClient));
telemetryClient.TrackMetric("MinInstanceBytes", bytes);
}

public static void TrackMaxInstanceBytes(this IDicomTelemetryClient telemetryClient, long bytes)
{
EnsureArg.IsNotNull(telemetryClient, nameof(telemetryClient));
telemetryClient.TrackMetric("MaxInstanceBytes", bytes);
}
}
esmadau marked this conversation as resolved.
Show resolved Hide resolved
16 changes: 15 additions & 1 deletion src/Microsoft.Health.Dicom.Core/Features/Store/StoreService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ public class StoreService : IStoreService
private readonly IStoreDatasetValidator _dicomDatasetValidator;
private readonly IStoreOrchestrator _storeOrchestrator;
private readonly IDicomRequestContextAccessor _dicomRequestContextAccessor;
private readonly IDicomTelemetryClient _dicomTelemetryClient;
private readonly TelemetryClient _telemetryClient;
private readonly StoreMeter _storeMeter;
private readonly ILogger _logger;
Expand All @@ -80,14 +81,16 @@ public StoreService(
StoreMeter storeMeter,
ILogger<StoreService> logger,
IOptions<FeatureConfiguration> featureConfiguration,
IDicomTelemetryClient dicomTelemetryClient,
TelemetryClient telemetryClient)
{
EnsureArg.IsNotNull(featureConfiguration?.Value, nameof(featureConfiguration));
_storeResponseBuilder = EnsureArg.IsNotNull(storeResponseBuilder, nameof(storeResponseBuilder));
_dicomDatasetValidator = EnsureArg.IsNotNull(dicomDatasetValidator, nameof(dicomDatasetValidator));
_storeOrchestrator = EnsureArg.IsNotNull(storeOrchestrator, nameof(storeOrchestrator));
_dicomRequestContextAccessor = EnsureArg.IsNotNull(dicomRequestContextAccessor, nameof(dicomRequestContextAccessor));
_telemetryClient = EnsureArg.IsNotNull(telemetryClient, nameof(telemetryClient));
_dicomTelemetryClient = EnsureArg.IsNotNull(dicomTelemetryClient, nameof(dicomTelemetryClient));
_telemetryClient = EnsureArg.IsNotNull(telemetryClient, nameof(_telemetryClient));
_storeMeter = EnsureArg.IsNotNull(storeMeter, nameof(storeMeter));
_logger = EnsureArg.IsNotNull(logger, nameof(logger));
}
Expand All @@ -104,6 +107,9 @@ public async Task<StoreResponse> ProcessAsync(
_dicomRequestContextAccessor.RequestContext.PartCount = instanceEntries.Count;
_dicomInstanceEntries = instanceEntries;
_requiredStudyInstanceUid = requiredStudyInstanceUid;
_dicomTelemetryClient.TrackInstanceCount(instanceEntries.Count);

long totalLength = 0, minLength = 0, maxLength = 0;

for (int index = 0; index < instanceEntries.Count; index++)
{
Expand All @@ -113,13 +119,21 @@ public async Task<StoreResponse> ProcessAsync(
if (length != null)
{
long len = length.GetValueOrDefault();
totalLength += len;
minLength = Math.Min(minLength, len);
maxLength = Math.Max(maxLength, len);
// Update Telemetry
_storeMeter.InstanceLength.Record(len);
_dicomRequestContextAccessor.RequestContext.TotalDicomEgressToStorageBytes += len;
}
}
finally
{
// Update Requests Telemetry
_dicomTelemetryClient.TrackTotalInstanceBytes(totalLength);
_dicomTelemetryClient.TrackMinInstanceBytes(minLength);
_dicomTelemetryClient.TrackMaxInstanceBytes(maxLength);

// Fire and forget.
int capturedIndex = index;

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
// -------------------------------------------------------------------------------------------------
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License (MIT). See LICENSE in the repo root for license information.
// -------------------------------------------------------------------------------------------------


namespace Microsoft.Health.Dicom.Core.Features.Telemetry;

public interface IDicomTelemetryClient
{
void TrackMetric(string name, int value);

void TrackMetric(string name, long value);
}
esmadau marked this conversation as resolved.
Show resolved Hide resolved
Loading