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

Consider Supporting Resource Pre-Logic Context Hooks #251

Closed
bflad opened this issue Jan 25, 2022 · 2 comments
Closed

Consider Supporting Resource Pre-Logic Context Hooks #251

bflad opened this issue Jan 25, 2022 · 2 comments
Labels
enhancement New feature or request

Comments

@bflad
Copy link
Contributor

bflad commented Jan 25, 2022

Module version

v0.5.0

Use-cases

When attempting to setup a provider with terraform-plugin-log/tflog, trying to add variables via tflog.With() or use subsystems requires each logging context to be explicitly built up in a lot of logic:

  • ApplyResourceChange RPC -> Context Handling -> Create/Update/Delete
  • ImportResourceState RPC -> Context Handling -> ImportState
  • PlanResourceChange RPC -> Context Handling -> PlanModifiers
  • ReadDataSource/ReadResource RPCs -> Context Handling -> Read
  • UpgradeResourceState RPC -> Context Handling -> StateUpgraders (Support upgrading resource state #42)
  • ValidateDataSourceConfig/ValidateResourceConfig RPCs -> Context Handling -> ConfigValidators/GetSchema

Currently the setup must be done within each and every one of those provider defined pieces, which could be confusing and/or error prone.

Attempted Solutions

Adding a helper function, which gets invoked within every CRUD, CustomizeDiff, etc. function. e.g.

func setupLoggingContext(ctx context.Context) context.Context {
  ctx = tflog.With(ctx, "example_key", "example value")

  return ctx
}

func (r exampleResource) Create(ctx context.Context, req CreateResourceRequest, resp *CreateResourceResponse) {
  ctx = setupLoggingContext(ctx)
  // ...
}

func (r exampleResource) Read(ctx context.Context, req ReadResourceRequest, resp *ReadResourceResponse) {
  ctx = setupLoggingContext(ctx)
  // ...
}

func (r exampleResource) Update(ctx context.Context, req UpdateResourceRequest, resp *UpdateResourceResponse) {
  ctx = setupLoggingContext(ctx)
  // ...
}

func (r exampleResource) Delete(ctx context.Context, req DeleteResourceRequest, resp *DeleteResourceResponse) {
  ctx = setupLoggingContext(ctx)
  // ...
}

func (r exampleResource) ModifyPlan(ctx context.Context, req ModifyResourcePlanRequest, resp *ModifyResourcePlanResponse) {
  ctx = setupLoggingContext(ctx)
  // ...
}

Proposal

The SDK could offer a new Resource interface, that can support function logic before all these various logic points are invoked, e.g.

type ResourceWithSetupContext interface {
  Resource

  // SetupContext is a hook which can be used to instantiate details inside the context before it is passed into Resource logic.
  SetupContext func(context.Context) context.Context
}

An example provider implementation:

func(r exampleResource) SetupContext(ctx context.Context) context.Context {
  ctx = tfsdk.NewSubsystem(ctx, /* ... */)

  // Provider may not have been configured yet.
  if r.client.configured {
    ctx = tfsdk.With(ctx, "account_id", r.client.accountID)
    ctx = tfsdk.With(ctx, "region", r.client.region)
  }

  return ctx
}

When defined on a resource, this logic would be invoked automatically prior to executing the provider defined Create, etc. functions.

One thing that isn't immediately clear is whether this should be split up per RPC/SDK functionality per resource, left as a monolithic configuration per resource, or potentially offered at an even higher level (e.g. Provider). Being monolithic at the resource level seems to be the most pragmatic choice given how providers are typically written with "API service" boundaries that may dictate needing multiple configurations across the provider. This does however prevent providing these hooks with "rich" information like Config, Plan, and State since that is dependent on the RPC.

References

@bflad bflad added the enhancement New feature or request label Jan 25, 2022
@bflad
Copy link
Contributor Author

bflad commented Dec 20, 2022

Provider developers can/should use a shared function or method for this purpose.

@bflad bflad closed this as completed Dec 20, 2022
@github-actions
Copy link

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.
If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 20, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

1 participant