Skip to content

Commit b2bccef

Browse files
authored
Add a constant dimension to package push metrics (#9213)
This allows us to measure package push SLO all as one % instead of splitting it by user tenant. Splitting by user tenant has proven to be very noisy and not an accurate representation of how we see push availability on NuGet.org. Progress on NuGet/Engineering#4524
1 parent 41027e0 commit b2bccef

2 files changed

Lines changed: 18 additions & 6 deletions

File tree

src/NuGetGallery/Telemetry/CustomerResourceIdEnricher.cs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ namespace NuGetGallery
1313
public class CustomerResourceIdEnricher : ITelemetryInitializer
1414
{
1515
private const string CustomerResourceId = "CustomerResourceId";
16+
private const string CustomerResourceIdConstant = "CustomerResourceIdConstant";
1617
private const string Prefix = "/tenants/";
1718
private static readonly string Empty = Prefix + Guid.Empty.ToString();
1819

@@ -49,6 +50,15 @@ public void Initialize(ITelemetry telemetry)
4950
var tenantId = httpContext?.User?.Identity.GetTenantIdOrNull();
5051
var customerResourceId = tenantId != null ? Prefix + tenantId : Empty;
5152
itemTelemetry.Properties[CustomerResourceId] = customerResourceId;
53+
54+
// This is necessary for the NuGet SLO/SLO pipeline to signal that there is a single "tenant" on NuGet.org
55+
// when measuring package push availability. This is an alernative approach to splitting package push
56+
// availability by authenticated user tenant (the "CustomerResourceId" property added above). The drawback
57+
// for using the authenticated user tenant is that it splits the availability measurement by tenant, leading
58+
// to very noisy results since the total volume per user tenant is quite low.
59+
//
60+
// Any constant value can be used here.
61+
itemTelemetry.Properties[CustomerResourceIdConstant] = nameof(NuGetGallery);
5262
}
5363

5464
protected virtual HttpContextBase GetHttpContext() => HttpContextBaseExtensions.GetCurrent();

tests/NuGetGallery.Facts/Telemetry/CustomerResourceIdEnricherFacts.cs

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -33,11 +33,11 @@ protected override HttpContextBase GetHttpContext()
3333
}
3434

3535
[Theory]
36-
[InlineData(typeof(RequestTelemetry), 0)]
37-
[InlineData(typeof(DependencyTelemetry), 0)]
38-
[InlineData(typeof(TraceTelemetry), 0)]
39-
[InlineData(typeof(ExceptionTelemetry), 0)]
40-
[InlineData(typeof(MetricTelemetry), 1)]
36+
[InlineData(typeof(RequestTelemetry), 1)]
37+
[InlineData(typeof(DependencyTelemetry), 1)]
38+
[InlineData(typeof(TraceTelemetry), 1)]
39+
[InlineData(typeof(ExceptionTelemetry), 1)]
40+
[InlineData(typeof(MetricTelemetry), 3)]
4141
public void EnrichesOnlyMetricTelemetry(Type telemetryType, int addedProperties)
4242
{
4343
// Arrange
@@ -56,7 +56,7 @@ public void EnrichesOnlyMetricTelemetry(Type telemetryType, int addedProperties)
5656
enricher.Initialize(telemetry);
5757

5858
// Assert
59-
Assert.Equal(1 + addedProperties, itemTelemetry.Properties.Count);
59+
Assert.Equal(addedProperties, itemTelemetry.Properties.Count);
6060
}
6161

6262
[Fact]
@@ -88,6 +88,7 @@ public void EnrichesTelemetryWithTenantId(string name)
8888

8989
// Assert
9090
Assert.Equal("/tenants/tenant-id", telemetry.Properties["CustomerResourceId"]);
91+
Assert.Equal("NuGetGallery", telemetry.Properties["CustomerResourceIdConstant"]);
9192
}
9293

9394
[Theory]
@@ -104,6 +105,7 @@ public void EnrichesTelemetryWithEmptyWhenTenantIdIsNotInClaims(string name)
104105

105106
// Assert
106107
Assert.Equal("/tenants/00000000-0000-0000-0000-000000000000", telemetry.Properties["CustomerResourceId"]);
108+
Assert.Equal("NuGetGallery", telemetry.Properties["CustomerResourceIdConstant"]);
107109
}
108110

109111
private TestableCustomerResourceIdEnricher CreateTestEnricher(IReadOnlyDictionary<string, string> claims)

0 commit comments

Comments
 (0)