Support for flat arrays in TelemetryLogger

[daesun-park]

Description

When using the telemetry logger, there are specific instances where logging a flat array or JSON object to an event field is necessary. To accommodate for these scenarios, this pull request focuses on making the following changes to the implementation of the TelemetryLogger class.

1. Accept a less-restrictive type than TelemetryEventPropertyType which includes arrays or objects where the values are strictly primitives (TelemetryEventPropertyType).
2. Stringify these flat objects/arrays when passing to another logger.

Reviewer Guidance

Questions

1. As mentioned by @markfields in the original internal task 1104, updating TelemetryEventPropertyType should maybe be avoided, as it would be quite a breaking change. Instead, I defined the following types/interfaces within the logger.ts file where our TelemetryLogger is implemented (instead of @fluidframework/common-definitions).

• TelemetryEventPropertyTypeExt
• ITelemetryBaseEventExt
• ITelemetryGenericEventExt
• ITelemetryPerformanceEventExt;

However, would it be best to instead change the update the existing types/interfaces defined in the @fluidframework/common-definitions instead?

2. As this pull request aims to provide support for flat arrays, which test cases should be updated/added to both the end-to-end and unit tests?

Link to Internal Task #1104

[msfluid-bot]

⯅ @fluid-example/bundle-size-tests: +2.79 KB

Metric Name	Baseline Size	Compare Size	Size Diff
aqueduct.js	387.57 KB	387.9 KB	⯅ +345 Bytes
connectionState.js	680 Bytes	680 Bytes	■ No change
containerRuntime.js	191.68 KB	192.02 KB	⯅ +345 Bytes
loader.js	151.53 KB	151.95 KB	⯅ +435 Bytes
map.js	42.67 KB	43.01 KB	⯅ +344 Bytes
matrix.js	127.15 KB	127.49 KB	⯅ +344 Bytes
odspDriver.js	149.41 KB	149.75 KB	⯅ +346 Bytes
odspPrefetchSnapshot.js	38.31 KB	38.65 KB	⯅ +352 Bytes
sharedString.js	147.79 KB	148.13 KB	⯅ +345 Bytes
Total Size	1.24 MB	1.24 MB	⯅ +2.79 KB

---

Baseline commit: 8422abd76f58a599f9d5e1bbfd64ec3dbd2a1dc8

Generated by :no_entry_sign: dangerJS against d1e714ad018b240e2aec3a771051832b858a51c0

[vladsud]

@daesun-park, is the description of the PR updated based on recent changes? Did I get it right that what you are trying to do is

• Allow new types on ChildLogger
• Do not change types for base logger.
• Address the conversion through TaggedLoggerAdapter

Is that right? That's reasonable directly, but as I pointed out earlier, it will require changes in most of our code. Search for ITelemetryProperties. Most of the usage in repo would need to be updated to ITelemetryPropertiesExt.

As a general direction, I think I like for type system we use internally to be different from type system used by base logger - it allows us to support more types (and convert them to strings or similar) without putting more requirements on the hosts. But I'd rather see it reflected in all of our repo (likely as two PRs).

While it's technically "breaking" change, I'd rather go with ITelemetryBaseProperties & ITelemetryProperties pair vs. ITelemetryProperties & ITelemetryPropertiesExt pair.

[ghost]

This PR has been automatically marked as stale because it has had no activity for 60 days. It will be closed if no further activity occurs within 8 days of this comment. Thank you for your contributions to Fluid Framework!