Skip to content

Commit a598f47

Browse files
MSNevMSNev
authored
Address issue with the AppInsightsExtCore using the wrong version number (#2718)
1 parent 847c292 commit a598f47

7 files changed

Lines changed: 136 additions & 41 deletions

File tree

‎common/config/rush/npm-shrinkwrap.json‎

Lines changed: 0 additions & 13 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎gruntfile.js‎

Lines changed: 50 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
1+
const { default: replace } = require("@rollup/plugin-replace");
2+
13
module.exports = function (grunt) {
24

35
const versionPlaceholder = '"#version#"';
6+
const extVersionPlaceholder = '"#extVersion#"';
47

58
const aiCoreDefaultNameReplacements = [
69
];
@@ -158,10 +161,19 @@ module.exports = function (grunt) {
158161
return new RegExp(str.replace(/([.+?^=!:${}()|\[\]\/\\])/g, '\\$1'), 'g');
159162
}
160163

161-
function setVersionNumber(path, packageVersion) {
162-
var expectedVersion = _createRegEx(versionPlaceholder);
163-
var replaceVersion = "'" + packageVersion + "'";
164+
function setVersionNumber(path, packageVersion, extPackageVersion) {
164165
var srcPath = path + '/src';
166+
var replacements = [{
167+
pattern: _createRegEx(versionPlaceholder),
168+
replacement: "'" + packageVersion + "'"
169+
}];
170+
171+
if (extPackageVersion) {
172+
replacements.push({
173+
pattern: _createRegEx(extVersionPlaceholder),
174+
replacement: "'" + extPackageVersion + "'"
175+
});
176+
}
165177

166178
// This is the grunt string-replace configuration to replace version placeholder with the actual version number
167179
return {
@@ -172,18 +184,31 @@ module.exports = function (grunt) {
172184
src: '**/*.ts'
173185
}],
174186
options: {
175-
replacements: [{
176-
pattern: expectedVersion,
177-
replacement: replaceVersion
178-
}]
187+
replacements: replacements
179188
}
180189
};
181190
}
182191

183-
function restoreVersionPlaceholder(path, packageVersion) {
184-
var expectedVersion1 = _createRegEx("'" + packageVersion + "'");
185-
var expectedVersion2 = _createRegEx('"' + packageVersion + '"');
192+
function restoreVersionPlaceholder(path, packageVersion, extPackageVersion) {
186193
var srcPath = path + '/src';
194+
var replacements = [{
195+
pattern: _createRegEx("'" + packageVersion + "'"),
196+
replacement: versionPlaceholder
197+
},{
198+
pattern: _createRegEx('"' + packageVersion + '"'),
199+
replacement: versionPlaceholder
200+
}];
201+
202+
if (extPackageVersion && extPackageVersion !== packageVersion) {
203+
replacements.push({
204+
pattern: _createRegEx("'" + extPackageVersion + "'"),
205+
replacement: extVersionPlaceholder
206+
});
207+
replacements.push({
208+
pattern: _createRegEx('"' + extPackageVersion + '"'),
209+
replacement: extVersionPlaceholder
210+
});
211+
}
187212

188213
// This is the grunt string-replace configuration to replace the actual version number with the version placeholder
189214
return {
@@ -194,13 +219,7 @@ module.exports = function (grunt) {
194219
src: '**/*.ts'
195220
}],
196221
options: {
197-
replacements: [{
198-
pattern: expectedVersion1,
199-
replacement: versionPlaceholder
200-
},{
201-
pattern: expectedVersion2,
202-
replacement: versionPlaceholder
203-
}]
222+
replacements: replacements
204223
}
205224
};
206225
}
@@ -321,9 +340,21 @@ module.exports = function (grunt) {
321340
var replaceCmds = buildCmds['string-replace'];
322341
// Read the actual module version from the package.json
323342
var packageVersion = pkg['version'];
343+
var extPackageVersion = packageVersion;
344+
345+
if (key !== "1dsPost" && key !== "1dsCore") {
346+
// Support for #extVersion# placeholder (old 1ds core version with major version incremented by 1)
347+
var idx = packageVersion.indexOf(".");
348+
if (idx !== -1) {
349+
var majorVersion = parseInt(packageVersion.substring(0, idx));
350+
if (!isNaN(majorVersion)) {
351+
extPackageVersion = (majorVersion + 1) + packageVersion.substring(idx);
352+
}
353+
}
354+
}
324355

325-
replaceCmds[key] = setVersionNumber(modulePath, packageVersion);
326-
replaceCmds[key + '-reverse'] = restoreVersionPlaceholder(modulePath, packageVersion);
356+
replaceCmds[key] = setVersionNumber(modulePath, packageVersion, extPackageVersion);
357+
replaceCmds[key + '-reverse'] = restoreVersionPlaceholder(modulePath, packageVersion, extPackageVersion);
327358
}
328359
}
329360

‎shared/AppInsightsCore/Tests/Unit/src/ai/ApplicationInsightsCore.Tests.ts‎

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -457,6 +457,61 @@ export class ApplicationInsightsCoreTests extends AITestClass {
457457
}
458458
});
459459

460+
this.testCase({
461+
name: 'ApplicationInsightsCore: track does not set ext.sdk.ver',
462+
useFakeTimers: true,
463+
test: () => {
464+
const channelPlugin = new ChannelPlugin();
465+
const appInsightsCore = new AppInsightsCore();
466+
appInsightsCore.initialize({ instrumentationKey: "00000000-1111-2222-817F-544738CC7C41" }, [channelPlugin]);
467+
468+
// Act
469+
const bareItem: ITelemetryItem = { name: 'test item' };
470+
appInsightsCore.track(bareItem);
471+
this.clock.tick(1);
472+
473+
// Test - AppInsightsCore should NOT set ext.sdk.ver (only AppInsightsExtCore does)
474+
Assert.equal(undefined, bareItem.ext, "ext should not be set by AppInsightsCore");
475+
}
476+
});
477+
478+
this.testCase({
479+
name: 'ApplicationInsightsCore: track does not set ext.sdk.ver when ext is pre-populated',
480+
useFakeTimers: true,
481+
test: () => {
482+
const channelPlugin = new ChannelPlugin();
483+
const appInsightsCore = new AppInsightsCore();
484+
appInsightsCore.initialize({ instrumentationKey: "00000000-1111-2222-817F-544738CC7C41" }, [channelPlugin]);
485+
486+
// Act - provide ext but no sdk.ver
487+
const bareItem: ITelemetryItem = { name: 'test item', ext: { "custom": { "field": "value" } } };
488+
appInsightsCore.track(bareItem);
489+
this.clock.tick(1);
490+
491+
// Test - AppInsightsCore should NOT add sdk.ver to ext
492+
Assert.ok(bareItem.ext, "ext should still exist");
493+
Assert.equal(undefined, bareItem.ext["sdk"], "sdk should not be added by AppInsightsCore");
494+
}
495+
});
496+
497+
this.testCase({
498+
name: 'ApplicationInsightsCore: track does not overwrite ext.sdk.ver if already set',
499+
useFakeTimers: true,
500+
test: () => {
501+
const channelPlugin = new ChannelPlugin();
502+
const appInsightsCore = new AppInsightsCore();
503+
appInsightsCore.initialize({ instrumentationKey: "00000000-1111-2222-817F-544738CC7C41" }, [channelPlugin]);
504+
505+
// Act - provide ext.sdk.ver explicitly
506+
const bareItem: ITelemetryItem = { name: 'test item', ext: { "sdk": { "ver": "custom-version" } } };
507+
appInsightsCore.track(bareItem);
508+
this.clock.tick(1);
509+
510+
// Test - AppInsightsCore should not touch the existing ext.sdk.ver
511+
Assert.equal("custom-version", bareItem.ext["sdk"]["ver"], "ext.sdk.ver should remain unchanged");
512+
}
513+
});
514+
460515
this.testCase({
461516
name: "DiagnosticLogger: Critical logging history is saved",
462517
test: () => {

‎shared/AppInsightsCore/Tests/Unit/src/ext/CoreTest.ts‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { AITestClass } from "@microsoft/ai-test-framework";
2-
import { AppInsightsCore as AIInternalCore, AppInsightsExtCore, EventLatency, FullVersionString, IChannelControls, IExtendedConfiguration, IExtendedTelemetryItem, IPropertyStorageOverride, blockDynamicConversion } from '../../../../src/index';
2+
import { AppInsightsCore as AIInternalCore, AppInsightsExtCore, EventLatency, ExtFullVersionString, FullVersionString, IChannelControls, IExtendedConfiguration, IExtendedTelemetryItem, IPropertyStorageOverride, blockDynamicConversion } from '../../../../src/index';
33
import dynamicProto from "@microsoft/dynamicproto-js";
44

55
export class CoreTest extends AITestClass {
@@ -148,7 +148,8 @@ export class CoreTest extends AITestClass {
148148
QUnit.assert.equal(actualEvent.name, "testEvent");
149149
QUnit.assert.equal(actualEvent.latency, EventLatency.Normal);
150150
QUnit.assert.ok(actualEvent.ext['sdk']['ver'].indexOf('1DS-Web-JS') > -1);
151-
QUnit.assert.equal(actualEvent.ext['sdk']['ver'], FullVersionString, actualEvent.ext['sdk']['ver']);
151+
QUnit.assert.equal(actualEvent.ext['sdk']['ver'], ExtFullVersionString, actualEvent.ext['sdk']['ver']);
152+
QUnit.assert.equal(actualEvent.ext['sdk']['ver'], FullVersionString, "FullVersionString should match ExtFullVersionString");
152153
QUnit.assert.equal(isNaN(actualEvent.timings.trackStart as number), false);
153154
QUnit.assert.equal(actualEvent.baseData['properties']['version'], '', actualEvent.baseData['properties']['version']);
154155
}
@@ -179,7 +180,8 @@ export class CoreTest extends AITestClass {
179180
var actualEvent: IExtendedTelemetryItem = coreTrackSpy.getCall(0).args[0];
180181
QUnit.assert.equal(actualEvent.name, "testEvent");
181182
QUnit.assert.ok(actualEvent.ext['sdk']['ver'].indexOf('1DS-Web-JS') > -1);
182-
QUnit.assert.equal(actualEvent.ext['sdk']['ver'], FullVersionString, actualEvent.ext['sdk']['ver']);
183+
QUnit.assert.equal(actualEvent.ext['sdk']['ver'], ExtFullVersionString, actualEvent.ext['sdk']['ver']);
184+
QUnit.assert.equal(actualEvent.ext['sdk']['ver'], FullVersionString, "FullVersionString should match ExtFullVersionString");
183185
QUnit.assert.equal(actualEvent.baseData['properties']['version'], 'version1', actualEvent.baseData['properties']['version']);
184186
}
185187
});
@@ -206,7 +208,8 @@ export class CoreTest extends AITestClass {
206208
var actualEvent: IExtendedTelemetryItem = coreTrackSpy.getCall(0).args[0];
207209
QUnit.assert.equal(actualEvent.name, "testEvent");
208210
QUnit.assert.ok(actualEvent.ext['sdk']['ver'].indexOf('1DS-Web-JS') > -1);
209-
QUnit.assert.equal(actualEvent.ext['sdk']['ver'], FullVersionString, actualEvent.ext['sdk']['ver']);
211+
QUnit.assert.equal(actualEvent.ext['sdk']['ver'], ExtFullVersionString, actualEvent.ext['sdk']['ver']);
212+
QUnit.assert.equal(actualEvent.ext['sdk']['ver'], FullVersionString, "FullVersionString should match ExtFullVersionString");
210213
QUnit.assert.equal(actualEvent.baseData['properties']['version'], 'testChannel=channelVer', actualEvent.baseData['properties']['version']);
211214
}
212215
});

