Skip to content

Commit 49a175f

Browse files
tjementumtjementum
committed
Round 4 convention and telemetry-test follow-ups (V-R4-1, V-R4-2, T-R4-3, T-R4-4, T-R4-6)
1 parent 49cce33 commit 49a175f

7 files changed

Lines changed: 89 additions & 14 deletions

File tree

‎application/account/Core/Features/FeatureFlags/Shared/PlanBasedFeatureFlagEvaluator.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ public async Task EvaluatePlanFlagsForTenantAsync(TenantId tenantId, Subscriptio
2727
// Required because this evaluator runs on every JWT refresh / login, and concurrent fan-in for
2828
// the same tenant otherwise hits the unique index as a 500. The lock is PostgreSQL-specific;
2929
// in-memory SQLite test runs cannot exhibit cross-process concurrency, so we skip it there.
30-
if (accountDbContext.Database.IsNpgsql())
30+
if (accountDbContext.Database.ProviderName is not "Microsoft.EntityFrameworkCore.Sqlite")
3131
{
3232
await accountDbContext.Database.ExecuteSqlAsync(
3333
$"SELECT pg_advisory_xact_lock(hashtextextended('plan_flags:' || {tenantId.Value}, 0))",

‎application/account/Tests/BackOffice/FeatureFlags/FeatureFlagBackOfficeTests.cs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -675,6 +675,7 @@ public async Task SetRolloutPercentage_WhenValidPercentageAsAdmin_ShouldUpdateBu
675675
CountBucketsInRange((int)rolloutBucketStart.Value, (int)rolloutBucketEnd.Value).Should().Be(50);
676676

677677
TelemetryEventsCollectorSpy.CollectedEvents.Should().ContainSingle(e => e.GetType().Name == "FeatureFlagRolloutPercentageUpdated");
678+
TelemetryEventsCollectorSpy.CollectedEvents[0].Properties["event.from_percentage"].Should().Be("0");
678679
TelemetryEventsCollectorSpy.CollectedEvents[0].Properties["event.to_percentage"].Should().Be("50");
679680
}
680681

‎application/account/Tests/FeatureFlags/FeatureFlagsManifestContractTests.cs‎

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,19 @@
11
using System.Text.Json;
22
using FluentAssertions;
3-
using SharedKernel.FeatureFlags;
43
using Xunit;
54

65
namespace Account.Tests.FeatureFlags;
76

8-
// Codegen contract: every public instance property defined on FeatureFlagDefinition must appear in
9-
// every entry of the generated manifest (application/shared-webapp/ui/featureFlags/featureFlags.generated.json).
10-
// The manifest is the source-of-truth bridge between C# and the TS registry consumed by useFeatureFlag.
11-
// When someone adds a new virtual property to FeatureFlagDefinition without updating
12-
// FeatureFlagsManifestEmitter, this test fails — preventing silent codegen drift.
7+
// Codegen contract: every public instance property defined on every concrete FeatureFlagDefinition
8+
// subtype must appear in the entry of the generated manifest for at least one flag
9+
// (application/shared-webapp/ui/featureFlags/featureFlags.generated.json). The manifest is the
10+
// source-of-truth bridge between C# and the TS registry consumed by useFeatureFlag. Reflecting over
11+
// the base class would miss subtype-only properties (a new `MyNewCapability { get; }` on a future
12+
// subtype), which is exactly the drift class the contract test exists to prevent.
1313
public sealed class FeatureFlagsManifestContractTests
1414
{
1515
[Fact]
16-
public void EveryPublicPropertyOnFeatureFlagDefinition_AppearsInManifestEntry()
16+
public void EveryPublicPropertyOnEveryFlagSubtype_AppearsInManifestEntry()
1717
{
1818
var manifestPath = ResolveManifestPath();
1919
File.Exists(manifestPath).Should().BeTrue($"manifest must exist at '{manifestPath}'; run `build --backend` to regenerate");
@@ -23,17 +23,18 @@ public void EveryPublicPropertyOnFeatureFlagDefinition_AppearsInManifestEntry()
2323
manifest.RootElement.ValueKind.Should().Be(JsonValueKind.Array);
2424
manifest.RootElement.GetArrayLength().Should().BeGreaterThan(0);
2525

26-
var firstEntry = manifest.RootElement[0];
27-
var manifestKeys = firstEntry.EnumerateObject().Select(p => p.Name).ToHashSet(StringComparer.Ordinal);
26+
var manifestKeys = manifest.RootElement.EnumerateArray()
27+
.SelectMany(entry => entry.EnumerateObject().Select(p => p.Name))
28+
.ToHashSet(StringComparer.Ordinal);
2829

29-
var expectedKeys = typeof(FeatureFlagDefinition)
30-
.GetProperties(BindingFlags.Public | BindingFlags.Instance)
30+
var expectedKeys = global::SharedKernel.FeatureFlags.FeatureFlags.GetAll()
31+
.SelectMany(f => f.GetType().GetProperties(BindingFlags.Public | BindingFlags.Instance))
3132
.Select(p => ToCamelCase(p.Name))
3233
.ToHashSet(StringComparer.Ordinal);
3334

3435
var missing = expectedKeys.Except(manifestKeys).ToArray();
3536
missing.Should().BeEmpty(
36-
$"FeatureFlagDefinition exposes properties that the manifest emitter does not project. "
37+
$"At least one FeatureFlagDefinition subtype exposes properties that the manifest emitter does not project. "
3738
+ $"Add these to FeatureFlagsManifestEmitter.cs and regenerate: {string.Join(", ", missing)}"
3839
);
3940
}

‎application/account/Tests/Tenants/BackOffice/SetTenantAbInclusionPinTests.cs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,8 @@ public async Task SetTenantAbInclusionPin_WhenAlwaysOn_ShouldPersistPin()
3232
storedPin.Should().Be(nameof(AbInclusionPin.AlwaysOn));
3333

3434
TelemetryEventsCollectorSpy.CollectedEvents.Should().ContainSingle(e => e.GetType().Name == "TenantAbInclusionPinUpdated");
35+
TelemetryEventsCollectorSpy.CollectedEvents[0].Properties["event.from_pin"].Should().Be("none");
36+
TelemetryEventsCollectorSpy.CollectedEvents[0].Properties["event.to_pin"].Should().Be("AlwaysOn");
3537
}
3638

3739
[Fact]

‎application/account/Tests/Users/BackOffice/SetUserAbInclusionPinTests.cs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,8 @@ public async Task SetUserAbInclusionPin_WhenAlwaysOn_ShouldPersistPin()
3232
storedPin.Should().Be(nameof(AbInclusionPin.AlwaysOn));
3333

3434
TelemetryEventsCollectorSpy.CollectedEvents.Should().ContainSingle(e => e.GetType().Name == "UserAbInclusionPinUpdated");
35+
TelemetryEventsCollectorSpy.CollectedEvents[0].Properties["event.from_pin"].Should().Be("none");
36+
TelemetryEventsCollectorSpy.CollectedEvents[0].Properties["event.to_pin"].Should().Be("AlwaysOn");
3537
}
3638

3739
[Fact]

‎application/account/Workers/FeatureFlagDefinitionReconciler.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ public async Task ReconcileAsync(CancellationToken cancellationToken)
5353
// Session-scoped advisory locks are PostgreSQL-only. In-memory SQLite tests run the same code
5454
// path and would otherwise blow up on `no such function: pg_advisory_lock` — the cross-replica
5555
// race the lock guards against cannot happen in a single-process unit test anyway.
56-
var isPostgres = accountDbContext.Database.IsNpgsql();
56+
var isPostgres = accountDbContext.Database.ProviderName is not "Microsoft.EntityFrameworkCore.Sqlite";
5757

5858
if (isPostgres)
5959
{

‎application/shared-kernel/Tests/Telemetry/ApplicationInsightsTelemetryInitializerTests.cs‎

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,75 @@ public void Initialize_WhenUserIsNotAuthenticated_ShouldNotSetSessionIdInTelemet
9090
telemetry.Context.GlobalProperties.Should().NotContainKey("user.theme");
9191
}
9292

93+
[Fact]
94+
public void Initialize_WhenTrackableFeatureFlagsEnabled_ShouldEmitScopedPerFlagProperties()
95+
{
96+
// Arrange - beta-features (Tenant) and experimental-ui (User) are the registry's two
97+
// TrackInTelemetry=true flags with different scopes. Symmetric with OpenTelemetryEnricherTests.
98+
var userInfo = new UserInfo
99+
{
100+
IsAuthenticated = true,
101+
Id = UserId.NewId(),
102+
TenantId = new TenantId(12345),
103+
Locale = "en-US",
104+
Role = "Admin",
105+
FeatureFlags = new HashSet<string> { "experimental-ui", "beta-features" }
106+
};
107+
108+
var executionContext = Substitute.For<IExecutionContext>();
109+
executionContext.UserInfo.Returns(userInfo);
110+
executionContext.TenantId.Returns(userInfo.TenantId);
111+
executionContext.ClientIpAddress.Returns(IPAddress.Parse("192.168.1.1"));
112+
113+
ApplicationInsightsTelemetryInitializer.SetContext(executionContext);
114+
115+
var telemetry = new RequestTelemetry();
116+
var initializer = new ApplicationInsightsTelemetryInitializer();
117+
118+
// Act
119+
initializer.Initialize(telemetry);
120+
121+
// Assert - per-flag dimensions scoped by who carries the setting, value is "enabled",
122+
// and the OTel-reserved feature_flag.* namespace is never emitted.
123+
telemetry.Context.GlobalProperties.Should().ContainKey("tenant.feature_flags.beta-features");
124+
telemetry.Context.GlobalProperties["tenant.feature_flags.beta-features"].Should().Be("enabled");
125+
telemetry.Context.GlobalProperties.Should().ContainKey("user.feature_flags.experimental-ui");
126+
telemetry.Context.GlobalProperties["user.feature_flags.experimental-ui"].Should().Be("enabled");
127+
telemetry.Context.GlobalProperties.Keys.Should().NotContain(k => k.StartsWith("feature_flag.", StringComparison.Ordinal));
128+
}
129+
130+
[Fact]
131+
public void Initialize_WhenNoTrackableFeatureFlagsEnabled_ShouldOmitFeatureFlagProperties()
132+
{
133+
// Arrange
134+
var userInfo = new UserInfo
135+
{
136+
IsAuthenticated = true,
137+
Id = UserId.NewId(),
138+
TenantId = new TenantId(12345),
139+
Locale = "en-US",
140+
Role = "Admin",
141+
FeatureFlags = new HashSet<string>()
142+
};
143+
144+
var executionContext = Substitute.For<IExecutionContext>();
145+
executionContext.UserInfo.Returns(userInfo);
146+
executionContext.TenantId.Returns(userInfo.TenantId);
147+
executionContext.ClientIpAddress.Returns(IPAddress.Parse("192.168.1.1"));
148+
149+
ApplicationInsightsTelemetryInitializer.SetContext(executionContext);
150+
151+
var telemetry = new RequestTelemetry();
152+
var initializer = new ApplicationInsightsTelemetryInitializer();
153+
154+
// Act
155+
initializer.Initialize(telemetry);
156+
157+
// Assert
158+
telemetry.Context.GlobalProperties.Keys.Should().NotContain(k => k.StartsWith("user.feature_flags.", StringComparison.Ordinal));
159+
telemetry.Context.GlobalProperties.Keys.Should().NotContain(k => k.StartsWith("tenant.feature_flags.", StringComparison.Ordinal));
160+
}
161+
93162
[Fact]
94163
public void Initialize_WhenSessionIdIsNull_ShouldNotSetSessionIdInTelemetry()
95164
{

0 commit comments

Comments
 (0)