PowerShell / DSC

This repo is for the DSC v3 project
MIT License
197 stars 26 forks source link

Need to support getting traces from child `dsc` processes #512

Closed SteveL-MSFT closed 2 days ago

SteveL-MSFT commented 1 month ago

Summary of the new feature / enhancement

Need to propagate the current tracing level to child resources and accept JSON tracing from child dsc resources.

DSC currently uses the tracing crate JSON format which differs from the simplified format we support for other types of resources.

This will greatly improve diagnosability when things fail using things like the group resource since currently those traces are hidden.

Proposed technical implementation details (optional)

Once https://github.com/PowerShell/DSC/pull/511 is merged, we can set the tracing level as part of the context and define a new argKind that passes the tracing level as an argument which the dsc based resources can accept.

We can then have dsc accept either the tracing JSON format in addition to the simplified format, or add an option to dsc to emit JSON traces using the simplified format. The benefit of accepting the tracing format is that any resources written in Rust can just emit that.

michaeltlombardi commented 1 month ago

The alternatives that occur to me for passing the trace level to the resource are:

  1. To reuse the DSC_TRACE_LEVEL environment variable.

    I think this is the simplest option, but there's no way to have a resource indicate whether it supports tracing unless we add another property to the manifest.

  2. To reuse the metadata pattern for passing information.

    This would, I think, be the more complex route, but worth considering for enabling resource authors and integrating tools to pass and retrieve metadata for resources.

  3. To define a new well-known property.

    I prefer this option as the more explicit and less complicated way to communicate with the resource than metadata and would enable per-resource tracing (sometimes when tracing a config operation, I don't want to have to sift through a dozen resource outputs when running down a specific small behavior).

    The primary downside I see to this is that we can't (currently) cascade values to resources in a configuration document. Configuration authors would either need to use a group resource that cascades the value or to explicitly define/reference it with each resource. I suppose the default value of the property could be the value of DSC_TRACE_LEVEL, which might be enough?

I don't think defining a new argKind would be a bad idea, either.

SteveL-MSFT commented 1 month ago

I did consider the env var route as it seemed the most obvious, but didn't want to introduce a new way to pass info to resources (although DSC_CONFIG_ROOT env var I guess already does that...)

michaeltlombardi commented 1 month ago

It seems like we need, at a high level:

I think the latter could be handled by a capability, indicating that the resource participates explicitly in trace messaging. There's several options for the former.

One thing that occurs to me if we only pass the value as an argKind is that env and stdin input resources can't participate. I'm also not sure of the best way for an adapter to forward trace level to their child resources.

SteveL-MSFT commented 1 month ago

We can defer the env argKind resources until someone needs it as I don't think it'll be the predominate type of resource. Similar to how the manifest defines how the resource wants to receive input, I think we could eventually support both as args and as metadata in the input JSON. The immediate need is purely for dsc itself to make it easier to diagnose issues since that binary plays a role in multiple resources and is particularly difficult to diagnose when used as a group currently.

michaeltlombardi commented 1 month ago

I'm totally on board for deferring, I just think that we need to track/identify the need for passing non-instance-property-data to resources, like trace level and security context, and for those resources to be able to indicate that they support those integrations. I think that's important for resource authors, integrating developers, and users.

I can open a separate issue for that, if preferred.

SteveL-MSFT commented 1 month ago

@michaeltlombardi please open a new issue to generalize an approach for metadata to resources

SteveL-MSFT commented 3 weeks ago

Looks like I had already added a DSC_TRACE_LEVEL env var which is used if defined, but command line --trace-level overrides. The piece that's missing is for dsc to understand the trace JSON that comes out and also have child dsc emit JSON traces (which should just be adding that parameter to the manifest).