‎shared/AppInsightsCore/src/ext/AppInsightsExtCore.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ import { ITelemetryItem } from "../interfaces/ai/ITelemetryItem";
1818
import { IPlugin } from "../interfaces/ai/ITelemetryPlugin";
1919
import { IConfigDefaults } from "../interfaces/config/IConfigDefaults";
2020
import { IExtendedConfiguration, IExtendedTelemetryItem, IPropertyStorageOverride } from "../interfaces/ext/DataModels";
21-
import { FullVersionString, getTime, isLatency } from "./extUtils";
21+
import { ExtFullVersionString, getTime, isLatency } from "./extUtils";
2222

2323
/**
2424
* The default settings for the config.
@@ -81,7 +81,7 @@ export class AppInsightsExtCore<C extends IExtendedConfiguration = IExtendedConf
8181

8282
let itemExt = telemetryItem.ext = telemetryItem.ext || {};
8383
itemExt.sdk = itemExt.sdk || {};
84-
itemExt.sdk.ver = FullVersionString;
84+
itemExt.sdk.ver = ExtFullVersionString;
8585
let baseData = telemetryItem.baseData = telemetryItem.baseData || {};
8686
baseData.properties = baseData.properties || {};
8787

‎shared/AppInsightsCore/src/ext/extUtils.ts‎

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,27 @@ import { IEventProperty, IExtendedTelemetryItem } from "../interfaces/ext/DataMo
1616
import { newGuid } from "../utils/CoreUtils";
1717
import { isReactNative } from "../utils/EnvUtils";
1818

19-
export const Version = "#version#";
20-
export const FullVersionString = "1DS-Web-JS-" + Version;
19+
/**
20+
* Identifies the version for the extended SDK
21+
*/
22+
export const ExtVersion = "#extVersion#";
23+
24+
/**
25+
* Identifies the full version for the extended SDK
26+
*/
27+
export const ExtFullVersionString = "1DS-Web-JS-" + ExtVersion;
28+
29+
/**
30+
* Identifies the version for the extended SDK (legacy constant)
31+
* @deprecated Use {@link ExtVersion} instead
32+
*/
33+
export const Version = ExtVersion;
34+
35+
/**
36+
* Identifies the full version for the extended SDK (legacy constant)
37+
* @deprecated Use {@link ExtFullVersionString} instead
38+
*/
39+
export const FullVersionString = ExtFullVersionString;
2140

2241
const ObjHasOwnProperty = Object.prototype.hasOwnProperty;
2342

‎shared/AppInsightsCore/src/index.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -401,7 +401,7 @@ export { ValueSanitizer } from "./ext/ValueSanitizer";
401401
// 1DS Utils
402402
export {
403403
isValueAssigned, isLatency, isUint8ArrayAvailable, getTenantId, sanitizeProperty,
404-
Version, FullVersionString, getCommonSchemaMetaData, getCookieValue,
404+
ExtVersion, Version, ExtFullVersionString, FullVersionString, getCommonSchemaMetaData, getCookieValue,
405405
extend, createGuid, isDocumentObjectAvailable, isWindowObjectAvailable,
406406
setProcessTelemetryTimings, getTime,
407407
isArrayValid, isValueKind, getFieldValueType,

0 commit comments

Comments
 (0)