feat(datastore): Decorate and log errors thrown during DataStore entrypoint initialization by markfields · Pull Request #27327 · microsoft/FluidFramework

markfields · 2026-05-16T21:11:44Z

Description

Component load failures are one of the top scenarios our partners need to be able to monitor and diagnose in production. This PR closes a long-standing gap where errors thrown during entryPoint initialization (e.g. for PureDataObject, that includes the initalization hooks) are not logged to FF tables.

While host applications can and do log these errors, there is context that's only readily available within FF that is really important for investigation - All the common ContainerRuntime props (notably clientType - is it the Summarizer?), as well as the DataStore's packagePath (indicates the component type) and id.

This PR logs an error event with all these props, and additionally annotates the error object* with the dataStore package/id info.

* Note: In testing I found that these telemetry props get stripped off on the way up by roundtripping through exceptionToResponse and responseToException. A bit curious to look into that flow and if we could cut out those tranforms.

github-actions · 2026-05-16T21:12:49Z

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

Based on the diff (252 lines, 7 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

Pull request overview

This PR adds telemetry for DataStore entry point initialization failures so component load issues include Fluid-specific context like DataStore identity and package path.

Changes:

Adds a shared helper for code-artifact-tagged DataStore telemetry properties.
Logs and annotates errors thrown while resolving FluidDataStoreRuntime.entryPoint.
Reuses the shared telemetry helper for DataStore realization errors and adds tests for entry point failure logging.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
`packages/utils/telemetry-utils/src/error.ts`	Aligns DataProcessingError wrapping factory usage.
`packages/runtime/runtime-utils/src/index.ts`	Exports the new DataStore telemetry helper.
`packages/runtime/runtime-utils/src/dataStoreHelpers.ts`	Adds shared DataStore identity telemetry property helper.
`packages/runtime/datastore/src/test/dataStoreRuntime.spec.ts`	Adds coverage for lazy entry point initialization and failure telemetry.
`packages/runtime/datastore/src/dataStoreRuntime.ts`	Wraps, annotates, logs, and rethrows entry point initialization failures.
`packages/runtime/container-runtime/src/dataStoreContext.ts`	Reuses the shared DataStore telemetry helper for realization errors.

github-actions · 2026-05-16T21:17:56Z

Fleet Review — Clean

No issues found across the reviewer fleet for this run.

View run

markfields · 2026-05-18T13:02:26Z

+						error,
+						"entryPointInitialization",
+					);
+					errorWrapped.addTelemetryProperties(


Note: In testing I found that these telemetry props get stripped off on the way up by roundtripping through exceptionToResponse and responseToException. A bit curious to look into that flow and if we could cut out those tranforms.

Does this have to do with privacy concerns? I know there can sometimes be policies to strip all error information away in case it contains user data or identifiable information.

Nope, it has to do with legacy architectural direction (URI-based routing within and across containers)

These properties are correctly tagged as "code artifacts", so hosts can decide if they're safe to log under different circumstances

ChumpChief

Love wrapping at the exit to custom code! Few organizational/API suggestions.

ChumpChief · 2026-05-18T17:14:10Z

-						fluidDataStoreId: this.id,
-					}),
-				);
+				errorWrapped.addTelemetryProperties(dataStoreLoadTelemetryProps(this));


Passing this tends to be a code smell, probably prefer either passing the specific members needed or else make it a member method if it truly needs access to this.

Didn't want to muck up the legacy-beta IFluidDataStoreContext interface. I can consider passing in just the props we need off the context.

That would be my preference too, passing big/powerful objects for small tasks is also usually a code smell :-D

ChumpChief · 2026-05-18T17:22:13Z

-			new LazyPromise(async () => provideEntryPoint(this)),
+			new LazyPromise(async () =>
+				provideEntryPoint(this).catch((error) => {
+					const errorWrapped = DataProcessingError.wrapIfUnrecognized(


Great idea!

agarwal-navin · 2026-05-18T22:11:12Z

-			new LazyPromise(async () => provideEntryPoint(this)),
+			new LazyPromise(async () =>
+				provideEntryPoint(this).catch((error) => {
+					const errorWrapped = DataProcessingError.wrapIfUnrecognized(


I am wondering if DataProcessingError is appropriate here. This runs application entry point logic and has nothing to do with Fluid right? So, maybe this is a UsageError or just a generic error?

Thinking more about this - we should separate out the path that loads the data store / DDS data from the path that loads the application logic. And we should be able to tell these apart from telemetry. I would suggest to not use data process error for the latter because that should not cause any problems with the data (it does fail summaries today but that is an unexpected scenario and will be fixed soon)

agarwal-navin · 2026-05-18T22:20:49Z

-			summarizeNow(createSummarizerResult.summarizer, "test"),
-			"Summarizing should not throw",
-		);
+			// Create a summarizer for the container and do a summary shouldn't throw.


Interesting test. I know you are not changing this but this comment isn't clear. I would suggest updating this to say that the data store is realized / loaded during summarization but is not initialized and hence summarization succeeds.

agarwal-navin · 2026-05-18T22:21:11Z

-				"Non interactive/summarizer client's data object should not be initialized",
-			"Loading data store in summarizer did not throw as it should, or threw an unexpected error.",
-		);
+			// In summarizer, load the data store should fail.


Update this comment to say that this throws an error and also logs the error in itExpects

agarwal-navin · 2026-05-18T22:21:48Z

-			"Initial creation of container and data store should succeed.",
-		);
-	});
+			// Load second container, load the data store will also call initializingFromExisting and succeed.


An observation, don't need to change this I guess - This step seems unnecessary.

markfields added 3 commits May 16, 2026 20:45

Add error logging and decoration when provideEntryPoint throws

4c4540d

Centralize datastore load telemetry props between entryPoint and realize

c99c812

unit tests

3091f0d

Copilot AI review requested due to automatic review settings May 16, 2026 21:11

Copilot started reviewing on behalf of markfields May 16, 2026 21:12 View session

Copilot AI reviewed May 16, 2026

View reviewed changes

Comment thread packages/runtime/container-runtime/src/dataStoreContext.ts

markfields added 3 commits May 16, 2026 22:04

Handle not-yet-undefined pkg

6adb42d

Another test fix

1a05491

formatting

4fbb68a

markfields requested review from ChumpChief, agarwal-navin, alexvy86, anthony-murphy, jatgarg and kian-thompson May 18, 2026 12:59

markfields commented May 18, 2026

View reviewed changes

ChumpChief reviewed May 18, 2026

View reviewed changes

agarwal-navin reviewed May 18, 2026

View reviewed changes

Conversation

markfields commented May 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Uh oh!

github-actions Bot commented May 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

github-actions Bot commented May 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Fleet Review — Clean

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ChumpChief left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

agarwal-navin May 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

markfields commented May 16, 2026 •

edited

Loading

github-actions Bot commented May 16, 2026 •

edited

Loading

github-actions Bot commented May 16, 2026 •

edited

Loading

agarwal-navin May 18, 2026 •

edited

Loading