Add RecommendedReaderVersion advisory mechanism to cDAC RuntimeInfo contract#125029
Add RecommendedReaderVersion advisory mechanism to cDAC RuntimeInfo contract#125029noahfalk wants to merge 2 commits intodotnet:mainfrom
Conversation
|
Tagging subscribers to this area: @steveisok, @tommcdon, @dotnet/dotnet-diag |
|
This adds the freshness check that showed up in the latter portion of the discussion on #124448. I went back and forth on where two version properties should actually live on the API but ultimately putting both on the RuntimeInfo contract seemed like a reasonable choice. I could also see arguments to put the properties on a new contract or treat them as something special that lives outside the contract API entirely. Assuming we keep them where they are we could use the contract doc to describe what functionality each version change contains once we have a 2+ versions to describe. |
There was a problem hiding this comment.
Pull request overview
Adds an advisory “recommended reader version” mechanism to the cDAC RuntimeInfo contract so diagnostic tools can detect when newer contracts are available and suggest updating.
Changes:
- Introduces a new
RecommendedReaderVersionruntime global (backed byCDAC_RECOMMENDED_READER_VERSION) indatadescriptor.inc. - Extends
IRuntimeInfo/RuntimeInfo_1withCurrentReaderVersion(compile-time constant) andRecommendedReaderVersion(reads the runtime global, returns 0 if absent). - Adds unit tests and updates the
RuntimeInfodata contract documentation to describe the scheme.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/native/managed/cdac/tests/RuntimeInfoTests.cs | Adds tests covering RecommendedReaderVersion behavior and updates test target construction to inject numeric globals. |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/RuntimeInfo_1.cs | Implements CurrentReaderVersion and reads RecommendedReaderVersion from target globals. |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Constants.cs | Adds the RecommendedReaderVersion global name constant. |
| src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/Contracts/IRuntimeInfo.cs | Adds the two new properties to the IRuntimeInfo contract surface. |
| src/coreclr/vm/datadescriptor/datadescriptor.inc | Defines CDAC_RECOMMENDED_READER_VERSION and exposes it as a new RecommendedReaderVersion cDAC global. |
| docs/design/datacontracts/RuntimeInfo.md | Documents the new global/property and the reader versioning scheme. |
...e/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/RuntimeInfo_1.cs
Outdated
Show resolved
Hide resolved
...e/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/RuntimeInfo_1.cs
Outdated
Show resolved
Hide resolved
…ontract Adds a new CDAC_RECOMMENDED_READER_VERSION constant and RecommendedReaderVersion global to datadescriptor.inc. The runtime team can increment this value when making changes where updating diagnostic tools is recommended. This is purely advisory and does not necessarily mean a breaking change occurred. The RuntimeInfo contract exposes two new properties: - CurrentReaderVersion: a compile-time constant representing the reader version this contract was authored against (currently 1) - RecommendedReaderVersion: reads the runtime global of the same name Diagnostic tools can compare these to determine whether to recommend an update to the user. When the runtime ships a higher RecommendedReaderVersion than a tool's CurrentReaderVersion, the tool knows newer contracts are available. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
d565cc0 to
30f63dc
Compare
...managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/Contracts/IRuntimeInfo.cs
Outdated
Show resolved
Hide resolved
Address review feedback: convert CurrentReaderVersion and RecommendedReaderVersion properties to GetCurrentReaderVersion() and GetRecommendedReaderVersion() methods to match the style of existing methods on the interface. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
| // Tools can use this to display an advisory notice to users encouraging them to update. Changing | ||
| // this value on its own is not considered a breaking change. See the RuntimeInfo.md data contract for details. | ||
| // When incrementing this value, also implement the new functionality in the cDAC reader and update the | ||
| // CurrentReaderVersion property on the IRuntimeInfo contract in |
There was a problem hiding this comment.
This comment refers to a "CurrentReaderVersion property", but the cDAC contract surface is a GetCurrentReaderVersion() method. Updating the wording here would help avoid confusion for future updates (and keep the guidance consistent with the actual API shape).
| // CurrentReaderVersion property on the IRuntimeInfo contract in | |
| // GetCurrentReaderVersion() implementation on the IRuntimeInfo contract in |
| `CurrentReaderVersion` constant in the cDAC implementation and the `RecommendedReaderVersion` | ||
| global value in the runtime. This causes older tools on older cDAC versions to observe | ||
| RecommendedReaderVersion > CurrentReaderVersion. The tool can notify the user that an update |
There was a problem hiding this comment.
The reader-versioning explanation refers to updating the "CurrentReaderVersion constant" in the reader, but the public contract API is GetCurrentReaderVersion(). Consider adjusting the wording to match the API name (or explicitly call out that it’s the value returned by GetCurrentReaderVersion()) to reduce ambiguity for tool authors.
| `CurrentReaderVersion` constant in the cDAC implementation and the `RecommendedReaderVersion` | |
| global value in the runtime. This causes older tools on older cDAC versions to observe | |
| RecommendedReaderVersion > CurrentReaderVersion. The tool can notify the user that an update | |
| value returned by the `GetCurrentReaderVersion()` API in the cDAC implementation and the `RecommendedReaderVersion` | |
| global value in the runtime. This causes older tools on older cDAC versions to observe | |
| RecommendedReaderVersion > GetCurrentReaderVersion(). The tool can notify the user that an update |
…ontract
Adds a new CDAC_RECOMMENDED_READER_VERSION constant and RecommendedReaderVersion global to datadescriptor.inc. The runtime team can increment this value when making changes where updating diagnostic tools is recommended. This is purely advisory and does not necessarily mean a breaking change occurred.
The RuntimeInfo contract exposes two new properties:
Diagnostic tools can compare these to determine whether to recommend an update to the user. When the runtime ships a higher RecommendedReaderVersion than a tool's CurrentReaderVersion, the tool knows newer contracts are available.