Skip to content
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

Add configuration telemetry events #3152

Merged
merged 9 commits into from
Apr 15, 2023
Merged

Conversation

JohnMcPMS
Copy link
Member

@JohnMcPMS JohnMcPMS commented Apr 14, 2023

Change

This change adds telemetry events from configuration actions, as well as more details to improve the error reporting (both in telemetry and to the user). The telemetry events can be disabled in code by the front end. For winget configure, the winget setting that controls telemetry is flowed through into the configuration code.

Two telemetry events are added by this change:

  • An event with details on a failed attempt to execute a configuration unit. This will only be logged for failures of publicly available units. It contains the name of the unit and module, as well as the names of the top level settings provided (but not the values).
  • An event with a summary of processing a configuration set, containing the overall result and error attribution, as well as the counts of configuration units, runs, and failures.

A new value was added to the result information to attribute any error to the appropriate portion of the overall system. The current partitioning of the blame is:

Source Description
None The source is not known, or more likely, there was no failure.
Internal The result came from inside the configuration system; this is likely a bug.
ConfigurationSet The configuration set was ill formed. For instance, referencing a configuration unit that does not exist or a dependency that is not present.
UnitProcessing The external module that processes the configuration unit generated the result.
SystemState The system state is causing the error.
Precondition The configuration unit was not run due to a precondition not being met. For example, when an assert in the configuration set is not in the desired state, all of the units with Apply intent will have this set.

Additionally, another string was added to the result information. The intention is that the existing string (Description) should be used for a "short", user presentable message. The new string (Details), can contain a longer value that is intended for logs or a "more details" type experience.

The PowerShell processor was changed to wrap almost all exceptions into the result information object now that the source can be clearly stated there. Failures coming directly from invoking the resource are attributed to the configuration unit processing, except in the cases where we detect the signature of an invalid setting value (then the configuration set [author] is blamed). All other exceptions are treated as an internal error.

Validation

  • A mechanism for using the diagnostics to examine telemetry event data was created.
  • Existing unit tests were updated to verify the error attribution and telemetry fields.
  • New tests were added to verify the telemetry controls and fields.
Microsoft Reviewers: Open in CodeFlow

@JohnMcPMS JohnMcPMS requested a review from a team as a code owner April 14, 2023 03:19
@@ -11,6 +11,11 @@ namespace Microsoft.Management.Configuration.Processor.Exceptions
/// </summary>
internal static class ErrorCodes
{
/// <summary>
/// The module of the unit was installed, but the unit was not found.
Copy link
Contributor

Choose a reason for hiding this comment

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

Update

public InvokeDscResourceException(string method, string resourceName, ModuleSpecification? module, string message, bool configurationSetSource = false)
: base(CreateMessage(method, resourceName, module, message))
{
// The property 'Fake' cannot be found on this object. Verify that the property exists and can be set.
Copy link
Contributor

Choose a reason for hiding this comment

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

This comment seems like it should be in ContainsPropertyError

@JohnMcPMS JohnMcPMS merged commit 02d2f93 into microsoft:master Apr 15, 2023
@JohnMcPMS JohnMcPMS deleted the config-telem branch April 15, 2023 21:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants