FluidFramework
FluidFramework copied to clipboard
logging null values in telemetry props
Description
This PR is a continuation of #9237.Currently, the getValidTelemetryProps() method handles logging in the following way:
// ensure only valid props get logged, since props of logging error could be in any shape
if (isTaggedTelemetryPropertyValue(val)) {
props[key] = {
value: filterValidTelemetryProps(val.value, key),
tag: val.tag,
};
} else {
props[key] = filterValidTelemetryProps(val, key);
}
And the filterValidTelemetryProps() method returns null values as "REDACTED (arbitrary object)".
function filterValidTelemetryProps(x: any, key: string): TelemetryEventPropertyType {
if (Array.isArray(x) && x.every((val) => isTelemetryEventPropertyValue(val))) {
return JSON.stringify(x);
}
if (isTelemetryEventPropertyValue(x)) {
return x;
}
// We don't support logging arbitrary objects
console.error(`UnSupported Format of Logging Error Property for key ${key}:`, x);
return "REDACTED (arbitrary object)";
}
This added change (shown below) is sufficient for handling the simple case for when value === null
, but will not support an array which contains a null value (i.e. [true, "string", null]
).
if (x === null) {
return String(x);
}
However, if we would like to support the logging of arrays containing null
values as well, we could also change the TelemetryEventPropertyType to support null
(which has been suggested by @vladsud, and may be introduced in #11337 ).
Reviewer Guidance
Questions
- The original pull request #9237 seems to want to address the
val === null
case, but it seems that support for flat arrays in errorLogging has been added since then. Is this sufficient, or should we make changes such that we log flat arrays containing null values as well (i.e.[true, "string", null]
)?