-
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
Open
MarioJGMsoft
wants to merge
28
commits into
microsoft:main
Choose a base branch
from
MarioJGMsoft:marioja/deprecateLogLevel
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
feat(telemetry-utils): deprecate LogLevel.default/error and tag unspecified events as essential #27207
Changes from 16 commits
Commits
Show all changes
28 commits
Select commit
Hold shift + click to select a range
09cc182
feat: deprecate LogLevel.default and .error and remove all calls to t…
MarioJGMsoft 3525674
feat: send LogLevel as undefined and default to essential on send logic
MarioJGMsoft f3c37bf
Merge branch 'main' into marioja/deprecateLogLevel
MarioJGMsoft 07925f4
Merge branch 'microsoft:main' into marioja/deprecateLogLevel
MarioJGMsoft 67614c8
refactor: properly ordered imports and removed LogLevel.default
MarioJGMsoft a568d12
docs: fixed comments
MarioJGMsoft 9a81682
docs: updated docs
MarioJGMsoft da948a1
docs: added changesets
MarioJGMsoft c0d9396
docs: added removal timeline
MarioJGMsoft f3a92b6
docs: updated deprecation message
MarioJGMsoft 0ba1037
docs: addressed docs comments
MarioJGMsoft eab130c
Merge branch 'microsoft:main' into marioja/deprecateLogLevel
MarioJGMsoft 5f1205d
docs: updated chengeset title
MarioJGMsoft ba39358
feat: applied changes to docs and code based on comments
MarioJGMsoft bad5774
feat: made logLevel required in various levels and added tests
MarioJGMsoft 71c1f07
fix: removed type import from normal import
MarioJGMsoft 2c13181
fix: applied changes based on review
MarioJGMsoft 3b3f5fe
docs: updated docs
MarioJGMsoft fbd560a
docs: removed changeset
MarioJGMsoft 5d5fcb1
docs: updated performance docs
MarioJGMsoft beeac4c
Merge branch 'main' into marioja/deprecateLogLevel
MarioJGMsoft bee89e1
chore: untrack dev-container bind-mounted files accidentally added
MarioJGMsoft 1131b62
docs: updated docs message
MarioJGMsoft 1205a20
feat: applied changes based on comments
MarioJGMsoft c033d8a
Merge branch 'main' into marioja/deprecateLogLevel
MarioJGMsoft 03adcc4
feat: updated guidance and returned childLogger--Sink tests
MarioJGMsoft 389760d
Merge branch 'main' into marioja/deprecateLogLevel
MarioJGMsoft ece6302
docs: updated docs
MarioJGMsoft File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| 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` | ||
|
jason-ha marked this conversation as resolved.
Outdated
|
||
| - `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. | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,20 @@ | ||
| --- | ||
| "@fluidframework/telemetry-utils": minor | ||
| "__section": other | ||
| --- | ||
|
|
||
| All internal implementations of ITelemetryLoggerExt now tag events with no explicit logLevel as essential | ||
|
Check failure on line 6 in .changeset/soft-doors-trade.md
|
||
|
|
||
| `@fluidframework/telemetry-utils` previously forwarded events with no explicit `logLevel` to the base logger as `undefined` (treated as `LogLevel.info` = `20`). It now forwards them as `LogLevel.essential` (= `30`). Specifically: | ||
|
|
||
| - `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. | ||
|
|
||
| #### 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 warning on line 20 in .changeset/soft-doors-trade.md
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.