Skip to content

Remove the TaggedLoggerAdapter compatibility fallback - #28144

Open
Mark Fields (markfields) wants to merge 4 commits into
microsoft:mainfrom
markfields:agents/remove-taggedloggeradapter-compat-check
Open

Remove the TaggedLoggerAdapter compatibility fallback#28144
Mark Fields (markfields) wants to merge 4 commits into
microsoft:mainfrom
markfields:agents/remove-taggedloggeradapter-compat-check

Conversation

@markfields

Copy link
Copy Markdown
Member

Description

Removes the deprecated internal TaggedLoggerAdapter and the container runtime fallback that adapted untagged loggers from loader 0.45-era container contexts.

Supported loaders provide IContainerContext.taggedLogger. The historical fallback is outside the current 12-month Runtime-to-Loader layer compatibility window, and @internal APIs carry no stability guarantee. The layer compatibility and telemetry utilities design documentation now clarify these expectations.

No customer-facing API reports changed, so this does not require a changeset or API Council review.

Validation:

  • CI readiness checks passed for @fluidframework/container-runtime and @fluidframework/telemetry-utils
  • ESLint passed
  • API report and documentation generation passed
  • @fluidframework/telemetry-utils: 256 tests passed
  • @fluidframework/container-runtime: 975 tests passed, 1 pending

Reviewer Guidance

The review process is outlined on this wiki page.

Please verify that the current layer compatibility window is the correct basis for removing the loader 0.45 fallback.

Loader 0.45 is outside the current Runtime-to-Loader compatibility window, so remove its untagged logger adapter and document the rolling policy that supersedes the historical fixed LTS.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI lite review requested due to automatic review settings September 2, 2026 16:46
@github-actions github-actions Bot added area: tools area: runtime Runtime related issues area: repo Repo related work area: website base: main PRs targeted against main branch labels Sep 2, 2026
@github-actions

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Hi! Thank you for opening this PR. Want me to review it?

Based on the diff (209 lines, 6 files), I've queued these reviewers:

  • Correctness — logic errors, race conditions, lifecycle issues
  • Security — vulnerabilities, secret exposure, injection
  • API Compatibility — breaking changes, release tags, type design
  • Performance — algorithmic regressions, memory leaks
  • Testing — coverage gaps, hollow tests

How this works

  • Adjust the reviewer set by ticking/unticking boxes above. Reviewer toggles alone don't trigger anything.

  • Tick Start review below to dispatch the review fleet.

  • After review finishes, tick Start review again to request another run — it auto-resets after each dispatch.

  • This comment updates as new commits land; your reviewer selections are preserved.

  • Start review

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The deprecated adapter and runtime fallback are cleanly removed with corresponding test/doc updates and no remaining in-repo references.

Pull request overview

This PR removes the deprecated internal TaggedLoggerAdapter and the container runtime’s legacy fallback that adapted untagged loggers from older (loader 0.45-era) container contexts, aligning runtime↔loader expectations with the documented rolling layer compatibility windows.

Changes:

  • Deleted TaggedLoggerAdapter from @fluidframework/telemetry-utils and removed its internal re-export.
  • Removed ContainerRuntime fallback logic that wrapped context.logger when context.taggedLogger was absent, now requiring context.taggedLogger.
  • Updated documentation to clarify that tagged logger adaptation is not provided outside the supported layer compatibility window.
File summaries
File Description
packages/utils/telemetry-utils/src/test/errorLogging.spec.ts Removes tests that exercised the deleted TaggedLoggerAdapter.
packages/utils/telemetry-utils/src/logger.ts Deletes the deprecated internal TaggedLoggerAdapter implementation.
packages/utils/telemetry-utils/src/internal.ts Stops re-exporting TaggedLoggerAdapter via the internal entrypoint.
packages/utils/telemetry-utils/DEV.md Documents that tagged logger adaptation is not provided outside the layer compatibility window.
packages/runtime/container-runtime/src/containerRuntime.ts Removes old-loader fallback and uses context.taggedLogger directly.
LayerCompatibility.md Clarifies that rolling support windows supersede the older fixed-loader-LTS compatibility policy.
Review details
  • Files reviewed: 6/6 changed files
  • Comments generated: 0
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Treat contexts missing both tagged logger support and compatibility details as legacy loaders even while generation zero is inside the calculated window. Preserve the explicit strict-check bypass and verify that legacy untagged loggers never receive runtime telemetry.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Keep the generic layer compatibility validation unchanged. Reject legacy loaders explicitly before runtime telemetry is initialized, regardless of the strict compatibility setting.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@github-actions

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Fleet Review — Clean

