Skip to content

Commit 8d67bf6

Browse files
xiaomi7732xiaomi7732
authored
Dev/saars/fix stop overwriting sdk version (#381)
* Opt in sdk overwriting * Add unit tests
1 parent f9cc03a commit 8d67bf6

File tree

4 files changed

+102
-12
lines changed

4 files changed

+102
-12
lines changed

‎src/ApplicationInsights.Kubernetes/Extensions/AppInsightsForKubernetesOptions.cs‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,5 +44,14 @@ public class AppInsightsForKubernetesOptions
4444
/// When set to null (also the default), a built-in checker will be used.
4545
/// </summary>
4646
public IClusterEnvironmentCheck? ClusterCheckAction { get; set; } = null;
47+
48+
/// <summary>
49+
/// For backward compatibility reason to allow the user to opt-in to
50+
/// keep overwriting the SDK version in telemetry. It doesn't make too
51+
/// much sense to use it and has caused quite a confusion on the support
52+
/// side.
53+
/// Default to false and look into totally get rid of it in the future.
54+
/// </summary>
55+
public bool OverwriteSDKVersion { get; set; }
4756
}
4857
}

‎src/ApplicationInsights.Kubernetes/TelemetryInitializers/KubernetesTelemetryInitializer.cs‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@
55
using Microsoft.ApplicationInsights.Extensibility.Implementation;
66
using Microsoft.ApplicationInsights.Kubernetes.Debugging;
77
using Microsoft.ApplicationInsights.Kubernetes.Utilities;
8+
using Microsoft.Extensions.DependencyInjection;
9+
using Microsoft.Extensions.Options;
810
using static Microsoft.ApplicationInsights.Kubernetes.TelemetryProperty;
911

1012
namespace Microsoft.ApplicationInsights.Kubernetes;
@@ -16,15 +18,19 @@ internal class KubernetesTelemetryInitializer : ITelemetryInitializer
1618
{
1719
private static readonly ApplicationInsightsKubernetesDiagnosticSource _logger = ApplicationInsightsKubernetesDiagnosticSource.Instance;
1820
private readonly SDKVersionUtils _sdkVersionUtils;
21+
private readonly AppInsightsForKubernetesOptions _options;
22+
1923
internal IK8sEnvironmentHolder K8SEnvironmentHolder { get; }
2024
internal ITelemetryKeyCache TelemetryKeyCache { get; }
2125

2226
public KubernetesTelemetryInitializer(
2327
IK8sEnvironmentHolder k8sEnvironmentHolder,
2428
SDKVersionUtils sdkVersionUtils,
25-
ITelemetryKeyCache telemetryKeyCache)
29+
ITelemetryKeyCache telemetryKeyCache,
30+
IOptions<AppInsightsForKubernetesOptions> options)
2631
{
2732
TelemetryKeyCache = telemetryKeyCache ?? throw new ArgumentNullException(nameof(telemetryKeyCache));
33+
_options = options?.Value ?? throw new ArgumentNullException(nameof(options));
2834
K8SEnvironmentHolder = k8sEnvironmentHolder ?? throw new ArgumentNullException(nameof(k8sEnvironmentHolder));
2935
_sdkVersionUtils = sdkVersionUtils ?? throw new ArgumentNullException(nameof(sdkVersionUtils));
3036
}
@@ -52,7 +58,10 @@ public void Initialize(ITelemetry telemetry)
5258
_logger.LogTrace("Application Insights for Kubernetes telemetry initializer is used but the content is not ready yet.");
5359
}
5460

55-
telemetry.Context.GetInternalContext().SdkVersion = _sdkVersionUtils.CurrentSDKVersion;
61+
if (_options.OverwriteSDKVersion)
62+
{
63+
telemetry.Context.GetInternalContext().SdkVersion = _sdkVersionUtils.CurrentSDKVersion;
64+
}
5665
}
5766

5867
private void InitializeTelemetry(ITelemetry telemetry, IK8sEnvironment k8sEnv)

‎tests/UnitTests/AppInsightsKubernetesOptionsTests.cs‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,9 @@ public void ShouldHaveDefaultOptions()
3434

3535
// No telemetry key processor
3636
Assert.Null(options.TelemetryKeyProcessor);
37+
38+
// Do NOT overwrite SDK version by default
39+
Assert.False(options.OverwriteSDKVersion);
3740
}
3841
}
3942

‎tests/UnitTests/KubernetesTelemetryInitializerTests.cs‎

Lines changed: 79 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,11 @@
22
using System.Diagnostics;
33
using Microsoft.ApplicationInsights.Channel;
44
using Microsoft.ApplicationInsights.DataContracts;
5+
using Microsoft.ApplicationInsights.Extensibility.Implementation;
56
using Microsoft.ApplicationInsights.Kubernetes.Debugging;
67
using Microsoft.ApplicationInsights.Kubernetes.Utilities;
8+
using Microsoft.Extensions.DependencyInjection;
9+
using Microsoft.Extensions.Options;
710
using Moq;
811
using Xunit;
912

@@ -21,7 +24,8 @@ public void ConstructorSetsNullGetsNull()
2124
KubernetesTelemetryInitializer target = new KubernetesTelemetryInitializer(
2225
null,
2326
SDKVersionUtils.Instance,
24-
keyCacheMock.Object
27+
keyCacheMock.Object,
28+
Options.Create(new AppInsightsForKubernetesOptions())
2529
);
2630
});
2731

@@ -40,7 +44,8 @@ public void ConstructorSetK8sEnvironment()
4044
KubernetesTelemetryInitializer target = new KubernetesTelemetryInitializer(
4145
k8sEnvironmentHolderMock.Object,
4246
SDKVersionUtils.Instance,
43-
keyCacheMock.Object);
47+
keyCacheMock.Object,
48+
Options.Create(new AppInsightsForKubernetesOptions()));
4449

4550
Assert.Equal(k8sEnvironmentHolderMock.Object, target.K8SEnvironmentHolder);
4651
Assert.Equal(keyCacheMock.Object, target.TelemetryKeyCache);
@@ -59,7 +64,8 @@ public void InitializeSetsRoleName()
5964

6065
KubernetesTelemetryInitializer target = new KubernetesTelemetryInitializer(k8sEnvironmentHolderMock.Object,
6166
SDKVersionUtils.Instance,
62-
keyCacheMock.Object);
67+
keyCacheMock.Object,
68+
Options.Create(new AppInsightsForKubernetesOptions()));
6369
ITelemetry telemetry = new TraceTelemetry();
6470
target.Initialize(telemetry);
6571

@@ -80,7 +86,8 @@ public void InitializeShouldNotOverwriteExistingRoleName()
8086
KubernetesTelemetryInitializer target = new KubernetesTelemetryInitializer(
8187
k8sEnvironmentHolderMock.Object,
8288
SDKVersionUtils.Instance,
83-
keyCacheMock.Object);
89+
keyCacheMock.Object,
90+
Options.Create(new AppInsightsForKubernetesOptions()));
8491

8592
ITelemetry telemetry = new TraceTelemetry();
8693
telemetry.Context.Cloud.RoleName = "Existing RoleName";
@@ -122,7 +129,8 @@ public void InitializeWithEmptyForOptionalPropertyDoesNotLogError()
122129
KubernetesTelemetryInitializer target = new KubernetesTelemetryInitializer(
123130
k8sEnvironmentHolderMock.Object,
124131
SDKVersionUtils.Instance,
125-
keyCacheMock.Object);
132+
keyCacheMock.Object,
133+
Options.Create(new AppInsightsForKubernetesOptions()));
126134
ITelemetry telemetry = new TraceTelemetry();
127135
target.Initialize(telemetry);
128136

@@ -162,7 +170,8 @@ public void InitializeWithEmptyForRequiredPropertyDoesLogError()
162170
KubernetesTelemetryInitializer target = new KubernetesTelemetryInitializer(
163171
k8sEnvironmentHolderMock.Object,
164172
SDKVersionUtils.Instance,
165-
keyCacheMock.Object);
173+
keyCacheMock.Object,
174+
Options.Create(new AppInsightsForKubernetesOptions()));
166175
ITelemetry telemetry = new TraceTelemetry();
167176
target.Initialize(telemetry);
168177

@@ -198,7 +207,8 @@ public void InitializeSetsCustomDimensions()
198207
KubernetesTelemetryInitializer target = new KubernetesTelemetryInitializer(
199208
k8sEnvironmentHolderMock.Object,
200209
SDKVersionUtils.Instance,
201-
keyCacheMock.Object);
210+
keyCacheMock.Object,
211+
Options.Create(new AppInsightsForKubernetesOptions()));
202212
ITelemetry telemetry = new TraceTelemetry();
203213
target.Initialize(telemetry);
204214

@@ -237,7 +247,8 @@ public void InitializeWillNotOverwriteExistingCustomDimension()
237247
KubernetesTelemetryInitializer target = new KubernetesTelemetryInitializer(
238248
k8sEnvironmentHolderMock.Object,
239249
SDKVersionUtils.Instance,
240-
keyCacheMock.Object);
250+
keyCacheMock.Object,
251+
Options.Create(new AppInsightsForKubernetesOptions()));
241252
ITelemetry telemetry = new TraceTelemetry();
242253
ISupportProperties telemetryWithProperties = telemetry as ISupportProperties;
243254
telemetryWithProperties.Properties["K8s.Container.ID"] = "Existing Cid";
@@ -257,7 +268,8 @@ public void TimeoutGettingK8sEnvNoException()
257268
KubernetesTelemetryInitializer target = new KubernetesTelemetryInitializer(
258269
k8sEnvironmentHolderMock.Object, // k8sEnvironmentHolderMock.Object.K8sEnvironment is null
259270
SDKVersionUtils.Instance,
260-
keyCacheMock.Object);
271+
keyCacheMock.Object,
272+
Options.Create(new AppInsightsForKubernetesOptions()));
261273

262274
ITelemetry telemetry = new TraceTelemetry();
263275
ISupportProperties telemetryWithProperties = telemetry as ISupportProperties;
@@ -284,7 +296,8 @@ public void ShouldUseTheValueByTheKeyProcessorForTelemetry()
284296
KubernetesTelemetryInitializer target = new KubernetesTelemetryInitializer(
285297
k8sEnvironmentHolderMock.Object,
286298
SDKVersionUtils.Instance,
287-
keyCacheMock.Object);
299+
keyCacheMock.Object,
300+
Options.Create(new AppInsightsForKubernetesOptions()));
288301
ITelemetry telemetry = new TraceTelemetry();
289302
target.Initialize(telemetry);
290303

@@ -293,4 +306,60 @@ public void ShouldUseTheValueByTheKeyProcessorForTelemetry()
293306
Assert.False(telemetryWithProperties.Properties.ContainsKey("Kubernetes.Container.ID"));
294307
Assert.True(telemetryWithProperties.Properties.ContainsKey("Kubernetes_Container_ID"));
295308
}
309+
310+
[Fact(DisplayName = "K8sTelemetryInitializer will not overwrite sdk version.")]
311+
public void ShouldNotOverwriteInternalSDK()
312+
{
313+
Mock<IK8sEnvironment> envMock = new();
314+
Mock<IK8sEnvironmentHolder> k8sEnvironmentHolderMock = new();
315+
Mock<ITelemetryKeyCache> keyCacheMock = new Mock<ITelemetryKeyCache>();
316+
317+
envMock.Setup(env => env.ContainerName).Returns("Hello.RoleName");
318+
envMock.Setup(env => env.ContainerID).Returns("Hello.Cid");
319+
320+
keyCacheMock.Setup(c => c.GetProcessedKey(It.IsAny<string>())).Returns<string>(input => input.Replace('.', '_'));
321+
k8sEnvironmentHolderMock.Setup(h => h.K8sEnvironment).Returns(envMock.Object);
322+
323+
// By default the SDK version should not be overwritten.
324+
KubernetesTelemetryInitializer target = new KubernetesTelemetryInitializer(
325+
k8sEnvironmentHolderMock.Object,
326+
SDKVersionUtils.Instance,
327+
keyCacheMock.Object,
328+
Options.Create(new AppInsightsForKubernetesOptions()));
329+
ITelemetry telemetry = new TraceTelemetry();
330+
331+
string originalSdkVersion = telemetry.Context.GetInternalContext().SdkVersion;
332+
target.Initialize(telemetry);
333+
334+
Assert.Equal(originalSdkVersion, telemetry.Context.GetInternalContext().SdkVersion);
335+
Assert.NotEqual(SDKVersionUtils.Instance.CurrentSDKVersion, telemetry.Context.GetInternalContext().SdkVersion);
336+
}
337+
338+
[Fact(DisplayName = "K8sTelemetryInitializer will overwrite sdk version by options.")]
339+
public void ShouldOverwriteInternalSDKByOptions()
340+
{
341+
Mock<IK8sEnvironment> envMock = new();
342+
Mock<IK8sEnvironmentHolder> k8sEnvironmentHolderMock = new();
343+
Mock<ITelemetryKeyCache> keyCacheMock = new Mock<ITelemetryKeyCache>();
344+
345+
envMock.Setup(env => env.ContainerName).Returns("Hello.RoleName");
346+
envMock.Setup(env => env.ContainerID).Returns("Hello.Cid");
347+
348+
keyCacheMock.Setup(c => c.GetProcessedKey(It.IsAny<string>())).Returns<string>(input => input.Replace('.', '_'));
349+
k8sEnvironmentHolderMock.Setup(h => h.K8sEnvironment).Returns(envMock.Object);
350+
351+
// By default the SDK version should not be overwritten.
352+
KubernetesTelemetryInitializer target = new KubernetesTelemetryInitializer(
353+
k8sEnvironmentHolderMock.Object,
354+
SDKVersionUtils.Instance,
355+
keyCacheMock.Object,
356+
Options.Create(new AppInsightsForKubernetesOptions() { OverwriteSDKVersion = true }));
357+
ITelemetry telemetry = new TraceTelemetry();
358+
359+
string originalSdkVersion = telemetry.Context.GetInternalContext().SdkVersion;
360+
target.Initialize(telemetry);
361+
362+
Assert.NotEqual(originalSdkVersion, telemetry.Context.GetInternalContext().SdkVersion);
363+
Assert.Equal(SDKVersionUtils.Instance.CurrentSDKVersion, telemetry.Context.GetInternalContext().SdkVersion);
364+
}
296365
}

0 commit comments

Comments
 (0)