-
Notifications
You must be signed in to change notification settings - Fork 576
feat(telemetry-utils): deprecate LogLevel.default/error and tag unspecified events as essential #27207
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
feat(telemetry-utils): deprecate LogLevel.default/error and tag unspecified events as essential #27207
Changes from 10 commits
09cc182
3525674
f3c37bf
07925f4
67614c8
a568d12
9a81682
da948a1
c0d9396
f3a92b6
0ba1037
eab130c
5f1205d
ba39358
bad5774
71c1f07
2c13181
3b3f5fe
fbd560a
5d5fcb1
beeac4c
bee89e1
1131b62
1205a20
c033d8a
03adcc4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| --- | ||
| "@fluidframework/core-interfaces": minor | ||
| "__section": deprecation | ||
| --- | ||
|
|
||
| Deprecated LogLevel.default and LogLevel.error | ||
|
|
||
| `LogLevel.default` and `LogLevel.error` in `@fluidframework/core-interfaces` are deprecated in favor of the semantically clearer `LogLevel.info` and `LogLevel.essential` respectively. | ||
|
|
||
| #### Migration | ||
|
|
||
| - `LogLevel.default` (= `20`) → use `LogLevel.info` | ||
| - `LogLevel.error` (= `30`) → use `LogLevel.essential` | ||
|
|
||
| Removal is tracked in [issue #26969](https://github.com/microsoft/FluidFramework/issues/26969) and is planned for the v3.0 major release. | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,20 @@ | ||
| --- | ||
| "@fluidframework/telemetry-utils": minor | ||
| "__section": other | ||
| --- | ||
|
|
||
| ChildLogger now tags events with no explicit `logLevel` as `essential` | ||
|
|
||
| `@fluidframework/telemetry-utils` previously forwarded events with no explicit `logLevel` to the base logger as `undefined` (effectively treated as `LogLevel.info` = `20`). It now forwards them as `LogLevel.essential` (= `30`). Specifically: | ||
|
Check warning on line 8 in .changeset/soft-doors-trade.md
|
||
|
|
||
| - `ChildLogger.shouldFilterOutEvent` now falls back to `LogLevel.essential` when an event has no `logLevel` (previously fell back to `LogLevel.default` = `LogLevel.info` = `20`). | ||
| - `ChildLogger.send` now forwards `LogLevel.essential` to the base logger when no `logLevel` is supplied, instead of forwarding `undefined`. | ||
| - The implicit `= LogLevel.default` parameter defaults on `TelemetryLogger.sendTelemetryEvent` and `sendPerformanceEvent` were removed; callers that omit `logLevel` now propagate `undefined` through the chain, where `ChildLogger` applies the new `essential` fallback above. | ||
|
|
||
| #### Impact for downstream consumers | ||
|
|
||
| Internal Fluid events that previously arrived at the base logger with `logLevel = undefined` will now arrive tagged as `30` (`essential`). Hosts that filter telemetry by numeric level should not start dropping `logLevel = 20` events as "non-essential" unless **all** running Fluid versions include this change — older versions still send those same critical events at level `20`, and dropping them would lose telemetry. | ||
|
Check failure on line 16 in .changeset/soft-doors-trade.md
|
||
|
|
||
| #### Events already explicitly tagged as `LogLevel.info` | ||
|
|
||
| Note that this change only affects events emitted with **no** explicit `logLevel`. A set of events that are already explicitly tagged with `LogLevel.info` (= `20`) was introduced in [PR #27126](https://github.com/microsoft/FluidFramework/pull/27126) and is unaffected by this change — those events continue to be emitted at level `20` by design and are the intended target for hosts that want to drop non-essential telemetry once all running Fluid versions include both PR #27126 and this change. | ||
|
Check failure on line 20 in .changeset/soft-doors-trade.md
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -81,13 +81,15 @@ export interface LogLevelConst { | |
|
|
||
| /** | ||
| * Default LogLevel | ||
| * @remarks Prefer {@link LogLevelConst.info | LogLevel.info} when selecting a level explicitly since this will be deprecated and removed in a future release. | ||
| * @deprecated Prefer {@link LogLevelConst.info | LogLevel.info} when selecting a level explicitly to preserve prior treatment. Planned to be removed in 3.0.0. | ||
| * @see {@link https://github.com/microsoft/FluidFramework/issues/26969 | Issue #26969} for removal tracking. | ||
| */ | ||
| readonly default: 20; | ||
|
|
||
| /** | ||
| * To log errors. | ||
| * @remarks Prefer {@link LogLevelConst.essential | LogLevel.essential} when selecting a level since this will be deprecated and removed in a future release. | ||
| * @deprecated Prefer {@link LogLevelConst.essential | LogLevel.essential} when selecting a level. Planned to be removed in 3.0.0. | ||
| * @see {@link https://github.com/microsoft/FluidFramework/issues/26969 | Issue #26969} for removal tracking. | ||
| */ | ||
| readonly error: 30; | ||
| } | ||
|
|
@@ -120,13 +122,13 @@ export interface ITelemetryBaseLogger { | |
| /** | ||
| * Log a telemetry event, if it meets the appropriate log-level threshold (see {@link ITelemetryBaseLogger.minLogLevel}). | ||
| * @param event - The event to log. | ||
| * @param logLevel - The log level of the event. Default: {@link LogLevelConst.default | LogLevel.default}. | ||
| * @param logLevel - The log level of the event. Default: {@link LogLevelConst.info | LogLevel.info}. | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Isn't the default supposed to be essential? Maybe it's clearer if we say it like "suggested interpretation of undefined: LogLevel.essential"? Since this is an interface so claiming a "default value" is kind of in vain, it could be implemented any way.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Applied change |
||
| */ | ||
| send(event: ITelemetryBaseEvent, logLevel?: LogLevel): void; | ||
|
|
||
| /** | ||
| * Minimum log level to be logged. | ||
| * @defaultValue {@link LogLevelConst.default | LogLevel.default}. | ||
| * @defaultValue {@link LogLevelConst.info | LogLevel.info}. | ||
| */ | ||
| minLogLevel?: LogLevel; | ||
| } | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.