No issues found across the reviewer fleet for this run.

View run

@github-actions

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

🔗 No broken links found! ✅

Your attention to detail is admirable.

linkcheck output

$ start-server-and-test "npm run serve -- --host 127.0.0.1 --no-open" http://127.0.0.1:3000 check-links
1: starting server using command "npm run serve -- --host 127.0.0.1 --no-open"
and when url "[ 'http://127.0.0.1:3000' ]" is responding with HTTP status code 200
running tests using command "npm run check-links"


> fluid-framework-website@0.0.0 serve
> docusaurus serve --host 127.0.0.1 --no-open

[SUCCESS] Serving "build" directory at: http://127.0.0.1:3000/

> fluid-framework-website@0.0.0 check-links
> linkcheck http://127.0.0.1:3000 --skip-file skipped-urls.txt

Crawling...

Stats:
  340615 links
    2042 destination URLs
    2297 URLs ignored
       0 warnings
       0 errors


@github-actions

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Bundle size comparison

Base commit: f57cde7412798b35161e3c2d068ce35ab26483fd
Head commit: 0d51517c2bdfb81f866ffc3c139ed65b4909eb45

Notable changes

No bundles changed by ≥ 500 bytes parsed.

Per-bundle deltas

@fluid-example/bundle-size-tests

  • fluidFrameworkAllAlpha.js: parsed 804388 → 804444 (+56), gzip 221073 → 221132 (+59)
  • azureClient.js: parsed 634027 → 633685 (-342), gzip 169868 → 169831 (-37)
  • odspClient.js: parsed 605273 → 605047 (-226), gzip 162668 → 162701 (+33)
  • aqueduct.js: parsed 537952 → 537626 (-326), gzip 144493 → 144421 (-72)
  • fluidFramework.js: parsed 413402 → 413435 (+33), gzip 117287 → 117287 (0)
  • sharedTree.js: parsed 402781 → 402807 (+26), gzip 114725 → 114731 (+6)
  • containerRuntime.js: parsed 314719 → 314365 (-354), gzip 86326 → 86211 (-115)
  • sharedString.js: parsed 175239 → 175246 (+7), gzip 49670 → 49677 (+7)
  • experimentalSharedTree.js: parsed 161846 → 161846 (0), gzip 46722 → 46722 (0)
  • matrix.js: parsed 159618 → 159625 (+7), gzip 45914 → 45921 (+7)
  • loader.js: parsed 147327 → 147343 (+16), gzip 40038 → 40048 (+10)
  • odspDriver.js: parsed 105689 → 105747 (+58), gzip 32933 → 33000 (+67)
  • directory.js: parsed 65669 → 65676 (+7), gzip 18493 → 18501 (+8)
  • 578.js: parsed 58686 → 58686 (0), gzip 17657 → 17657 (0)
  • odspPrefetchSnapshot.js: parsed 45921 → 45902 (-19), gzip 15347 → 15357 (+10)
  • map.js: parsed 45820 → 45827 (+7), gzip 14119 → 14126 (+7)
  • 252.js: parsed 44384 → 44384 (0), gzip 13741 → 13741 (0)
  • summarizerDelayLoadedModule.js: parsed 31287 → 31287 (0), gzip 7929 → 7929 (0)
  • socketModule.js: parsed 26992 → 26962 (-30), gzip 8019 → 8052 (+33)
  • createNewModule.js: parsed 12464 → 12464 (0), gzip 4792 → 4805 (+13)
  • summaryModule.js: parsed 3888 → 3888 (0), gzip 1874 → 1874 (0)
  • connectionState.js: parsed 909 → 909 (0), gzip 500 → 500 (0)
  • sharedTreeAttributes.js: parsed 845 → 852 (+7), gzip 493 → 502 (+9)
  • debugAssert.js: parsed 429 → 429 (0), gzip 299 → 299 (0)
  • FluidFramework-HashFallback.js: parsed 419 → 419 (0), gzip 313 → 313 (0)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: repo Repo related work area: runtime Runtime related issues area: tools area: website base: main PRs targeted against main branch

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants