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

Need to support getting traces from child dsc processes #512

Closed
SteveL-MSFT opened this issue Aug 4, 2024 · 7 comments · Fixed by #541
Closed

Need to support getting traces from child dsc processes #512

SteveL-MSFT opened this issue Aug 4, 2024 · 7 comments · Fixed by #541
Assignees
Labels
Issue-Enhancement The issue is a feature or idea Needs Triage
Milestone

Comments

@SteveL-MSFT
Copy link
Member

SteveL-MSFT commented Aug 4, 2024

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 #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.

@SteveL-MSFT SteveL-MSFT added Issue-Enhancement The issue is a feature or idea Needs Triage labels Aug 4, 2024
@SteveL-MSFT SteveL-MSFT self-assigned this Aug 4, 2024
@SteveL-MSFT SteveL-MSFT added this to the 3.0-RC milestone Aug 4, 2024
@michaeltlombardi
Copy link
Collaborator

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
Copy link
Member Author

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
Copy link
Collaborator

It seems like we need, at a high level:

  • One or more standardized ways to send a resource non-instance-property - for example, security context, trace-level, potentially other metadata.
  • A way for resources to indicate in their manifest (and surface in the resource info output) what the resource supports.

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
Copy link
Member Author

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
Copy link
Collaborator

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
Copy link
Member Author

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

@SteveL-MSFT
Copy link
Member Author

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).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Issue-Enhancement The issue is a feature or idea Needs Triage
Projects
None yet
2 participants